1577580 - LeakSanitizer: [@ js::frontend::LazyScriptCreationData::init] involving oomTest

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

The following testcase crashes on mozilla-central revision 8edbf8fe48bf (build with --enable-debug --enable-more-deterministic --enable-address-sanitizer --disable-optimize, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// Adapted from randomly chosen test: js/src/jit-test/tests/parser/bug-1576865-1.js
var x = "function f() { \
    {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; {}; } \
}";
oomTest(function() {
    evaluate(x);
});

Backtrace:

Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x56333b3779f3 in __interceptor_malloc (/home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf+0x35f79f3)
    #1 0x56333b51bf34 in SystemMalloc::malloc(unsigned long) memory/build/malloc_decls.h:38:1
    #2 0x56333b51be68 in DummyArenaAllocator<SystemMalloc>::moz_arena_malloc(unsigned long, unsigned long) memory/build/malloc_decls.h:38:1
    #3 0x56333b51bd4c in moz_arena_malloc memory/build/malloc_decls.h:116:1
    #4 0x56333cf8767a in js_arena_malloc(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/js/Utility.h:392:10
    #5 0x56333cf87e66 in JSAtom** js_pod_arena_malloc<JSAtom*>(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/js/Utility.h:600:26
    #6 0x56333d0df0f0 in JSAtom** js::AllocPolicyBase::maybe_pod_arena_malloc<JSAtom*>(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/js/AllocPolicy.h:31:12
    #7 0x56333d0df0c4 in JSAtom** js::AllocPolicyBase::pod_arena_malloc<JSAtom*>(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/js/AllocPolicy.h:44:12
    #8 0x56333d0df062 in JSAtom** js::AllocPolicyBase::pod_malloc<JSAtom*>(unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/js/AllocPolicy.h:70:12
    #9 0x56333d0dec78 in mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy>::convertToHeapStorage(unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/mozilla/Vector.h:930:30
    #10 0x56333d0de7d6 in mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy>::growStorageBy(unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/mozilla/Vector.h:1018:12
    #11 0x56333d3b6fd0 in bool mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy>::append<JSAtom*>(JSAtom* const*, JSAtom* const*) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/mozilla/Vector.h:1323:9
    #12 0x56333d3b696f in bool mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy>::append<JSAtom*>(JSAtom* const*, unsigned long) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/mozilla/Vector.h:1379:10
    #13 0x56333d3b4be0 in bool mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy>::appendAll<JSAtom*, 24ul, js::SystemAllocPolicy>(mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy> const&) /home/ubuntu/shell-cache/js-dbg-optDisabled-64-dm-asan-linux-x86_64-8edbf8fe48bf/objdir-js/dist/include/mozilla/Vector.h:1372:10
    #14 0x56333d3aebd5 in js::frontend::LazyScriptCreationData::init(JSContext*, mozilla::Vector<JSAtom*, 24ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::frontend::FunctionBox const*, 8ul, js::TempAllocPolicy>&, bool) js/src/frontend/SharedContext.h:293:29
/snip

For detailed crash information, see attachment.

(you may need to disable jemalloc as well)

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/afe2efc57104
user: Ted Campbell
date: Tue Aug 27 23:02:37 2019 +0000
summary: Bug 1576865 - Add missing call to ReportOutOfMemory in LazyScriptCreationData. r=jwalden

Ted, is bug 1576865 a likely regressor?

Comment 3

[Fuzzing Team]

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

Comment 4

[Ted Campbell [:tcampbell]]

This still happens if I revert Bug 1576865 so I think it is probably from Bug 1567579. The testcase is mutated from the one I added this week though. Will take a look since it is surprising we leak this.

Comment 5

[Ted Campbell [:tcampbell]]

Ugh. The LazyScriptCreationData::closedOverBindings are allocated on SystemAllocPolicy which means they can leak when functionbox is thrown away without destructing. The data type of closedOverBindings should change it's alloc policy. This would also have avoided Bug 1576865. The reason I had suggested we use frontend::AtomVector in first place was consistency with rest of frontend, but that was a mistake in retrospect.

Comment 6

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

Pernosco trace:

https://pernos.co/debug/tuifsXGKS9QBzDejcIIJeg/index.html

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1577580 - Cleanup LazyScriptData when not in deferred mode. r?mgaudet

The LazyScriptCreationData arrays are not cleaned up when parse fails
since FunctionBox destructors are not run. As a workaround, clear the
data when failures occur during non-deferred lazy script creation.

NOTE: The deferred case may still leak in certain cases but it is
currently disabled.

Comment 8

[Ted Campbell [:tcampbell]]

We'll test this next week and then probably uplift to early beta. The change should probably right nightly for a few days.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ea73e899bd4
Cleanup LazyScriptData when not in deferred mode. r=mgaudet

Comment 10

[Ted Campbell [:tcampbell]]

After this rides Nightly for a few days, we should uplift this to fix a memory leak in Beta70.

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/4ea73e899bd4

Comment 12

[Julien Cristau [:jcristau]]

(In reply to Ted Campbell [:tcampbell] from comment #10)

After this rides Nightly for a few days, we should uplift this to fix a memory leak in Beta70.

It's been a week, time to do that now? :)

Comment 13

[Ted Campbell [:tcampbell]]

Thanks for the reminder. I created a test case to demonstrate this in Beta and have realized that this memory leak is exceptionally rare and requires an OOM at a specific moment. I was originally thinking this may occur during syntax errors, but they won't fail at this critical point.

The fix we have is still sensible for m-c, but it isn't worth resources to uplift this edge-case of an edge-case to beta.