1694916 - 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, P4)

Description

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

evaluate("function f() { (function(){})(); }; f();", {
    global: newGlobal({
        disableLazyParsing: true,
    }),
});

AddressSanitizer:DEADLYSIGNAL
=================================================================
==30707==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5564709f4cc7 bp 0x7ffd1721ef30 sp 0x7ffd1721ef30 T0)
==30707==The signal is caused by a WRITE memory access.
==30707==Hint: address points to the zero page.
    #0 0x5564709f4cc7 in mozilla::Span<JS::GCCellPtr const, 18446744073709551615ul>::operator[](unsigned long) const /home/skygentoo/shell-cache/js-64-asan-linux-x86_64-8708c121e21c/objdir-js/dist/include/mozilla/Span.h:713:5
    #1 0x5564709f4cc7 in js::BaseScript::enclosingScope() const /home/skygentoo/trees/mozilla-central/js/src/vm/JSScript.cpp:713:10
    #2 0x556470a9e394 in JSFunction::enclosingScope() const /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:454:60
    #3 0x556470a9e394 in js::frontend::CompilationInput::initFromLazy(js::BaseScript*, js::ScriptSource*) /home/skygentoo/trees/mozilla-central/js/src/frontend/CompilationStencil.h:293:40
    #4 0x556470a9e394 in bool DelazifyCanonicalScriptedFunctionImpl<char16_t>(JSContext*, JS::Handle<JSFunction*>, JS::Handle<js::BaseScript*>, js::ScriptSource*) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1548:17
    #5 0x5564709eebfe in DelazifyCanonicalScriptedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1576:10
    #6 0x5564709eebfe in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1609:10
    #7 0x556470a66625 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:407:12
    #8 0x5564709ee71f in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.cpp:1597:24
    #9 0x55647041cf85 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) /home/skygentoo/trees/mozilla-central/js/src/vm/JSFunction.h:407:12
    #10 0x55647041cf85 in Interpret(JSContext*, js::RunState&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:3256:27
    #11 0x55647040021a in js::RunScript(JSContext*, js::RunState&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:405:13
    #12 0x556470439033 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:772:13
    #13 0x55647073674a in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/vm/CompilationAndEvaluation.cpp:461:10
    #14 0x55647025ef23 in Evaluate(JSContext*, unsigned int, JS::Value*) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:2496:19
    #15 0x556470432bb6 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:435:13
    #16 0x556470432bb6 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:520:12
    #17 0x55647041cb35 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:580:10
    #18 0x55647041cb35 in js::CallFromStack(JSContext*, JS::CallArgs const&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:584:10
    #19 0x55647041cb35 in Interpret(JSContext*, js::RunState&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:3243:16
    #20 0x55647040021a in js::RunScript(JSContext*, js::RunState&) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:405:13
    #21 0x556470439033 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:772:13
    #22 0x556470736fde in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) /home/skygentoo/trees/mozilla-central/js/src/vm/CompilationAndEvaluation.cpp:468:10
    #23 0x5564702a45c4 in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:989:10
    #24 0x5564702a2fb5 in Process(JSContext*, char const*, bool, FileKind) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:1580:14
    #25 0x55647024ae5c in ProcessArgs(JSContext*, js::cli::OptionParser*) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:10515:10
    #26 0x55647024ae5c in Shell(JSContext*, js::cli::OptionParser*, char**) /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:11250:10
    #27 0x55647023ce62 in main /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:12137:12
    #28 0x7fba07b5be39 in __libc_start_main (/lib64/libc.so.6+0x23e39)
    #29 0x5564701754d9 in _start (/home/skygentoo/shell-cache/js-64-asan-linux-x86_64-8708c121e21c/js-64-asan-linux-x86_64-8708c121e21c+0x178a4d9)

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

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

Comment 1

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

Attachment: debug stack

Comment 2

[Tooru Fujisawa [:arai]]

