Closed
Bug 759020
Opened 12 years ago
Closed 12 years ago
jstests.py swallows some seg faults
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: n.nethercote, Assigned: terrence)
Details
Attachments
(2 files)
1.14 KB,
patch
|
Details | Diff | Splinter Review | |
1.31 KB,
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
I landed the patch from bug 754181 and then had to back it out because it caused a seg fault on ecma_5/misc/future-reserved-words.js. But I'd run the test suite! So what went wrong? Let's see... with that patch applied, if I run it in jstests.py: [bayou:~/moz/mi6/js/src/tests] python jstests.py ../d64/js ecma_5/misc/future-reserved-words.js PASS But if I use jstests.py's -s option to get the command and run that by itself: [bayou:~/moz/mi6/js/src/tests] /home/njn/moz/mi6/js/src/d64/js -f shell.js -f ecma_5/shell.js -f ecma_5/misc/shell.js -f ./ecma_5/misc/future-reserved-words.js <...lots of output snipped...> PASSED! Implement FutureReservedWords per-spec: implements: strict function expression argument, SyntaxError: implements is a reserved identifier PASSED! Implement FutureReservedWords per-spec: implements: function expression argument retroactively strict, SyntaxError: implements is a reserved identifier Segmentation fault (core dumped) Hmm, that's not good.
Assignee | ||
Comment 1•12 years ago
|
||
What platform? I was not able to reproduce this by inserting a trivial segfault.
Reporter | ||
Comment 2•12 years ago
|
||
Inserting a trivial segfault isn't enough; only some seg faults are swallowed. Steps to reproduce: - Apply the attached patch. - Run ecma_5/misc/future-reserved-words.js. - The seg fault is swallowed. I've reproduced this on Linux64 and Mac64 debug builds. It's a totally reliable NULL deref crash, so I'm confident there's no non-determinism involved.
Attachment #628552 -
Flags: feedback?(terrence)
Assignee | ||
Comment 3•12 years ago
|
||
I broke this by fixing a bug, which was previously masked by yet another bug :-P. The bug I inadvertently fixed is to correctly post-process the child status code. "Real" process exit codes are in [0..128), with information about how the process exited, whether it generated a core dump, and what signal killed it, etc stored in higher bits. I opened Bug 754691 recently to try to fix the other problem, which is that we only consider a process as crashed if (simplifying a bit here) the test prints FAIL and the error code is not 0. The right fix here is to use the real process crash status provided by the OS to decide whether a process has crashed.
Assignee | ||
Comment 4•12 years ago
|
||
On deeper inspection, it turns out I was wrong. The subprocess modules does indeed return a baked error code, but a strange one where the signal is returned as a -signum. With this, the iscrashed tests in results.py make more sense (although an explicit sign check would still be clearer). This patch updates the returncode we pass to TestOutput to have the same format in the unix runner as in the windows runner.
Assignee: n.nethercote → terrence
Attachment #628975 -
Flags: review?(dmandelin)
Reporter | ||
Updated•12 years ago
|
Attachment #628552 -
Flags: feedback?(terrence)
Comment 5•12 years ago
|
||
Comment on attachment 628975 [details] [diff] [review] v0 Review of attachment 628975 [details] [diff] [review]: ----------------------------------------------------------------- Yay adapting for random OS corners. :-)
Attachment #628975 -
Flags: review?(dmandelin) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b178b0631bf0
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b178b0631bf0
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•