1761608 - Crash [@ ModuleValidator<char16_t>::finish()] or Assertion failure: error == CompileArgsError::OutOfMemory, at wasm/AsmJS.cpp:2163

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20220325-2b624fdb002e (opt build, run with --fuzzing-safe --ion-offthread-compile=off --more-compartments --wasm-compiler=optimizing test.js):

a = newGlobal()
Debugger(a)
evaluate("function h() { 'use asm'; return {}}", {
    global: a
})

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055cb98f79b4f in ModuleValidator<char16_t>::finish() ()
#1  0x000055cb98f32427 in js::CompileAsmJS(JSContext*, js::frontend::ParserAtomsTable&, js::frontend::Parser<js::frontend::FullParseHandler, char16_t>&, js::frontend::ParseNode*, bool*) ()
#2  0x000055cb98e183c1 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::asmJS(js::frontend::ListNode*) ()
#3  0x000055cb996d2e75 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementList(js::frontend::YieldHandling) ()
#4  0x000055cb996de04b in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::FunctionBodyType) ()
#5  0x000055cb996dcf85 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionFormalParametersAndBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionNode**, js::frontend::FunctionSyntaxKind, mozilla::Maybe<unsigned int> const&, bool) ()
#6  0x000055cb996dc410 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::innerFunctionForFunctionBox(js::frontend::FunctionNode*, js::frontend::ParseContext*, js::frontend::FunctionBox*, js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::frontend::Directives*) ()
#7  0x000055cb996df435 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionDefinition(js::frontend::FunctionNode*, unsigned int, js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TaggedParserAtomIndex, js::frontend::FunctionSyntaxKind, js::GeneratorKind, js::FunctionAsyncKind, bool) ()
#8  0x000055cb996d475e in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionStmt(unsigned int, js::frontend::YieldHandling, js::frontend::DefaultHandling, js::FunctionAsyncKind) ()
#9  0x000055cb996d3997 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementListItem(js::frontend::YieldHandling, bool) ()
#10 0x000055cb996d29e4 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementList(js::frontend::YieldHandling) ()
#11 0x000055cb996f6510 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::globalBody(js::frontend::GlobalSharedContext*) ()
#12 0x000055cb9970f04c in ScriptCompiler<char16_t>::compile(JSContext*, js::frontend::SharedContext*) ()
#13 0x000055cb9970ebc3 in bool CompileGlobalScriptToStencilAndMaybeInstantiate<char16_t>(JSContext*, js::frontend::CompilationInput&, JS::SourceText<char16_t>&, js::ScopeKind, mozilla::Variant<mozilla::UniquePtr<js::frontend::ExtensibleCompilationStencil, JS::DeletePolicy<js::frontend::ExtensibleCompilationStencil> >, RefPtr<js::frontend::CompilationStencil>, js::frontend::CompilationGCOutput*>&) ()
#14 0x000055cb996f9918 in js::frontend::CompileGlobalScript(JSContext*, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&, js::ScopeKind) ()
#15 0x000055cb995991d1 in Evaluate(JSContext*, unsigned int, JS::Value*) ()
[...]
#22 0x000055cb9958b0e9 in main ()
rax	0x55cb99a8120a	94332944650762
rbx	0x7ffcd93b8890	140723953043600
rcx	0x55cb9ab48068	94332962242664
rdx	0x0	0
rsi	0x7fe73041e101	140630923796737
rdi	0x7fe73041e100	140630923796736
rbp	0x7ffcd93b9490	140723953046672
rsp	0x7ffcd93b8870	140723953043568
r8	0x7ffcd93b889c	140723953043612
r9	0x7fe731100fa8	140630937309096
r10	0x5	5
r11	0x0	0
r12	0x0	0
r13	0x0	0
r14	0x7ffcd93b9540	140723953046848
r15	0x0	0
rip	0x55cb98f79b4f <ModuleValidator<char16_t>::finish()+2287>
=> 0x55cb98f79b4f <_ZN15ModuleValidatorIDsE6finishEv+2287>:	movl   $0x873,0x0
   0x55cb98f79b5a <_ZN15ModuleValidatorIDsE6finishEv+2298>:	callq  0x55cb98c235f0 <abort>

