Assertion failure: isAvailable(r), at js/src/wasm/WasmBaselineCompile.cpp:692

RESOLVED FIXED in Firefox 53

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: decoder, Assigned: lth)

Tracking

(Blocks 1 bug, {assertion, regression, testcase})

Trunk
mozilla53
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

(Whiteboard: [fuzzblocker])

Attachments

(3 attachments)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision ee8e6503d47c+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2 --without-intl-api --enable-debug). To reproduce, you can run the following code in the JS shell (running with --wasm-always-baseline might be necessary):

var data = os.file.readFile(file, 'binary');
new WebAssembly.Instance(new WebAssembly.Module(data.buffer));



Backtrace:

==4956==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000044d634b bp 0x7ffe8cf5ebe0 sp 0x7ffe8cf5ebb0 T0)
    #0 0x44d634a in js::jit::SpecializedRegSet<js::jit::AllocatableSetAccessors<js::jit::TypedRegisterSet<js::jit::Register> >, js::jit::TypedRegisterSet<js::jit::Register> >::take(js::jit::Register) js/src/jit/RegisterSets.h:750:9
    #1 0x44d634a in js::wasm::BaseCompiler::allocGPR(js::jit::Register) js/src/wasm/WasmBaselineCompile.cpp:693
    #2 0x44ae0e0 in js::wasm::BaseCompiler::captureJoinRegUnlessVoid(js::wasm::ExprType) js/src/wasm/WasmBaselineCompile.cpp:1712:13
    #3 0x446b7fa in js::wasm::BaseCompiler::endBlock(js::wasm::ExprType) js/src/wasm/WasmBaselineCompile.cpp:4834:17
    #4 0x446ea77 in js::wasm::BaseCompiler::emitEnd() js/src/wasm/WasmBaselineCompile.cpp:5043:30
    #5 0x448a1c5 in js::wasm::BaseCompiler::emitBody() js/src/wasm/WasmBaselineCompile.cpp:6493:13
    #6 0x449d858 in js::wasm::BaseCompiler::emitFunction() js/src/wasm/WasmBaselineCompile.cpp:7106:10
    #7 0x44a2b38 in js::wasm::BaselineCompileFunction(js::wasm::IonCompileTask*) js/src/wasm/WasmBaselineCompile.cpp:7363:10
    #8 0xdb7d2b in js::wasm::CompileFunction(js::wasm::IonCompileTask*) js/src/wasm/WasmIonCompile.cpp:3793:16
    #9 0xcc5ec3 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, js::wasm::FunctionGenerator*) js/src/wasm/WasmGenerator.cpp:917:14
    #10 0xc73d3a in DecodeFunctionBody(js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/wasm/WasmCompile.cpp:60:12
    #11 0xc73d3a in DecodeCodeSection(js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:89
    #12 0xc73d3a in js::wasm::Compile(js::wasm::ShareableBytes const&, js::wasm::CompileArgs const&, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmCompile.cpp:190
    #13 0xe2d1b9 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/wasm/WasmJS.cpp:388:27
    #14 0x605810 in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5656:14
[...]

Marking as fuzzblocker because this happens with high frequency.
Posted file Testcase
Assignee: nobody → lhansen
Posted file Minimized test case
Patch + test case.

The bug was likely introduced by bug 1316850, which rearranged the value popping machinery to match an earlier state.

Along an unreachable edge the exit code for Block and If will capture the value in the join register that must have come along another, taken edge, but this code depends on the join register being free at that point.  Since we sync() before a block the join register will be free if we have restored the stack state to its pre-block state, *but* that depends on the compiler's value stack being popped before we attempt to capture the join register.  The bug is that the patch in bug 1316850 put the popping code next to popControl, not next to the popStack code where it truly belongs.
Attachment #8813241 - Flags: review?(bbouvier)
Comment on attachment 8813241 [details] [diff] [review]
bug1319415-pop-then-capture.patch

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

Looks good to me, thanks!

It now seems that popValueStackTo and popStackOnBlockExit are used next to each other all the time, and popStackOnBlockExit always has its sibling popValueStackTo call close. Could we refactor this, and make a helper exitBlock(Control&) (or something like this) that would call popValueStackTo and contain the body of popStackOnBlockExit? That would reduce API surface and make it harder to make mistakes. (popControl() could almost go into exitBlock too, if it were not for emitElse...)

::: js/src/jit-test/tests/wasm/regress/baseline-pop-before-capture.js
@@ +1,1 @@
> +var src =

Can you either load(libdir + 'wasm.js') or guard against the existence of the global WebAssembly object?

@@ +8,5 @@
> +  (export "" 0))`;
> +
> +var bin = wasmTextToBinary(src);
> +
> +new WebAssembly.Module(bin);

Can you also instanciate it, call the unique export and check the return value? (bonus points if you use wasmFullPass, which does the binary -> text -> binary path too)
Attachment #8813241 - Flags: review?(bbouvier) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #4)
> Comment on attachment 8813241 [details] [diff] [review]
> bug1319415-pop-then-capture.patch
> 
> Review of attachment 8813241 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, thanks!
> 
> It now seems that popValueStackTo and popStackOnBlockExit are used next to
> each other all the time, and popStackOnBlockExit always has its sibling
> popValueStackTo call close. Could we refactor this, 

Yes!  However, I expect them to move apart again (at least this is fairly likely), see comments on bug 1318654, which is why I left it alone here.

> Can you either load(libdir + 'wasm.js') or guard against the existence of the global WebAssembly object?
> 
> Can you also instanciate it, call the unique export and check the return value? 

Will do.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9e3af2b0e9913d87e031ec993bd25a41d82cafb
Bug 1319415 - pop value stack on block end before attempting to capture join register. r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/f9e3af2b0e99
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.