1731435 - Assertion failure: mLength + aIncr <= reserved(), at dist/include/mozilla/Vector.h:1145 with WebAssembly function references

Status: RESOLVED FIXED

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

Description

[Christian Holler (:decoder)]

The attached testcase crashes on mozilla-central revision 20210917-51f797b813fd (build with --enable-debug --enable-fuzzing, run with --no-threads --fuzzing-safe --baseline-warmup-threshold=0 --disable-oom-functions --wasm-function-references test.js).

Backtrace:

    Assertion failure: mLength + aIncr <= reserved(), at mozilla/Vector.h:1145
    UndefinedBehaviorSanitizer:DEADLYSIGNAL
    ==2637==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x563c7b674802 bp 0x7ffda9dcaa70 sp 0x7ffda9dcaa40 T2637)
    ==2637==The signal is caused by a WRITE memory access.
    ==2637==Hint: address points to the zero page.
        #0 0x563c7b674802 in infallibleGrowByUninitialized dist/include/mozilla/Vector.h:1145:3
        #1 0x563c7b674802 in infallibleEmplaceBack<js::wasm::StackType &> dist/include/mozilla/Vector.h:706:5
        #2 0x563c7b674802 in js::wasm::OpIter<js::wasm::BaseCompilePolicy>::infalliblePush(js::wasm::StackType) /js/src/wasm/WasmOpIter.h:406:50
        #3 0x563c7b5a00ca in js::wasm::OpIter<js::wasm::BaseCompilePolicy>::readBrOnNull(unsigned int*, js::wasm::ResultType*, js::wasm::BaseNothingVector*, mozilla::Nothing*) /js/src/wasm/WasmOpIter.h:2271:5
        #4 0x563c7b59fcde in js::wasm::BaseCompiler::emitBrOnNull() /js/src/wasm/WasmBaselineCompile.cpp:3383:14
        #5 0x563c7b5cf564 in js::wasm::BaseCompiler::emitBody() /js/src/wasm/WasmBaselineCompile.cpp:8441:9
        #6 0x563c7b5ffe9a in emitFunction /js/src/wasm/WasmBaselineCompile.cpp:9527:8
        #7 0x563c7b5ffe9a in js::wasm::BaselineCompileFunctions(js::wasm::ModuleEnvironment const&, js::wasm::CompilerEnvironment 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:9695:12
        #8 0x563c7b6d83a7 in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) /js/src/wasm/WasmGenerator.cpp:732:12
        #9 0x563c7b6da1bb in locallyCompileCurrentTask /js/src/wasm/WasmGenerator.cpp:784:8
        #10 0x563c7b6da1bb in js::wasm::ModuleGenerator::finishFuncDefs() /js/src/wasm/WasmGenerator.cpp:924:24
        #11 0x563c7b6a0244 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) /js/src/wasm/WasmCompile.cpp:681:13
        #12 0x563c7b69fbce 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:703:8
        #13 0x563c7b740371 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) /js/src/wasm/WasmJS.cpp:1765:7
        #14 0x563c7a393de0 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:401:13
        #15 0x563c7a3a2884 in CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /js/src/vm/Interpreter.cpp:417:8
        #16 0x563c7a39528c in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /js/src/vm/Interpreter.cpp:593:14
        #17 0x563c7ae8a02c in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/jit/BaselineIC.cpp:1567:10
        #18 0x148954d527a2  (<unknown module>)

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Lars T Hansen [:lth]]

Looks like this is an error that's local to readBrOnNull. It incorrectly assumes that, since it popped a value, it can use infalliblePush. But this is not so: the poorly-named checkBranchValue can push values (that's what its third argument is for), requiring the subsequent push to be fallible.

No other code seems to be confused about this. It's sec-high I guess based only on the behavior (scribble on memory) but this is Nightly-only code that's behind a flag that's off by default, so a fix should not be blocked by anything.

Comment 4

[Lars T Hansen [:lth]]

Attachment: Bug 1731435 - Fix. r?rhunt

Comment 5

[Lars T Hansen [:lth]]

In the patch, I took the opportunity to change some method names to clarify that the methods are pushing values.

The problem with the infallible push was fixed by introducing a variant of push() that takes a StackType, as this corresponds well with the old code, but I don't know if that's a desirable thing to have. I'm no longer very familiar with this part of the code.

Comment 6

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

Fix. r=rhunt
https://hg.mozilla.org/integration/autoland/rev/9013de5043168b8e1b47e63495cd899fb4053fb2
https://hg.mozilla.org/mozilla-central/rev/9013de504316

Comment 7

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 8

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Unable to reproduce bug 1731435 using build mozilla-central 20210917093507-51f797b813fd. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.