Closed Bug 503620 Opened 15 years ago Closed 15 years ago

TM: crash in JITted code with property iteration after calling method from prototype chain

Categories

(Core :: JavaScript Engine, defect)

1.9.1 Branch
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 503080

People

(Reporter: jruderman, Assigned: Waldo)

References

Details

(4 keywords, Whiteboard: [sg:dupe 503080])

var t = {f:1, g:2, q:{}, b:Object.prototype}; try { t.__defineSetter__("d", 3); } catch(e) { } for (var w = 0; w < 2; ++w) { for (var p in t) { try { "" + t[p]; } catch(e) { } } } Null deref in JITted code (under js_MonitorLoopEdge).
autoBisect shows this is probably related to bug 479110 : The first bad revision is: changeset: 26780:790c0eda3122 user: Andreas Gal date: Sat Apr 04 17:23:34 2009 -0400 summary: Bug 479110 - TM: avoid frequent mismatch exits. r=brendan
Blocks: 479110
Flags: wanted1.9.1.x?
Flags: blocking1.9.2?
Keywords: regression
Stealing, although if someone feels motivated enough to do this over the weekend feel free -- I might or might not get to it in that time.
Assignee: general → jwalden+bmo
__defineSetter__ is a complete red herring -- looks like all that's necessary is for a method found on the prototype chain to be called. This crashes identically to what's in comment 0: var t = {f:1, g:2, q:{}, b:Object.prototype}; t.isPrototypeOf(); for (var w = 0; w < 2; ++w) { for (var p in t) "" + t[p]; }
Status: NEW → ASSIGNED
Flags: wanted1.9.1.x?
Summary: TM: crash in JITted code with property iteration after failed defineSetter → TM: crash in JITted code with property iteration after calling method from prototype chain
This affects 1.9.1 as well. Setting to security-sensitive because the stacks show memory addresses at the top 2 lines of the stack. It is also a topcrash (we don't want another repeat of the 3.5.1 firedrill, don't we?). bp-7df1adc8-b6b3-492a-a5e9-472522090715
Group: core-security
blocking1.9.1: --- → ?
Keywords: topcrash
OS: Mac OS X → All
Hardware: x86 → All
Version: Trunk → 1.9.1 Branch
(In reply to comment #4) > This affects 1.9.1 as well. Setting to security-sensitive because the stacks > show memory addresses at the top 2 lines of the stack. It is also a topcrash > (we don't want another repeat of the 3.5.1 firedrill, don't we?). > > bp-7df1adc8-b6b3-492a-a5e9-472522090715 Topcrash at #41, and also this stack above gleaned from the Waldo's testcase matches the entirety of other users' stacks (with the top 2 lines being memory addresses).
Looks like a near-null crash, but still. Lets learn from 2 days ago. Waldo, are you still working on this or want me to take a look?
> Setting to security-sensitive because the stacks > show memory addresses at the top 2 lines of the stack. When you see memory addresses above js_MonitorLoopEdge on the stack, that just means it's in JIT-generated code. To guess whether a JIT crash is a security problem, you can look at the address it tried to access (near-null in this case). But in the JIT, it often takes some debugging to find out for sure that the bug is safe.
Yeah, still working on this amongst other bugs and reviews -- continuing investigation today for sure.
Any progress here? I am a bit worried about this one.
The problem seems to be that {} and Object.prototype have the same shape (they share scopes), and when we go into imacro code to try to call toString we shape-guard, assume based on that that we're good to access the slot containing toString, and crash because {} has NULL dslots. Is that bad? Probably; you can write anywhere if you set up the shared scope correctly, although then you have to unwind enough to take advantage of that write -- probably feasible, not necessarily easy. As to why the call up the proto-chain makes it crash...not sure yet. Still investigating.
Whiteboard: [sg:critical?]
Er. Not write anywhere, sorry, read from anywhere -- whole different ballgame!
Whiteboard: [sg:critical?]
How can {} and Object.prototype have the same shape if only one of them has toString? I thought we brand scopes.
(In reply to comment #12) > How can {} and Object.prototype have the same shape if only one of them has > toString? I thought we brand scopes. Because {} is unmutated. See bug 503080, which is about to be fixed. /be
Branding is done on call, that's not the issue in any scenario. Even if we branded "methods" when set, {} shares the proto-scope owned by Object.prototype. This is a big fat bug, but jorendorff will fix it in bug 503080. /be
Is this bug a dup of bug 501986 ? /be
If this is a dupe, can we get bug 501986 nominated, instead? We're trying to decide if this is blocking 3.5.2, or blocking-when-we-get-a-patch.
WFM on tracemonkey branch (rev 8e334d4b4d36).
(In reply to comment #17) > WFM on tracemonkey branch (rev 8e334d4b4d36). Strongly suggests this is a dup of bug 501986 which in turn was fixed by the patch for bug 503080. Unless there's a spot-fix for bug 501986 that doesn't regress something we can't stand regressing (perf), we could track bug 503080's patch and take it for 1.9.1.x when it's baked. I'll let someone else mark this a dup. /be
(In reply to comment #18) > (In reply to comment #17) > > WFM on tracemonkey branch (rev 8e334d4b4d36). > > Strongly suggests this is a dup of bug 501986 which in turn was fixed by the > patch for bug 503080. autoBisect gladly confirms that this bug is fixed by bug 503080 : The first bad revision is: changeset: 30378:3915e2d2c748 user: Jason Orendorff date: Tue Jul 21 16:25:11 2009 -0500 summary: Bug 503080 - Remove prototype-scope-sharing. r=brendan.
Status: ASSIGNED → RESOLVED
blocking1.9.1: ? → ---
Closed: 15 years ago
Resolution: --- → DUPLICATE
Flags: blocking1.9.2?
Whiteboard: [sg:dupe 503080]
Group: core-security
A testcase for this bug was already added in the original bug (bug 503080).
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.