Closed
Bug 810767
Opened 12 years ago
Closed 12 years ago
Update Valgrind flags in jit_test.py
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: gkw, Assigned: gkw)
References
Details
Attachments
(1 file, 1 obsolete file)
1.84 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
Valgrind flags in js/src/jit_test.py need updating. In particular, we should use --smc-check=all-non-file instead of --smc-check=all, and adding --gen-suppressions=all and --show-possibly-lost=no would be nice. Some questions remain: 1. Is DEBUGGER_INFO at the top of jit_tests.py used? There are some Valgrind options present here that I'm not sure if I should change. 2. I'd like to add a default --suppressions= argument to point to e.g. build/valgrind/cross-architecture.sup Or should I point it to _valgrind (the one that appears after Make finishes processing)? How should it be done? 3. What should be the default exit code be, when Valgrind encounters an error? 1 or another number, e.g. 77? These options were previously last touched in Sep 2009 by Graydon Hoare in https://hg.mozilla.org/mozilla-central/rev/6eda292e83a7 but I don't think he's working on JS anymore.
Attachment #680549 -
Flags: feedback?(terrence)
Assignee | ||
Comment 1•12 years ago
|
||
> Created attachment 680549 [details] [diff] [review]
> WIP v0
(There were a couple of whitespace removals in this patch as well)
Comment 2•12 years ago
|
||
Comment on attachment 680549 [details] [diff] [review] WIP v0 Review of attachment 680549 [details] [diff] [review]: ----------------------------------------------------------------- The patch looks fine to me. DEBUGGER_INFO does appear to be dead, though I'm not certain... it looks like it was copied from build/automationutils.py. I don't know about the suppression file or the exit code.
Attachment #680549 -
Flags: feedback+
Comment 3•12 years ago
|
||
Comment on attachment 680549 [details] [diff] [review] WIP v0 I'm not a peer on the jit-test code or on valgrind, so I'm just going to let Nick have this one.
Attachment #680549 -
Flags: feedback?(terrence)
Assignee | ||
Comment 4•12 years ago
|
||
> 1. Is DEBUGGER_INFO at the top of jit_tests.py used? There are some Valgrind
> options present here that I'm not sure if I should change.
This doesn't seem used - automationutils.py doesn't seem to be connected to jit_test.py as discussed on IRC. I'll remove this in the next patch.
Assignee | ||
Comment 5•12 years ago
|
||
(1) Removed DEBUGGER_INFO. (2) Let's consider adding default suppression parameters in another bug. (3) Retained the same error exit code 1 - since it's also used for Valgrind TBPL builds at https://hg.mozilla.org/build/tools/file/default/scripts/valgrind/valgrind.sh Let's fix up the flags first by landing this patch. Requesting review from njn for real this time.
Assignee: general → gary
Attachment #680549 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #692499 -
Flags: review?(n.nethercote)
Updated•12 years ago
|
Attachment #692499 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Setting checkin-needed - inbound is closed for now.
Keywords: checkin-needed
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d37287856bd
Keywords: checkin-needed
Assignee | ||
Comment 8•12 years ago
|
||
> (2) Let's consider adding default suppression parameters in another bug. I filed bug 821965.
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7d37287856bd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•