1280938 - [wasm] Assertion failure: !imm8.invalid, at js/src/jit/arm/Assembler-arm.cpp:3322

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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 --target=i686-pc-linux-gnu --enable-simulator=arm). To reproduce, you can run the following code in the JS shell:

var data = os.file.readFile(file, 'binary');
Wasm.instantiateModule(new Uint8Array(data.buffer));



Backtrace:

==17437==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x096c7bf5 bp 0xff9b8be8 sp 0xff9b8b20 T0)
    #0 0x96c7bf4 in js::jit::Assembler::UpdateBoundsCheck(unsigned char*, unsigned int) js/src/jit/arm/Assembler-arm.cpp:3319:5
    #1 0xb57ff21 in SpecializeToHeap(js::wasm::CodeSegment&, js::wasm::Metadata const&, unsigned char*, unsigned int) js/src/asmjs/WasmCode.cpp:112:9
    #2 0xb57ff21 in js::wasm::CodeSegment::create(JSContext*, mozilla::Vector<unsigned char, 0u, js::SystemAllocPolicy> const&, js::wasm::LinkData const&, js::wasm::Metadata const&, unsigned char*, unsigned int) js/src/asmjs/WasmCode.cpp:224
    #3 0x845c034 in js::wasm::Module::instantiate(JSContext*, JS::Handle<JS::GCVector<JSFunction*, 0u, js::TempAllocPolicy> >, JS::Handle<js::ArrayBufferObjectMaybeShared*>, JS::MutableHandle<js::WasmInstanceObject*>) const js/src/asmjs/WasmModule.cpp:329:15
    #4 0x82e62f0 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/asmjs/Wasm.cpp:1260:12
    #5 0xb5c1be3 in InstantiateModule(JSContext*, unsigned int, JS::Value*) js/src/asmjs/WasmJS.cpp:52:10
    #6 0xa504d28 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235:15
[...]
    #22 0x80ab1e1 in _start (/home/decoder/LangFuzz/work/remote/builds/debugarmsim-wasm/build/js+0x80ab1e1)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV js/src/jit/arm/Assembler-arm.cpp:3319:5 in js::jit::Assembler::UpdateBoundsCheck(unsigned char*, unsigned int)
==17437==ABORTING

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Lars T Hansen [:lth]]

Generic linking error (not a baseline compiler issue).  Able to repro on arm simulator on Mac.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Memory size is 65532 pages, so 1073676288 bytes, and this value can't be encoded in an ARM immediate compare instruction (it doesn't the precondition in Imm8 ctor, being that it must have "a maximum of 8 contiguous set bits, with an optional wrapped left rotation to even bit positions"). Or quoting AsmJS.cpp:

// From the asm.js spec Linking section:
//  the heap object's byteLength must be either
//    2^n for n in [12, 24)
//  or
//    2^24 * n for n >= 1.

wasm doesn't have this precondition, as far as I can tell.

Comment 4

[Luke Wagner [:luke]]

Ah, right.  So I think the fix here I think is:
 - if we want to keep using immediates, to round up the length to the next representable immediate, allocate that much, and use PROT_NONE pages (and signal handling) to catch access in this slop region
 - but I think we want to avoid immediates (and patching in general), loading them into registers from TLS, which would also avoid this whole limitation.

In the short-term, though, perhaps we could simply throw an error during validation when given a non-multiple-of-16mb initial size?

Comment 5

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

Luke/Benjamin, what's next for this wasm bug?

Comment 6

[Luke Wagner [:luke]]

Does it still reproduce?  I would've thought this was fixed a while ago by the guard page bug.

Comment 7

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

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/d21d1aa42342
user:        Benjamin Bouvier
date:        Thu Jun 30 14:41:20 2016 +0200
summary:     Bug 1268024: Don't allow the memory's max size to be lower than the initial size; r=luke

Benjamin/Luke, is bug 1268024 a likely fix?

Comment 8

[Luke Wagner [:luke]]

Yeah, it makes sense that this has been fixed.

Comment 9

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

Resolving FIXED by bug 1268024.