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

RESOLVED FIXED in Firefox 30

Status

()

--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: luke)

Tracking

(Blocks: 1 bug, {testcase, valgrind})

Trunk
mozilla30
x86_64
Linux
testcase, valgrind
Points:
---

Firefox Tracking Flags

(firefox30 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 8371888 [details]
Valgrind stack with --vex-iropt-register-updates=allregs-at-mem-access

+++ 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.
(Assignee)

Comment 2

5 years ago
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)
(Reporter)

Updated

5 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
(Assignee)

Comment 3

5 years ago
Created attachment 8372313 [details] [diff] [review]
fix-leak
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(Reporter)

Updated

5 years ago
status-firefox30: --- → fixed
You need to log in before you can comment on or make changes to this bug.