This happens fairly frequently, marking as fuzzblocker.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220327213550-13eaa20fcca0.
The bug appears to have been introduced in the following build range:

Start: 12cd729f81852185af58bc36df4c93917a33c7dd (20220316191445)
End: 51d54dda3ba14b41a4f350590afdd7285e29f70b (20220316195508)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=12cd729f81852185af58bc36df4c93917a33c7dd&tochange=51d54dda3ba14b41a4f350590afdd7285e29f70b

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

This is odd. We intend for CheckPreconditions to abort such that CompileArgs::build is infallible (outside of OOMs), but this is not happening.

1. wasm::CompileArgs::build will fail if ion is selected && debugging is observing wasm
2. AsmJS.cpp: CheckPreconditions will abort compilation if CompileOptions::asmJSOption is not enabled
3. CompileOptions::asmJSOption will not be enabled if debugging is observing asm JS
4. js.cpp: Evaluate builtin will compile using the CompileOptions using the realm of its caller [1], not the supplied global (if there is any)

This means that if the caller realm is not being debugged, but the new script's global is, the AsmJS compilation will get to the state where observingWasm = true, observingAsmJS = false. This causes CheckPreconditions to not protect us.

Two questions:

Yury: What is the difference between asmJS debugging observed and wasm debugging observed?

(picking a best guess) Matthew: Why do we compile using the caller's realms CompileOptions in js shell Evaluate, and not the global's CompileOptions? If there's a reason, does it make sense to propagate a flag like asmJSOption from the global's options to the caller's options?

e.g.

       // Validate the consistency between the passed CompileOptions and
       // global's option.
       {
         JSAutoRealm ar(cx, global);
         JS::CompileOptions globalOptions(cx);
 
         // If the passed global discards source, delazification isn't allowed.
         // The CompileOption should discard source as well.
         if (globalOptions.discardSource && !options.discardSource) {
           JS_ReportErrorASCII(cx, "discardSource option mismatch");
           return false;
         }
+        options.asmJSOption = globalOptions.asmJSOption;
       }
     }

[1] https://searchfox.org/mozilla-central/rev/66b43502bdbe4b62cbc8b0c8ff10efc5707352e4/js/src/shell/js.cpp#2209
[2] https://searchfox.org/mozilla-central/rev/66b43502bdbe4b62cbc8b0c8ff10efc5707352e4/js/src/shell/js.cpp#2342

Comment 5

[Yury Delendik (:yury)]

What is the difference between asmJS debugging observed and wasm debugging observed?

In asmJS case, if debugging is enabled/observed, the source code is treated as regular JS. In wasm case, wasm JIT is using baseline compiler in debug mode. Before we took shortcut and reused the former to be used for wasm debugging, that was wrong and now it is a good time to disconnect these two.

Comment 6

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

:rhunt, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 7

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

(picking a best guess) Matthew: Why do we compile using the caller's realms CompileOptions in js shell Evaluate, and not the global's CompileOptions? If there's a reason, does it make sense to propagate a flag like asmJSOption from the global's options to the caller's options?

I'd actually redirect this to Arai; my guess looking at the code is that this all made sense and then the global option was retrofit on later. I think synchronizing the asmJS flag makes sense too.

Comment 8

[Tooru Fujisawa [:arai]]

To my understanding, not much specific reason other than using the current global's option is reasonable in the code's order.
if we use provided global's option, it needs special branch to inherit the provided global's option instead of the current global's option, around CompileOptions allocation.

I'll check if using global property's option works.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1761608 - Use the given global's default compile option in evaluate shell function. r?mgaudet!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/316cad4401b7
Use the given global's default compile option in evaluate shell function. r=mgaudet

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/316cad4401b7

Comment 12

[Ryan Hunt [:rhunt] (on leave until early May)]

Thanks for fixing this!

Comment 13

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

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

Comment 14

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220331093541-260e22f03e98.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.