valgrind errors in sessionstore mochitest chrome tests

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: sayrer, Assigned: mrbkap)

Tracking

({fixed1.9.0.2, intermittent-failure, valgrind})

unspecified
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.0.x +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

11 years ago
These are JS Engine errors.
Reporter

Comment 1

11 years ago
    if ((!js_IdIsIndex(id, &i) &&
         id != ATOM_TO_JSID(cx->runtime->atomState.lengthAtom)) ||
        obj->fslots[JSSLOT_ARRAY_LENGTH] == 0 ||
        i >= ARRAY_DENSE_LENGTH(obj) ||
        obj->dslots[i] == JSVAL_HOLE)

If the first || left-hand side is false, then either js_IdIsIndex(id, &i), in which case i will be set, or id == 'length', in which case i will not be set and the use of i in the fourth line will UMR.

/be
Assignee: general → shaver
That...could be true, yes. *blush*

I'll patch this today, unless someone beats me to it.

sayrer: is there a ready-to-go setup on sm-valgrind for me to test this on?  I don't have a Linux VM or desktop install atm.
Flags: wanted1.9.0.x?
Posted patch Proposed fixSplinter Review
I just ran this patch through mochitests and things came out clean [1].

[1] I did leak a bunch of stuff, but in my recollection, I've never *not* leaked when running a full set of MochiTests in a debug build.
Assignee: shaver → mrbkap
Status: NEW → ASSIGNED
Attachment #328325 - Flags: review?(shaver)

Updated

11 years ago
Blocks: 438871
Comment on attachment 328325 [details] [diff] [review]
Proposed fix

r=shaver, thanks
Attachment #328325 - Flags: review?(shaver) → review+

Updated

11 years ago
Keywords: valgrind
Pushed as changeset 6059b09ca278.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 328325 [details] [diff] [review]
Proposed fix

I'm guessing we want this on the 1.9 branch.
Attachment #328325 - Flags: approval1.9.0.2?

Updated

11 years ago
Flags: in-testsuite+
Flags: in-litmus-
Comment on attachment 328325 [details] [diff] [review]
Proposed fix

Approved for 1.9.0.2. Please land in CVS. a=ss
Attachment #328325 - Flags: approval1.9.0.2? → approval1.9.0.2+
Fix checked into the 1.9 branch.
Keywords: fixed1.9.0.2
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Whiteboard: [orange]
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.