1597200 - Assertion failure: srcOffset >= bytes, at js/src/wasm/WasmBaselineCompile.cpp:1810

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Christian Holler (:decoder)]

The attached testcase crashes on mozilla-central revision 72c52c0101cf (build with --enable-valgrind --enable-gczeal --enable-tests --enable-fuzzing --enable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with --no-threads --wasm-compiler=baseline test.js).

Backtrace:

==28810==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55d11cb618c0 bp 0x7ffe3bc0f370 sp 0x7ffe3bc0f1e0 T0)
==28810==The signal is caused by a WRITE memory access.
==28810==Hint: address points to the zero page.
    #0 0x55d11cb618bf in js::wasm::BaseStackFrame::shuffleStackResultsTowardFP(unsigned int, unsigned int, unsigned int, js::jit::Register) js/src/wasm/WasmBaselineCompile.cpp:1809:5
    #1 0x55d11cb5d6b9 in js::wasm::BaseCompiler::popStackResults(js::wasm::ABIResultIter&, js::wasm::StackHeight) js/src/wasm/WasmBaselineCompile.cpp:4074:12
    #2 0x55d11ca34409 in js::wasm::BaseCompiler::popBlockResults(js::wasm::ResultType, js::wasm::StackHeight, js::wasm::BaseCompiler::ContinuationKind) js/src/wasm/WasmBaselineCompile.cpp:4161:9
    #3 0x55d11ca3d333 in js::wasm::BaseCompiler::emitBr() js/src/wasm/WasmBaselineCompile.cpp:8848:3
    #4 0x55d11ca8a4b0 in js::wasm::BaseCompiler::emitBody() js/src/wasm/WasmBaselineCompile.cpp:11389:9
    #5 0x55d11caf9f03 in js::wasm::BaseCompiler::emitFunction() js/src/wasm/WasmBaselineCompile.cpp:12278:8
    #6 0x55d11caf9f03 in js::wasm::BaselineCompileFunctions(js::wasm::ModuleEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmBaselineCompile.cpp:12443
    #7 0x55d11cce853e in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmGenerator.cpp:743:12
    #8 0x55d11ccec124 in js::wasm::ModuleGenerator::locallyCompileCurrentTask() js/src/wasm/WasmGenerator.cpp:776:8
    #9 0x55d11ccec124 in js::wasm::ModuleGenerator::finishFuncDefs() js/src/wasm/WasmGenerator.cpp:914
    #10 0x55d11cb37922 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:570:13
    #11 0x55d11cb368db in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*, JS::OptimizedEncodingListener*) js/src/wasm/WasmCompile.cpp:593:8
    #12 0x55d11ce29eb8 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) js/src/wasm/WasmJS.cpp:1180:7
    [...]

This is similar to bug 1595466 but seems to reproduce even after the fix for that.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Christian Holler (:decoder)]

The rev in comment 0 is older because I forgot to update the build on the triage machine, but I just also reproduced this on d6844fe546ad.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Maybe another fallout from multi-value?

Comment 4

[Andy Wingo [:wingo]]

Thanks, will look.

Comment 5

[Andy Wingo [:wingo]]

Amusingly, this needed a fix to wabt to decode: https://github.com/WebAssembly/wabt/pull/1231

Comment 6

[Andy Wingo [:wingo]]

For the record:

$ ~/src/wabt/out/gcc/Debug/wasm2wat --enable-threads --enable-sign-extension --enable-multi-value --no-check test.wasm
(module
  (type (;0;) (func (param i32 i32 i32) (result i32)))
  (func $main (type 0) (param i32 i32 i32) (result i32)
    local.get 0
    i32.const -63
    br_if 0 (;@0;)
    loop (result i32)  ;; label = @1
      local.get 0
      i32.eqz
      i32.eqz
      i32.const -8
      i32.popcnt
      i32.div_s
      i32.const -23
      i32.div_s
      i32.const 59
      i32.const -2
      i32.popcnt
      i32.popcnt
      loop (param i32 i32 i32) (result i32)  ;; label = @2
        atomic.fence
        atomic.fence
        atomic.fence
        atomic.fence
        i32.const -23
        i32.div_s
        i32.const -63
        i32.eqz
        i32.eqz
        br 0 (;@2;)
      end
      unreachable
      return
      unreachable
      atomic.fence
      i32.const -8
      i32.popcnt
      i32.div_s
      i32.const 0
      unreachable
      i32.popcnt
      return
      i32.extend8_s
      unreachable
      i32.popcnt
      atomic.fence
      unreachable
    end)
  (export "main" (func $main)))

Comment 7

[Andy Wingo [:wingo]]

Minimal repro:

new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
(module
  (func (export "main") (result i32)
    (i32.const 1)
    (i32.const 2)
    (i32.const 3)
    (loop (param i32 i32 i32)
      (i32.popcnt)
      (i32.const -63)
      (br 0))
    (unreachable)))`)));

Comment 8

[Andy Wingo [:wingo]]

Attachment: Bug 1597200 - Fix shuffling of WebAssembly stack results toward FP

In a sadly similar bogosity as in bug 1595466, the multi-value stack
value shuffling code was confused about "stack height" (distance from
FP), "stack offset" (distance from SP), and whether the bytes to be
copied were towards the FP or the SP.

Comment 9

[Andy Wingo [:wingo]]

I am a bit speechless that this code had bugs that were this obvious :/ I will try to produce a test case that can be called.

Comment 10

[Lars T Hansen [:lth]]

sec-moderate because there might be some risk here of exposing heap addresses if we can confuse a pointer with an integer.

Comment 11

[Andy Wingo [:wingo]]

For the record, this bug is a shell-only Nightly bug -- i.e., not the browser.

Comment 12

[Lars T Hansen [:lth]]

(In reply to Andy Wingo [:wingo] from comment #11)

For the record, this bug is a shell-only Nightly bug -- i.e., not the browser.

Yeah. See https://bugzilla.mozilla.org/show_bug.cgi?id=1595466#c12 et seq for reasons for sec-moderate. But this is fine to just land, for sure.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/e12f0cfd62926fd3bee18a6d3f1983776804545a

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e12f0cfd6292