Closed Bug 1281131 Opened 8 years ago Closed 8 years ago

[wasm] Assertion failure: mLength + aIncr <= reserved(), at dist/include/mozilla/Vector.h:1098

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: decoder, Assigned: lth)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision e9723c6c6136+ (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');
Wasm.instantiateModule(new Uint8Array(data.buffer));



Backtrace:

==18464==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000002a0fb0d bp 0x7ffdb80e9310 sp 0x7ffdb80e9260 T0)
    #0 0x2a0fb0c in mozilla::Vector<js::wasm::BaseCompiler::Stk, 8ul, js::SystemAllocPolicy>::infallibleGrowByUninitialized(unsigned long) dist/include/mozilla/Vector.h:1098:3
    #1 0x2a0fb0c in void mozilla::Vector<js::wasm::BaseCompiler::Stk, 8ul, js::SystemAllocPolicy>::infallibleEmplaceBack<js::wasm::BaseCompiler::Stk>(js::wasm::BaseCompiler::Stk&&) dist/include/mozilla/Vector.h:656
    #2 0x2a0fb0c in js::wasm::BaseCompiler::push() js/src/asmjs/WasmBaselineCompile.cpp:710
    #3 0x29d26fa in js::wasm::BaseCompiler::pushVoid() js/src/asmjs/WasmBaselineCompile.cpp:1197:9
    #4 0x29d26fa in js::wasm::BaseCompiler::pushControl(mozilla::UniquePtr<js::wasm::BaseCompiler::PooledLabel, js::wasm::BaseCompiler::UniquePooledLabelFreePolicy>*, mozilla::UniquePtr<js::wasm::BaseCompiler::PooledLabel, js::wasm::BaseCompiler::UniquePooledLabelFreePolicy>*) js/src/asmjs/WasmBaselineCompile.cpp:1612
    #5 0x29d26fa in js::wasm::BaseCompiler::emitLoop() js/src/asmjs/WasmBaselineCompile.cpp:4405
    #6 0x29e7bf7 in js::wasm::BaseCompiler::emitBody() js/src/asmjs/WasmBaselineCompile.cpp:5688:13
    #7 0x29f0745 in js::wasm::BaseCompiler::emitFunction() js/src/asmjs/WasmBaselineCompile.cpp:6215:10
    #8 0x29f47a2 in js::wasm::BaselineCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmBaselineCompile.cpp:6482:10
    #9 0x72c845 in js::wasm::CompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3477:16
    #10 0x6afca0 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:824:14
    #11 0x645e3d in DecodeFunctionBody(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/Wasm.cpp:940:12
    #12 0x645e3d in DecodeCodeSection(JSContext*, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/Wasm.cpp:968
    #13 0x645e3d in DecodeModule(JSContext*, mozilla::UniquePtr<char [], JS::FreePolicy>, js::wasm::ShareableBytes const&, JS::MutableHandle<js::ArrayBufferObject*>) js/src/asmjs/Wasm.cpp:1141
    #14 0x63bc72 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/asmjs/Wasm.cpp:1249:27
    #15 0x59a42e in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5226:14
[...]
    #29 0x461088 in _start (/home/ubuntu/build/build/js+0x461088)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV dist/include/mozilla/Vector.h:1098:3 in mozilla::Vector<js::wasm::BaseCompiler::Stk, 8ul, js::SystemAllocPolicy>::infallibleGrowByUninitialized(unsigned long)
==18464==ABORTING
Attached file Testcase
/me wipes egg from face.
Assignee: nobody → lhansen
Specifically, a loop pushes two control items, each control item pushes a void value.  But the optimization in emitBody that checks for stack capacity every 50 iterations checks only that there's space for 64 items.  Hence any sufficiently long sequence of LOOP nodes can overflow the value stack.
Fix + test case.
Attachment #8764228 - Flags: review?(bbouvier)
Comment on attachment 8764228 [details] [diff] [review]
bug1281131-reserve-enough.patch

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

/me looking for a pun based on pushy, stops.

Thanks for the push.

::: js/src/asmjs/WasmBaselineCompile.cpp
@@ +5675,5 @@
>              CHECK(alloc_.ensureBallast());
> +
> +            // The pushiest opcode is LOOP, which pushes two values
> +            // per instance.
> +            CHECK(stk_.reserve(stk_.length() + overhead*2));

nit: spaces before and after the *
Attachment #8764228 - Flags: review?(bbouvier) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e0f025cbc9a30747fbcaca41d68d93569b1216a
Bug 1281131 - make sure to reserve enough stack space for the worst case. r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/1e0f025cbc9a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: