1567040 - LeakSanitizer: [@ js::wasm::StackMap::create]

Status: RESOLVED FIXED

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

Description

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

The following testcase crashes on mozilla-central revision b6d154b23098 (build with --enable-debug --enable-address-sanitizer, run with --fuzzing-safe --no-threads --no-baseline --no-ion --wasm-compiler=baseline and the environment variables ASAN_OPTIONS=detect_leaks=1 LSAN_OPTIONS=max_leaks=1):

// Adapted from randomly chosen test: js/src/jit-test/tests/wasm/regress/oom-masm-baseline.js
oomTest(function() {
    return new WebAssembly.Module(wasmTextToBinary("(module (func i32.const 0))"));
});

Backtrace:

Direct leak of 12 byte(s) in 1 object(s) allocated from:
    #0 0x55ead0c059f3 in __interceptor_malloc (/home/ubuntu/shell-cache/js-dbg-64-asan-linux-x86_64-b6d154b23098/js-dbg-64-asan-linux-x86_64-b6d154b23098+0x262c9f3)
    #1 0x55ead3406155 in js_arena_malloc(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-dbg-64-asan-linux-x86_64-b6d154b23098/objdir-js/dist/include/js/Utility.h:392:10
    #2 0x55ead3406155 in js_malloc(unsigned long) /home/ubuntu/shell-cache/js-dbg-64-asan-linux-x86_64-b6d154b23098/objdir-js/dist/include/js/Utility.h:396
    #3 0x55ead3406155 in js::wasm::StackMap::create(unsigned int) js/src/wasm/WasmGC.h:89
    #4 0x55ead3bd34b3 in js::wasm::StackMapGenerator::createStackMap(char const*, mozilla::Vector<bool, 32ul, js::SystemAllocPolicy> const&, unsigned int, js::wasm::HasRefTypedDebugFrame, mozilla::Vector<js::wasm::Stk, 0ul, js::SystemAllocPolicy> const&) js/src/wasm/WasmBaselineCompile.cpp:2346:26
    #5 0x55ead3babe0c in js::wasm::BaseCompiler::createStackMap(char const*, mozilla::Vector<bool, 32ul, js::SystemAllocPolicy> const&, unsigned int, js::wasm::HasRefTypedDebugFrame) js/src/wasm/WasmBaselineCompile.cpp:3296:31
/snip

For detailed crash information, see attachment.

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/5ca49059949b
user: Julian Seward
date: Fri Dec 14 13:00:44 2018 +0100
summary: Bug 1476251 - Generate stack maps in the Wasm Baseline compiler. r=lth.

Julian, is bug 1476251 a likely regressor?

Comment 3

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

Also setting needinfo? from our WebAssembly folks.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

I assume Julian will take a look here.

Comment 5

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 6

[Lars T Hansen [:lth]]

Deferring to Julian, for now.

Comment 7

[Lars T Hansen [:lth]]

Ryan, maybe you can take a look, ask Julian for specifics if necessary?

Comment 8

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1567040 - Destroy stackMap if we fail to append it to stackMaps_ due to OOM. r?jseward

Comment 9

[Ryan Hunt [:rhunt] (on leave until early May)]

Trying to get ASAN working so I can test this better, but my guess is the attached patch will fix this.

Comment 10

[Ryan Hunt [:rhunt] (on leave until early May)]

Got ASAN working and can confirm that this patch resolves the issue.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d551e573d4ce
Destroy stackMap if we fail to append it to stackMaps_ due to OOM. r=jseward

Comment 12

[Arthur Iakab [arthur_iakab]]

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

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift to beta? Thanks.

Comment 14

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 9086276 [details]
Bug 1567040 - Destroy stackMap if we fail to append it to stackMaps_ due to OOM. r?jseward

Beta/Release Uplift Approval Request

• User impact if declined: We may leak a stack map if an OOM occurs during initialization.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small modification to free a pointer when we have an OOM.
• String changes made/needed:

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9086276 [details]
Bug 1567040 - Destroy stackMap if we fail to append it to stackMaps_ due to OOM. r?jseward

Avoid a leak, seems good for beta uplift (should be in beta 6)

Comment 16

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ed65dcc5356c