Closed Bug 1491336 Opened 3 years ago Closed 3 years ago

Crash [@ js::Scope::environmentChainLength] or Assertion failure: lazyScript->enclosingScriptHasEverBeenCompiled(), at vm/Debugger.cpp:5563


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed
firefox64 --- fixed


(Reporter: decoder, Assigned: arai)



(5 keywords, Whiteboard: [jsbugmon:update])

Crash Data


(2 files)

The following testcase crashes on mozilla-central revision 2a4cf603095a (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --disable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe min.js):

var g = newGlobal();
var dbg = new Debugger(g);
g.eval("" + function f() {
    () => {};
for (s of dbg.findScripts()) s.lineCount;


received signal SIGSEGV, Segmentation fault.
0x08646d2b in js::Scope::environmentChainLength (this=0x0) at js/src/vm/Scope.cpp:344
#0  0x08646d2b in js::Scope::environmentChainLength (this=0x0) at js/src/vm/Scope.cpp:344
#1  0x087e35cf in js::frontend::EmitterScope::checkEnvironmentChainLength (this=0xffffc22c, bce=0xffffc458) at js/src/frontend/EmitterScope.cpp:67
#2  0x087eab6d in js::frontend::EmitterScope::enterFunction (this=<optimized out>, bce=<optimized out>, funbox=<optimized out>) at js/src/frontend/EmitterScope.cpp:639
#3  0x087da688 in js::frontend::BytecodeEmitter::emitFunctionFormalParametersAndBody (this=0xffffc458, pn=0xf6ebe0a0) at js/src/frontend/BytecodeEmitter.cpp:7427
#4  0x087c77fa in js::frontend::BytecodeEmitter::emitTree (this=0xffffc458, pn=0xf6ebe0a0, valueUsage=js::frontend::ValueUsage::WantValue, emitLineNote=js::frontend::BytecodeEmitter::EMIT_LINENOTE) at js/src/frontend/BytecodeEmitter.cpp:7943
#5  0x087c80f3 in js::frontend::BytecodeEmitter::emitTree (this=<optimized out>, pn=<optimized out>, valueUsage=js::frontend::ValueUsage::WantValue, emitLineNote=<optimized out>, valueUsage=js::frontend::ValueUsage::WantValue) at js/src/frontend/BytecodeEmitter.cpp:8330
#6  0x087c837d in js::frontend::BytecodeEmitter::emitFunctionScript (this=<optimized out>, fn=<optimized out>, isTopLevel=<optimized out>) at js/src/frontend/BytecodeEmitter.cpp:2638
#7  0x087c955b in js::frontend::CompileLazyFunction (cx=<optimized out>, lazy=..., chars=<optimized out>, length=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:851
#8  0x085d4f68 in JSFunction::createScriptForLazilyInterpretedFunction (cx=0xf6e1a800, fun=...) at js/src/vm/JSFunction.cpp:1640
#9  0x0854694d in JSFunction::getOrCreateScript (cx=0xf6e1a800, fun=...) at js/src/vm/JSFunction.h:536
#10 0x0856bf93 in js::LazyScript::functionDelazifying (script=..., cx=0xf6e1a800) at js/src/vm/JSScript-inl.h:83
#11 DelazifyScript (cx=cx@entry=0xf6e1a800, lazyScript=lazyScript@entry=...) at js/src/vm/Debugger.cpp:5294
#12 0x0856d4bd in DebuggerScriptGetLineCountMatcher::match (lazyScript=..., this=<synthetic pointer>) at js/src/vm/Debugger.cpp:5627
#13 JS::detail::GCVariantImplementation<js::LazyScript*, js::WasmInstanceObject*>::match<DebuggerScriptGetLineCountMatcher, mozilla::Variant<JSScript*, js::LazyScript*, js::WasmInstanceObject*> > (v=..., matcher=<synthetic pointer>) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/32/compiler/gcc/instrumentation/none/type/opt/dist/include/js/GCVariant.h:102
#14 JS::detail::GCVariantImplementation<JSScript*, js::LazyScript*, js::WasmInstanceObject*>::match<DebuggerScriptGetLineCountMatcher, mozilla::Variant<JSScript*, js::LazyScript*, js::WasmInstanceObject*> > (v=..., matcher=<synthetic pointer>) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/32/compiler/gcc/instrumentation/none/type/opt/dist/include/js/GCVariant.h:104
#15 js::MutableWrappedPtrOperations<mozilla::Variant<JSScript*, js::LazyScript*, js::WasmInstanceObject*>, JS::Rooted<mozilla::Variant<JSScript*, js::LazyScript*, js::WasmInstanceObject*> > >::match<DebuggerScriptGetLineCountMatcher> (matcher=<synthetic pointer>, this=0xffffcc30) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/32/compiler/gcc/instrumentation/none/type/opt/dist/include/js/GCVariant.h:182
#16 DebuggerScript_getLineCount (cx=0xf6e1a800, argc=0, vp=0xffffcd48) at js/src/vm/Debugger.cpp:5646
#17 0x081d0777 in CallJSNative (args=..., native=0x856d310 <DebuggerScript_getLineCount(JSContext*, unsigned int, JS::Value*)>, cx=0xf6e1a800) at js/src/vm/Interpreter.cpp:449
#40 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9973
eax	0x0	0
ebx	0xffffc22c	-15828
ecx	0x8b96ff4	146370548
edx	0x0	0
esi	0x1	1
edi	0xffffc458	-15272
ebp	0x0	0
esp	0xffffbf54	4294950740
eip	0x8646d2b <js::Scope::environmentChainLength() const+43>
=> 0x8646d2b <js::Scope::environmentChainLength() const+43>:	movzbl 0x4(%edx),%ecx
   0x8646d2f <js::Scope::environmentChainLength() const+47>:	cmp    $0xc,%cl
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
user:        Tooru Fujisawa
date:        Thu Jul 26 12:36:18 2018 +0900
summary:     Bug 1434305 - Part 10: Support LazyScript in Debugger::ScriptQuery::findScripts. r=jimb

This iteration took 242.111 seconds to run.
Arai-san, is bug 1434305 a likely regressor?
Blocks: 1434305
Flags: needinfo?(arai.unmht)
yeah, the arrow function is removed by constant folding, and the function isn't get emitted.
we should check if the function is emitted, after delazifying the enclosing script,
and if it's not emitted, we should throw error
Now, accessing the properties which triggers delazification throws if the function for the script is removed by constant folding or DCE or something.
Assignee: nobody → arai.unmht
Flags: needinfo?(arai.unmht)
Attachment #9010134 - Flags: review?(jimb)
Can we get review here?
Flags: needinfo?(jimb)
Attachment #9010134 - Flags: review?(jorendorff)
Comment on attachment 9010134 [details] [diff] [review]
Do not support delazification of functions which is optimized out.

Review of attachment 9010134 [details] [diff] [review]:

Thank you, arai.

::: js/src/jit-test/tests/debug/Debugger-findScripts-optimized-out.js
@@ +22,5 @@
> +        continue;
> +    }
> +
> +    // Check inner functions only.
> +    if (s.displayName.startsWith("inner") || s.displayName.startsWith("enclosing/")) {

displayName no longer includes this information, so the test doesn't actually run for any of the scripts.

So maybe:

    try {
        s.lineCount;  // don't assert
    } catch (exc) {
        // If that didn't throw, it's fine. If it did, check the message.
        assertEq(exc.message, "function is optimized out");

::: js/src/vm/Debugger.cpp
@@ +5572,5 @@
> +
> +        if (!lazyScript->enclosingScriptHasEverBeenCompiled()) {
> +            // If the function corresponds to this script is removed by
> +            // constant folding, delazifying the enclosing script doesn't touch
> +            // this script while emitting the bytecode.

How about:

    // It didn't work! Delazifying the enclosing script still didn't delazify
    // this script. This happens when the function corresponding to this script
    // was removed by constant folding.
Attachment #9010134 - Flags: review?(jorendorff)
Attachment #9010134 - Flags: review?(jimb)
Attachment #9010134 - Flags: review+
Bug 1491336 - Do not support delazification of functions which is optimized out. r=jorendorff
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Is there a user impact which justifies backport consideration here or can this ride the trains?
Flags: needinfo?(jimb)
Flags: needinfo?(arai.unmht)
Flags: in-testsuite+
[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1434305

User impact if declined: Possible crash with null-dereference while setting breakpoint in debugger

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This adds error handling for a path that was previously assertion

String changes made/needed:
Flags: needinfo?(arai.unmht)
Attachment #9015101 - Flags: review+
Attachment #9015101 - Flags: approval-mozilla-beta?
Comment on attachment 9015101 [details] [diff] [review]
(mozilla-beta) Do not support delazification of functions which is optimized out. r=jorendorff

Crash fix, uplift approved for 63 beta 13, thanks.
Attachment #9015101 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.