Summary: | REGRESSION (r36267): visiting this site reliably crashes WebKit nightly | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Paul Downey <paul.downey> | ||||||||
Component: | JavaScriptCore | Assignee: | Oliver Hunt <oliver> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gsherwood, mjs | ||||||||
Priority: | P1 | Keywords: | InRadar, NeedsReduction, Regression | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.5 | ||||||||||
URL: | http://tiddlytools.com | ||||||||||
Attachments: |
|
Description
Paul Downey
2008-12-17 10:26:08 PST
Confirmed with a local release build r39393. Thread 0 Crashed: 0 com.apple.JavaScriptCore 0x006d8111 JSC::Interpreter::cti_op_eq(void*, ...) + 497 (JSCell.h:181) I'll take this on. Created attachment 26164 [details]
Stack trace
Strangely, this bug is caused by r36267, the introduction of the eval code cache. This is the offending JS being cached: function _out(place){ var s=place.lastChild.button.style; s.marginLeft=".3em"; s.display="block"; s.padding=".2em"; };_out(w.output); It has 136 characters, and if I change EvalCodeCache to only cache scripts with 136 chars, it still crashes with this being the only script cached. It also isn't executing the same eval CodeBlock reentrantly, if there was some subtle problem with that. I wonder if there's subtle dependency built into the eval code when it's compiled, which doesn't hold when it's reused. I am getting what might be similar crashes from some blogs such as: www.instapundit.com Always reproduceable with builds against libcurl and libsoup. Seems like it has started over the past couple months. I'm wondering if the eval cache goes wrong if the eval defines a function -- eg, an activation captures the scope chain of the eval... i wonder if this could cause something to go wrong with the scope chain? I *think* I have it. As far as i can tell eval("function f(){...}; f()") will cache code block containing a function f(), but if the function f never escapes the eval, there will be no reference to it from anywhere but the eval cache, and the eval cache does not mark cached data leading to the possibility of the function and/or its constants being collected. (In reply to comment #10) > I *think* I have it. As far as i can tell > eval("function f(){...}; f()") > > will cache code block containing a function f(), but if the function f never > escapes the eval, there will be no reference to it from anywhere but the eval > cache, and the eval cache does not mark cached data leading to the possibility > of the function and/or its constants being collected. Let me try marking the cached CodeBlock and see if that fixes the bug. Well, a patch that should have fixed the marking does not seem to have done the trick :-/ :-/ As far as i can tell there must be some kind of gc error, but i cannot fathom what it is. Okay, my first version of the patch assumed that functions were being kept on the cached codeblock, but this was not the case. Have now corrected that assumption and everything seems to work. I cannot for the life of me make a testcase for this and it should be trivial to do so. grumble. Created attachment 26724 [details]
Fixeration!
\o/
Here is a test case that crashes before Oliver's patch but works afterwards, at least with the JSC shell and COLLECT_ON_EVERY_ALLOCATION enabled: var s = "function f() { print('foo'); } f();"; var evalString = function() { return eval(s); } evalString(); [[[1,2,3],[1,2,3],[1,2,3]],[[1,2,3],[1,2,3]]] print(evalString()); I'll try to make something for DRT. Comment on attachment 26724 [details]
Fixeration!
Have finally managed to make testcase that works
Created attachment 26730 [details]
Fixeration mkII
Fixed with layout test
Comment on attachment 26730 [details] Fixeration mkII > // We don't need to mark our own codeblock as the JSGlobalObject takes care of that "codeblock" should be CodeBlock. Other than that, r=me. Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/bytecode/CodeBlock.cpp M JavaScriptCore/bytecode/CodeBlock.h M JavaScriptCore/bytecode/EvalCodeCache.h M JavaScriptCore/parser/Nodes.cpp M JavaScriptCore/parser/Nodes.h M LayoutTests/ChangeLog A LayoutTests/fast/js/cached-eval-gc-expected.txt A LayoutTests/fast/js/cached-eval-gc.html A LayoutTests/fast/js/resources/cached-eval-gc.js Committed r39910 |