Thank you!

this is a mismatch between the compile option and global option.
evaluate's compile option around lazy/full parse is controlled by forceFullParse property of the options parameter, that defaults to false here,
and global's option is set by disableLazyParsing property of the options parameter of newGlobal.

then, when compiling a code inside evaluate function, CompileOptions is created before entering the global,
so the passed global's disableLazyParsing behavior isn't reflected to forceFullParse_,
and even if we enter, passing forceFullParse can override the option.

So, we should check consistency between CompileOptions and passed global's behavior, before executing the compiled code, or maybe before compiling, inside evaluate function.

this issue is JS shell-only, and doesn't affect browsers.

• execute is JS shell-only function
• forceFullParse is used only for self-hosting and browser chrome code, so this cannot be exploited by web content
• this is caused only when disableLazyParsing is set to true, but disableLazyParsing isn't modified outside of JS shell (maybe we should remove disableLazyParsing from RealmBehaviors)

Comment 3

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

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

Comment 4

[Andrew McCreight [:mccr8]]

shell-only, so I'm unhiding

Comment 5

[Tooru Fujisawa [:arai]]

plan:

• remove RealmBehaviors.disableLazyParsing
• add consistency check between CompileOption and Realm/RealmBehavior, in evaluate
  • CompileOptions.discardSource vs RealmBehaviors.discardSource
  • CompileOptions.debuggerObservesAsmJS vs Realm::debuggerObservesAsmJS
  • instrumentationKinds vs RealmInstrumentation::getInstrumentationKinds

Comment 6

[Tooru Fujisawa [:arai]]

bug 1689483 is also about consistency between compilation (and decode) and global's option.

:mgaudet, in the bug's case, the caller is responsible for passing consistent the CompileOptions for encode, decode, that matches target global's option/behavior, right?

if that's the case, JS shell is also responsible for the same, and adding the consistency check inside evaluate should make sense.

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Yeah, evaluate needs consistency checking. I have https://phabricator.services.mozilla.com/D103521 waiting for Ted to look at (attached to Bug 1689403 )

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

I originally misread Arai's diagnosis of this. The realm behaviour part in particular didn't quite twig.

So re-testing today, D103521 does not prevent this. I think Arai's plan in Comment 5 makes sense.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1694916 - Part 1: Remove RealmBehaviors.disableLazyParsing. r=mgaudet

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1694916 - Part 2: Check the consistency of discardSource/instrumentationKinds in evaluate JS shell function. r=mgaudet

asmJSOption can also be different between given CompileOptions and given global,
but asmJSOption doesn't affect after compilation, so it's not checked.

Depends on D107006

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a6ad4bb8645
Part 1: Remove RealmBehaviors.disableLazyParsing. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/3fe9243aefea
Part 2: Check the consistency of discardSource/instrumentationKinds in evaluate JS shell function. r=mgaudet

Comment 12

[Cristian Brindusan [:cbrindusan]]

Backed out 2 changesets (bug 1694916) for sm bustages in js.cpp.
https://hg.mozilla.org/integration/autoland/rev/ffc0ca350122b5265ef2fc9e0ca1c29d02dbad80

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=3fe9243aefea15c9341b01c77a65c2fa212a0137&selectedTaskRun=VW0HwzbXRza2Q14Hc77GiQ.0

Failure log:
https://treeherder.mozilla.org/logviewer?job_id=332090021&repo=autoland&lineNumber=3146

Comment 13

[Tooru Fujisawa [:arai]]

patch conflict is unexpectedly resolved.
will rebase and land again.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d56b4f5af4ea
Part 1: Remove RealmBehaviors.disableLazyParsing. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/cd8a6b1a2caa
Part 2: Check the consistency of discardSource/instrumentationKinds in evaluate JS shell function. r=mgaudet

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d56b4f5af4ea
https://hg.mozilla.org/mozilla-central/rev/cd8a6b1a2caa