1688190 - AddressSanitizer: SEGV or Assertion failure: !warmUpData_.isEnclosingScript() (Enclosing scope is not computed yet), at vm/JSScript.cpp:705

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Debug stack

setDiscardSource();
let f = async function* () {};
f().next().then(function() {
    f().next().then(function() {});
});

AddressSanitizer:DEADLYSIGNAL
=================================================================
==4156==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5614b98966a7 bp 0x7ffe7ddd3d70 sp 0x7ffe7ddd3d70 T0)
==4156==The signal is caused by a WRITE memory access.
==4156==Hint: address points to the zero page.
    #0 0x5614b98966a7 in mozilla::Span<JS::GCCellPtr const, 18446744073709551615ul>::operator[](unsigned long) const /home/skygentoo/shell-cache/js-64-asan-linux-x86_64-a3cd8f83fefa/objdir-js/dist/include/mozilla/Span.h:713:5
    #1 0x5614b98966a7 in js::BaseScript::enclosingScope() const /home/skygentoo/trees/mozilla-central/js/src/vm/JSScript.cpp:713:10
    #2 0x5614b994543e in JSFunction::enclosingScope() const /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:457:60
    #3 0x5614b994543e in js::frontend::CompilationInput::initFromLazy(js::BaseScript*) /home/skygentoo/trees/mozilla-central/js/src/frontend/CompilationInfo.h:203:40
    #4 0x5614b994543e in bool DelazifyCanonicalScriptedFunctionImpl<mozilla::Utf8Unit>(JSContext*, JS::Handle<JSFunction*>, JS::Handle<js::BaseScript*>, js::ScriptSource*) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1573:25
    #5 0x5614b9890473 in DelazifyCanonicalScriptedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1621:12
    #6 0x5614b9890473 in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1660:10
    #7 0x5614b9909445 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:410:12
    #8 0x5614b98900ef in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1648:24
    #9 0x5614b92d52c8 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:410:12
    #10 0x5614b92d52c8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:605:8
    #11 0x5614b92d89a1 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:647:10
    #12 0x5614b92d89a1 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:664:8
    #13 0x5614b971fd01 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.h:106:10
    #14 0x5614b971fd01 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) /home/skygentoo/trees/mozilla-central/js/src/builtin/Promise.cpp:1904:10
    #15 0x5614b92d5e84 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:503:13
    #16 0x5614b92d5e84 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:594:12
    #17 0x5614b92d89a1 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:647:10
    #18 0x5614b92d89a1 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:664:8
    #19 0x5614b9ebd32b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/jsapi.cpp:2861:10
    #20 0x5614b9879c82 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JSObject*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/jsapi.h:1472:10
    #21 0x5614b9879c82 in js::InternalJobQueue::runJobs(JSContext*) /home/skygentoo/trees/mozilla-central/js/src/vm/JSContext.cpp:777:14
    #22 0x5614b9878cde in js::RunJobs(JSContext*) /home/skygentoo/trees/mozilla-central/js/src/vm/JSContext.cpp:714:17
    #23 0x5614b90fb987 in RunShellJobs(JSContext*) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:1082:5
    #24 0x5614b90e9604 in Shell(JSContext*, js::cli::OptionParser*, char**) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:11136:3
    #25 0x5614b90debd2 in main /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:12012:12
    #26 0x7fc14b39ce39 in __libc_start_main (/lib64/libc.so.6+0x23e39)
    #27 0x5614b9013d99 in _start (/home/skygentoo/shell-cache/js-64-asan-linux-x86_64-a3cd8f83fefa/js-64-asan-linux-x86_64-a3cd8f83fefa+0x18bcd99)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/skygentoo/shell-cache/js-64-asan-linux-x86_64-a3cd8f83fefa/objdir-js/dist/include/mozilla/Span.h:713:5 in mozilla::Span<JS::GCCellPtr const, 18446744073709551615ul>::operator[](unsigned long) const
==4156==ABORTING

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/bad2b6719d73
user:        Ted Campbell
date:        Sun Jan 17 22:54:10 2021 +0000
summary:     Bug 1687174 - Avoid extra loops in Stencil instantiation for full parse. r=arai

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar sh ./configure --enable-address-sanitizer --disable-jemalloc --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests, tested on m-c rev a3cd8f83fefa.

