Closed Bug 969133 Opened 8 years ago Closed 8 years ago

OdinMonkey: Valgrind detects leak - 96 bytes are definitely lost (direct) with GrowStuff on the stack

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox30 --- fixed

People

(Reporter: gkw, Assigned: luke)

Details

(Keywords: testcase, valgrind)

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #913876 +++

(function(stdlib, foreign, heap) {
    "use asm";
    var Float64ArrayView = new stdlib.Float64Array(heap);
    function f() {}
    return f
})(this, {}, new ArrayBuffer())

shows "96 bytes in 1 blocks are definitely lost" on m-c changeset 1e9f169c9715 with --no-baseline --ion-parallel-compile=off --no-ion.

valgrind --vex-iropt-register-updates=allregs-at-mem-access --leak-check=full --smc-check=all-non-file ./js-opt-64-dm-vg-ts-er-linux-1e9f169c9715 --no-baseline --ion-parallel-compile=off --no-ion testcase.js

(happens also with --vex-iropt-register-updates=allregs-at-each-insn)


Is this another false positive, as per bug 913876?

My configure flags are:

AR=ar sh ./configure --enable-optimize=-O1 --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-valgrind --with-ccache --enable-threadsafe <other NSPR options>
Flags: needinfo?(luke)
> Is this another false positive, as per bug 913876?

That was an invalid write, but this is a leak. I doubt it's a false positive.
Yep, definitely real.  Looks like bug 865516 and bug 917800 introduced the leak by passing JS_smprintf results to LinkFail, which only expects string literals and doesn't free.  Easy fix, though.
Flags: needinfo?(luke)
Component: JavaScript Engine → JavaScript Engine: JIT
Attached patch fix-leakSplinter Review
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8372313 - Flags: review?(benj)
Comment on attachment 8372313 [details] [diff] [review]
fix-leak

Review of attachment 8372313 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch, thanks!
Attachment #8372313 - Flags: review?(benj) → review+
https://hg.mozilla.org/mozilla-central/rev/be7fa3989aed
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.