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)
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).
Comment 1•15 years ago
|
||
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
Assignee | ||
Comment 2•15 years ago
|
||
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
Assignee | ||
Comment 3•15 years ago
|
||
__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
Updated•15 years ago
|
status1.9.1:
--- → wanted
Flags: wanted1.9.1.x?
Reporter | ||
Updated•15 years ago
|
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
Comment 4•15 years ago
|
||
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
Comment 5•15 years ago
|
||
(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).
Comment 6•15 years ago
|
||
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?
Reporter | ||
Comment 7•15 years ago
|
||
> 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.
Assignee | ||
Comment 8•15 years ago
|
||
Yeah, still working on this amongst other bugs and reviews -- continuing investigation today for sure.
Comment 9•15 years ago
|
||
Any progress here? I am a bit worried about this one.
Assignee | ||
Comment 10•15 years ago
|
||
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?]
Assignee | ||
Comment 11•15 years ago
|
||
Er. Not write anywhere, sorry, read from anywhere -- whole different ballgame!
Whiteboard: [sg:critical?]
Comment 12•15 years ago
|
||
How can {} and Object.prototype have the same shape if only one of them has toString? I thought we brand scopes.
Comment 13•15 years ago
|
||
(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
Comment 14•15 years ago
|
||
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
Comment 15•15 years ago
|
||
Is this bug a dup of bug 501986 ?
/be
Comment 16•15 years ago
|
||
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.
Reporter | ||
Comment 17•15 years ago
|
||
WFM on tracemonkey branch (rev 8e334d4b4d36).
Comment 18•15 years ago
|
||
(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
Comment 19•15 years ago
|
||
(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.
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
blocking1.9.1: ? → ---
Closed: 15 years ago
status1.9.1:
wanted → ---
Resolution: --- → DUPLICATE
Updated•15 years ago
|
Flags: blocking1.9.2?
Updated•15 years ago
|
Whiteboard: [sg:dupe 503080]
Updated•13 years ago
|
Group: core-security
Comment 21•12 years ago
|
||
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.
Description
•