Closed Bug 1122833 Opened 5 years ago Closed 5 years ago

Assertion failure: !isInterpretedLazy(), at jsfun.h or Assertion failure: hasScript(), at jsfun.h

Categories

(Core :: JavaScript Engine, defect, critical)

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox36 --- unaffected
firefox37 --- unaffected
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: gkw, Assigned: shu)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(3 files)

// jsfunfuzz-generated code
s = newGlobal();
// Randomly chosen test: js/src/jit-test/tests/auto-regress/bug490191.js
evalcx("\
    (function() {\
        eval(\" \
            (function(){ \
                __defineGetter__(\\\"y\\\", function(){}) \
            }); \
        \")();\
    })()\
", s);
gc()
gc()
evalcx("\
    setJitCompilerOption('baseline.warmup.trigger', 1);\
    y;\
", s)

asserts js debug shell on m-c changeset 5438e3f74848 with --fuzzing-safe --no-threads --ion-eager --no-fpu at Assertion failure: !isInterpretedLazy(), at jsfun.h. During reduction, I also saw this: Assertion failure: hasScript(), at jsfun.h

Debug configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random js tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/5438e3f74848/js/src/jit-test/tests/auto-regress/bug490191.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b14f46d65f73
user:        Shu-yu Guo
date:        Wed Jan 14 15:18:42 2015 -0800
summary:     Bug 963879 - Part 1: Overhaul ScopeIter and StaticScopeIter to share iteration logic and to go through evals. (r=luke)

Setting s-s and assuming sec-critical because this seems to involve gc. Shu-yu, is bug 963879 a likely regressor?
Flags: needinfo?(shu)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x51dbe, 0x0002672f js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`JSFunction::isHeavyweight(this=<unavailable>) const + 207 at jsfun.h:94, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0002672f js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`JSFunction::isHeavyweight(this=<unavailable>) const + 207 at jsfun.h:94
    frame #1: 0x0082c0e8 js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`js::StaticScopeIter<(this=<unavailable>)0>::hasDynamicScopeObject() const + 104 at ScopeObject-inl.h:106
    frame #2: 0x007fd878 js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`AssertDynamicScopeMatchesStaticScope(cx=<unavailable>, script=<unavailable>, scope=<unavailable>) + 216 at Stack.cpp:147
    frame #3: 0x007fd3ba js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`js::InterpreterFrame::prologue(this=0x02856e10, cx=0x01d67b80) + 442 at Stack.cpp:212
    frame #4: 0x0077577b js-dbg-opt-32-dm-nsprBuild-darwin-5438e3f74848`Interpret(cx=<unavailable>, state=<unavailable>) + 971 at Interpreter.cpp:1508
(lldb)
Duplicate of this bug: 1122802
This is a relazification crash.

I'm not sure why my patch surfaced this, since the StaticScopeIter worked basically the same way before. The problem is that whether or not a JSFunction is heavyweight can only be checked on delazified JSFunctions. So SSI has to possibly delazify functions to check hasDynamicScopeChain. But there are several places that use StaticScopeIter<NoGC>, so I'm not sure how to fix...
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Explanation of bug in the comment in the patch.
Attachment #8550672 - Flags: review?(till)
Comment on attachment 8550672 [details] [diff] [review]
Don't relazify scripts with direct eval.

Review of attachment 8550672 [details] [diff] [review]:
-----------------------------------------------------------------

Great catch! I wonder how this worked up until now ... It might well be the source of (some of) our various lazy script-related crashes.
Attachment #8550672 - Flags: review?(till) → review+
Does the underlying defect affect older branches too?
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #8)
> Does the underlying defect affect older branches too?

Hmm, yes it does.
Please don't use being a security bug that affects older branches as a reason not to land either this, some other workaround, or a backout on inbound/central -- debug builds are basically unusable for real web browsing right now, given that they crash on most google properties, new york times articles, etc. (bug 1122364).  That's an unacceptable state to leave central builds in.
(In reply to Till Schneidereit [:till] from comment #9)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #8)
> > Does the underlying defect affect older branches too?
> 
> Hmm, yes it does.

No, it doesn't affect older branches. Prior to my patch, functions with direct evals wouldn't be on their inner functions-by-way-of-eval's static scope chains.
(In reply to Shu-yu Guo [:shu] from comment #11)
> No, it doesn't affect older branches. Prior to my patch, functions with
> direct evals wouldn't be on their inner functions-by-way-of-eval's static
> scope chains.

Oh, I see. There goes my hope regarding the other crashes, then.
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/f0ab64c88102
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Is there any followup bug to land a test case and/or make isHeavyweight less of a relazification hazard (either renaming it to include the not-relazified assumption or, as discussed on irc on Friday, moving the heavyweight bit to JSFunction::flags or LazyScript)?  I didn't notice one, and it seems like we should do that given the super-tenuous current situation.
(In reply to Luke Wagner [:luke] from comment #14)
> Is there any followup bug to land a test case and/or make isHeavyweight less
> of a relazification hazard (either renaming it to include the not-relazified
> assumption or, as discussed on irc on Friday, moving the heavyweight bit to
> JSFunction::flags or LazyScript)?  I didn't notice one, and it seems like we
> should do that given the super-tenuous current situation.

Not yet. The tricky thing is that even if we move it off the JSScript, it's not known until the first delazification. So really the bit still intrinsically lives on JSScript, but is cached elsewhere after it's been computed. I don't really feel great about that scheme, it doesn't fundamentally make the situation cleaner.
(In reply to Shu-yu Guo [:shu] from comment #15)
> I don't really feel great about that scheme, it doesn't
> fundamentally make the situation cleaner.

I think what you're saying is that, even with this change, one could not access isHeavyweight in all states.  That makes sense.  I was thinking, though: isHeavyweight is defined in SharedContext.h to be the disjunction of a few simple conditions, all of which it seems like we can easily/cheaply determine during the syntax-only parse.  So on first glance it seems like we could determine isHeavyweight eagerly.
FWIW I ran into a similar issue in bug 1123120 with the new relazifyFunctions() shell function and -D. There we have js_Disassemble1 -> ScopeCoordinateName -> ScopeCoordinateToStaticScopeShape -> StaticScopeIter -> isHeavyWeight.

The easiest fix there is to not relazify scripts with pc counts, but I agree it'd be nice if we could fix isHeavyweight.
Duplicate of this bug: 1122364
Group: javascript-core-security
Group: core-security
You need to log in before you can comment on or make changes to this bug.