Closed Bug 979721 Opened 6 years ago Closed 6 years ago

Leaking BaselineBailout::buffer_

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: khuey, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lsan][MemShrink])

Attachments

(1 file)

Andrew ran Firefox with Leak Sanitizer enabled.  The JS reftests run shows the following output:

Indirect leak of 7168 byte(s) in 7 object(s) allocated from:
#0 0x446434 in calloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:90
#1 0x7fc62cbca9dd in js_calloc /builds/slave/try-l64-asan-00000000000000000/build/obj-firefox/js/src/../../dist/include/js/Utility.h:140
#2 0x7fc62cbca9dd in init /builds/slave/try-l64-asan-00000000000000000/build/js/src/jit/BaselineBailouts.cpp:105
#3 0x7fc62cbca9dd in js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::IonBailoutIterator&, bool, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*) /builds/slave/try-l64-asan-00000000000000000/build/js/src/jit/BaselineBailouts.cpp:1286
#4 0x7fc62cbd2fa4 in js::jit::ExceptionHandlerBailout(JSContext*, js::jit::InlineFrameIteratorMaybeGC<(js::AllowGC)1> const&, js::jit::ExceptionBailoutInfo const&, js::jit::BaselineBailoutInfo**) /builds/slave/try-l64-asan-00000000000000000/build/js/src/jit/Bailouts.cpp:169
#5 0x7fc62ce6dc74 in HandleExceptionIon /builds/slave/try-l64-asan-00000000000000000/build/js/src/jit/IonFrames.cpp:419
#6 0x7fc62ce6dc74 in js::jit::HandleException(js::jit::ResumeFromException*) /builds/slave/try-l64-asan-00000000000000000/build/js/src/jit/IonFrames.cpp:600
#7 0x7fc632053fff (+0x22fff)

At least one path where that can happen is when we take the early return at hg.mozilla.org/mozilla-central/annotate/e5b09585215f/js/src/jit/BaselineBailouts.cpp#l1379.  There are probably other paths too.
Whiteboard: [lsan]
Attached patch PatchSplinter Review
Thanks Andrew/Kyle, very nice find.

This patch moves the takeBuffer() call to right before the return. I also checked what the callers do with this buffer, and it looks like they all unconditionally call FinishBailoutToBaseline, where we free it.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8385964 - Flags: review?(kvijayan)
Attachment #8385964 - Flags: review?(kvijayan) → review+
Whiteboard: [lsan] → [lsan][MemShrink]
Kudos to mccr8 for trying LSAN!
https://hg.mozilla.org/mozilla-central/rev/4c9d799155d2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.