Closed Bug 1567040 Opened 5 months ago Closed 3 months ago

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

Categories

(Core :: Javascript: WebAssembly, defect, P2, critical)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed
firefox71 --- verified

People

(Reporter: gkw, Assigned: rhunt)

References

(Blocks 2 open bugs, Regression)

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(2 files)

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.

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?

Flags: needinfo?(jseward)
Regressed by: 1476251

Also setting needinfo? from our WebAssembly folks.

Flags: needinfo?(lhansen)
Flags: needinfo?(bbouvier)

I assume Julian will take a look here.

Component: JavaScript Engine → Javascript: WebAssembly
Flags: needinfo?(bbouvier)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Deferring to Julian, for now.

Flags: needinfo?(lhansen)

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

Flags: needinfo?(rhunt)
Priority: -- → P2

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

Flags: needinfo?(rhunt)
Flags: needinfo?(jseward)

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

Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/d551e573d4ce
Destroy stackMap if we fail to append it to stackMaps_ due to OOM. r=jseward
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Can you request uplift to beta? Thanks.

Flags: needinfo?(rhunt)

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:
Flags: needinfo?(rhunt)
Attachment #9086276 - Flags: approval-mozilla-beta?

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)

Attachment #9086276 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.