Closed
Bug 285219
Opened 19 years ago
Closed 19 years ago
RangeError: reserved slot index out of range
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.8beta2
People
(Reporter: timeless, Assigned: brendan)
References
()
Details
(Keywords: js1.5)
Attachments
(1 file, 2 obsolete files)
3.17 KB,
patch
|
shaver
:
review+
|
Details | Diff | Splinter Review |
analysis from brendan: eval(__s, __o) ... same as __o.eval(__s), more or less that's the bug http://lxr.mozilla.org/mozilla/source/extensions/venkman/resources/content/venkman-eval.js#50
Assignee | ||
Comment 1•19 years ago
|
||
Yeah, the push-a-mostly-zeros-compilation-frame code is copied and specialized in four places. I don't want to mess with coalescing it now; it hates me, and I'll probably bust something. /be
Attachment #176696 -
Flags: review?(shaver)
Assignee | ||
Updated•19 years ago
|
Assignee | ||
Comment 2•19 years ago
|
||
Comment on attachment 176696 [details] [diff] [review] proposed fix Oops, that wasn't the patch I wanted to attach. /be
Attachment #176696 -
Attachment is obsolete: true
Attachment #176696 -
Flags: review?(shaver)
Assignee | ||
Comment 3•19 years ago
|
||
Rediff, then attach, duh. Anyway, all good here. Testcase is trivial -- this should not crash you in the js shell: var o = {hi: 'there'}; eval("var r = /re(1)(2)(3)/g", o); /be
Attachment #176698 -
Flags: review?(shaver)
Comment on attachment 176698 [details] [diff] [review] proposed fix Aw, it doesn't hate you, it just has a bit of a temper. r=shaver
Attachment #176698 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 5•19 years ago
|
||
Fixed. /be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 6•19 years ago
|
||
Brendan, your test case definitely crashes in the js shell, but when I put it in the test library framework it doesn't crash. ?
QA Contact: pschwartau → moz
Assignee | ||
Comment 7•19 years ago
|
||
Bob, did you wrap the test in a function? Any other changes? /be
Comment 8•19 years ago
|
||
Could this have caused bug 285244?
Comment 9•19 years ago
|
||
(In reply to comment #8) > Could this have caused bug 285244? if it mess with file js3250.dll so it seem that this "fix" caused bug 285244 i just replaced today js3250.dll with the same file from 2005-03-07 trunk and bug 285244 disappear
Comment 10•19 years ago
|
||
(In reply to comment #7) > Bob, did you wrap the test in a function? Any other changes? Nope. I need to revert my build to test this again. More news at 11.
Comment 11•19 years ago
|
||
Yep, this caused bug 285244. Tested by locally applying to a build that did not have the bug.
Depends on: 285244
Assignee | ||
Updated•19 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•19 years ago
|
||
bz is kindly offering to test this. /be
Attachment #176698 -
Attachment is obsolete: true
Attachment #176789 -
Flags: review?(shaver)
Assignee | ||
Comment 13•19 years ago
|
||
The attachment 176789 [details] [diff] [review] cures the bug in the previous patch: JSFRAME_EVAL should not be propagated from a caller into a compiler frame for a function (body). Function bodies are separately scoped and activated scripts, so the deoptimizations that accompany eval should not afflict them. Here, the deoptimization from JSOP_SETVAR to JSOP_SETCONST was not accompanied by the usual logic to set JSFUN_HEAVYWEIGHT, so we end up with a crash in js_Interpret, case JSOP_SETCONST:, expecting fp->varobj to be non-null when it wasn't set up as it would have been were the function flagged heavyweight. Many thanks to bz for testing and debugging. Selected IRC log: <bz> aha <bz> here we go <bz> bingo <bz> (gdb) p fp->flags & 0x30 <bz> $2 = 32 <bz> You want the stack here, right? <brendan> y <bz> http://web.mit.edu/bzbarsky/www/test.txt <brendan> frame 26 <brendan> er, 27 <brendan> p *script <bz> (gdb) p *script <bz> $3 = {code = 0x84d2c18 ";", length = 525, main = 0x84d2c18 ";", version = 0, <bz> numGlobalVars = 0, atomMap = {vector = 0x84d3cc0, length = 59}, <bz> filename = 0x84ccb7d "chrome://minit_drag/content/tablib.js", lineno = 1, depth = 7, <bz> trynotes = 0x84d2f14, principals = 0x80a7784, object = 0x0} <brendan> oh, an extension <brendan> it calls eval <brendan> there's the missing call <brendan> ok, thanks [. . .] <bz> Looking * bz is trying to find the file <bz> note that all steps to reproduce need an extension <brendan> y <bz> though apparently a whole bunch trigger this <brendan> copy/paste, the story of js code creation <brendan> ;-) <bz> yeah <bz> ok, have file open <bz> one eval call <bz> if(gBrowser.mTabContainer._appendChild) //flowing tabs <bz> eval("miniT_drag.getNewIndex ="+miniT_drag.getNewIndex.toString().replace( <bz> 'return i;', <bz> 'if(event.clientY < gBrowser.mTabs[i].boxObject.y + gBrowser.mTabs[i].boxObject.height)\ <bz> return i;')); <brendan> insanity <brendan> source rewriting and recompiling <bz> yeah <brendan> self-modifying code <bz> uh-huh /be
Status: REOPENED → ASSIGNED
Comment on attachment 176789 [details] [diff] [review] one-line fix to last patch, should fix the regression r=shaver, best one ever.
Attachment #176789 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 15•19 years ago
|
||
Fixed, for real. /be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Comment 16•19 years ago
|
||
Checking in regress-285219.js; /cvsroot/mozilla/js/tests/ecma_3/RegExp/regress-285219.js,v <-- regress-285219.js initial revision: 1.1
Flags: testcase+
You need to log in
before you can comment on or make changes to this bug.
Description
•