Closed Bug 540774 Opened 13 years ago Closed 13 years ago

"Assertion failure: top < StackDepth(ss->printer->script)" decompiling upvar

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: jruderman, Assigned: brendan)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

js> "" + (function() { var m; this.e = function() { return m; }; })
Assertion failure: top < StackDepth(ss->printer->script), at ../jsopcode.cpp:998

This affects part of jsfunfuzz itself, so jsfunfuzz is hitting it very frequently.
I see this too, on 10.6.2.

autoBisect shows this is probably related to bug 536564:

The first bad revision is:
changeset:   37037:36bbd730e24f
user:        Brendan Eich
date:        Thu Jan 14 09:33:14 2010 -0800
summary:     Analyze module pattern and private-statics pattern in order to despecialize from methods to slots/sprops (536564, r=jorendorff).
Blocks: 536564
Keywords: regression
Hardware: PowerPC → All
Jason, do you have time for this or should I take it?
I can fix this, it's my fault for using SRC_HIDDEN but expecting it to affect the decompiler's stack model.

I still do not think we should hide tm-only bugs like this one.

/be
Assignee: general → brendan
OS: Mac OS X → All
Priority: -- → P1
Target Milestone: --- → mozilla1.9.3a1
Status: NEW → ASSIGNED
Gary, my reading of the code, and testing on Mac, shows that we recover from this in opt builds. This bug should *not* be s-s.

/be
Group: core-security
(In reply to comment #4)
> Gary, my reading of the code, and testing on Mac, shows that we recover from
> this in opt builds. This bug should *not* be s-s.
> 
> /be

Indeed. Jesse probably filed this as s-s because it affects jsfunfuzz frequently, rather than it being a security bug by itself.
Attached patch fixSplinter Review
Brute force, when in doubt.

/be
Attachment #422649 - Flags: review?(jorendorff)
Attachment #422649 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/tracemonkey/rev/36377d06bb35

/be
Whiteboard: fixed-in-tracemonkey
The alternative would be to add a JSOP_UNBRAND case that peeks for JSOP_POP after, avoiding SRC_HIDDEN. That would cost a bit more code in the decompiler while avoiding the SRC_HIDDEN.

Either way, something is hardcoded: that a hidden pop after an unbrand is nevertheless "main line" so the decompiler must update its model stack depth; or else that any "main line" abstract interpreter must special-case pop-after-unbrand to hide the entire "unbrand statement", while again updating its stack depth model.

I'll let this stand unless someone can make the case for the other way.

/be
http://hg.mozilla.org/mozilla-central/rev/36377d06bb35
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.