Not sure if this is s-s, I'd leave it to Ted/arai.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1687174

Comment 2

[Tooru Fujisawa [:arai]]

Thanks!

This doesn't happen outside of JS shell.

The issue is that, we disallow lazy parsing (setDiscardSource) after lazily parsed the top-level script,
and the condition at https://searchfox.org/mozilla-central/rev/358fbca0398ac651f5ea6030be39b1870ec180a5/js/src/frontend/Stencil.cpp#762
depends on consistent RealmBehaviors::discardSource() value,
across top-level compilation and delazification.

Here's the list of RealmBehaviors::setDiscardSource callsites:
https://searchfox.org/mozilla-central/search?q=symbol:_ZN2JS14RealmBehaviors16setDiscardSourceEb&redirect=false

• DedicatedWorkerGlobalScope::WrapGlobalObject case
  • passed to JS_NewGlobalObject
• JSRuntime::createSelfHostingGlobal case
  • passed to NewRealm
• xpc::CreateSandboxObject case
  • passed to JS_NewGlobalObject
• xpc::InitGlobalObjectOptions case
  • passed to JS_NewGlobalObject
• XRE_XPCShellMain case (also testing only)
  • alters after creating global, but before parsing any script
• SetDiscardSource case (JS shell testing function)
  • alters anytime

So, only JS shell testing function can alter the flag after parsing script.

We could do:

• add debug assertion that CanLazilyParse is true for delazification,
• remove the testing functions (setDiscardSource and setLazyParsingDisabled) and instead add command-line parameter to flip those behaviors
• or, ignore those RealmBehavior options for delazifications

Comment 3

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #2)

• remove the testing functions (setDiscardSource and setLazyParsingDisabled) and instead add command-line parameter to flip those behaviors

or use newGlobal({ disableLazyParsing: true}) in those testcases, and add discardSource option too.

Comment 4

[Tooru Fujisawa [:arai]]

Plan:

• add discardSource option to newGlobal
• rewrite testcases to use newGlobal instead of setDiscardSource and setLazyParsingDisabled functions
• remove setDiscardSource and setLazyParsingDisabled
• make most of RealmBehaviors fields immutable after creating realm:
  • add public API for RealmBehaviors::setNonLive
  • make RealmBehaviorsRef return const reference
  • remove mutable variant of Realm::behaviors

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 1: Add discardSource option to newGlobal. r?tcampbell!

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 2: Use newGlobal with discardSource option in testcases. r?tcampbell!

Depends on D102757

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 3: Use newGlobal with disableLazyParsing option in testcases. r?tcampbell!

Depends on D102758

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 4: Remove setLazyParsingDisabled and setDiscardSource testing functions. r?tcampbell!

Depends on D102759

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 5: Add JS::SetRealmNonLive. r?tcampbell!

Depends on D102760

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1688190 - Part 6: Make JS::RealmBehaviorsRef return const reference. r?tcampbell!

Depends on D102761

Comment 11

[Ted Campbell [:tcampbell]]

Thanks for the report Gary! As :arai points out, this depends on discardSource which is only used in the jsshell and privilege chrome-js code.

The ScriptWarmupData and BaseScript cleanup last year has also been paying off by making these sorts of bugs into robust nullptr crashes instead.

@sectriage: This bug can be un-hidden.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22bf465ca004
Part 1: Add discardSource option to newGlobal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/0a62713c08b4
Part 2: Use newGlobal with discardSource option in testcases. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/ff856aa267d0
Part 3: Use newGlobal with disableLazyParsing option in testcases. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/f416c198c3e9
Part 4: Remove setLazyParsingDisabled and setDiscardSource testing functions. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/598768646a01
Part 5: Add JS::SetRealmNonLive. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/f2e017b239ac
Part 6: Make JS::RealmBehaviorsRef return const reference. r=tcampbell

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/22bf465ca004
https://hg.mozilla.org/mozilla-central/rev/0a62713c08b4
https://hg.mozilla.org/mozilla-central/rev/ff856aa267d0
https://hg.mozilla.org/mozilla-central/rev/f416c198c3e9
https://hg.mozilla.org/mozilla-central/rev/598768646a01
https://hg.mozilla.org/mozilla-central/rev/f2e017b239ac