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

VERIFIED FIXED in mozilla18

Status

()

Core
JavaScript Engine
--
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Assigned: terrence)

Tracking

(Blocks: 1 bug, {valgrind})

Trunk
mozilla18
x86_64
Linux
valgrind
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 664817 [details]
m-c changeset 08d435dedc7f Valgrind stack with line numbers

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.
(Reporter)

Comment 1

6 years ago
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)

Updated

6 years ago
Assignee: general → terrence
(Assignee)

Comment 2

6 years ago
Created attachment 665118 [details]
testcase
(Assignee)

Comment 3

6 years ago
Created attachment 665119 [details] [diff] [review]
v0: fix
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
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Reporter)

Comment 7

6 years ago
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.