Closed Bug 794365 Opened 7 years ago Closed 7 years ago

Valgrind on tbpl detects leak - 4 bytes are definitely lost (direct) with JSInlineString::uninline on the stack

Categories

(Core :: JavaScript Engine, defect, major)

x86_64
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla18

People

(Reporter: gkw, Assigned: terrence)

References

(Blocks 1 open bug)

Details

(Keywords: valgrind)

Attachments

(3 files)

Valgrind detects a leak of 4 bytes (direct) with XPCConvert::JSData2Native on the stack, see attached snippet which comes from:

https://tbpl.mozilla.org/php/getParsedLog.php?id=15521837&tree=Firefox&full=1

Guessing Core: XPConnect, please change component if necessary.
It's probably Core: JavaScript Engine since JSInlineString::uninline is the common function among similar stacks.
Assignee: nobody → general
Component: XPConnect → JavaScript Engine
Summary: Valgrind on tbpl detects leak - 4 bytes are definitely lost (direct) with XPCConvert::JSData2Native on the stack → Valgrind on tbpl detects leak - 4 bytes are definitely lost (direct) with JSInlineString::uninline on the stack
Assignee: general → terrence
Attached file testcase
Attached patch v0: fixSplinter Review
Attachment #665119 - Flags: review?(wmccloskey)
Comment on attachment 665119 [details] [diff] [review]
v0: fix

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

::: js/src/vm/String.cpp
@@ +514,5 @@
> +    for (uint32_t i = 0; i < NUM_SMALL_CHARS * NUM_SMALL_CHARS; i++)
> +        length2StaticTable[i] = NULL;
> +
> +    for (uint32_t i = 0; i < INT_STATIC_LIMIT; i++)
> +        intStaticTable[i] = NULL;

Probably easier to use PodArrayZero as is done in the StaticStrings constructor. Or you could just factor that code out.
Attachment #665119 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/898ae4d394b3
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
A type of test for this bug has already been landed because it is already marked in-testsuite+ -> VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.