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
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.
What platform? I was not able to reproduce this by inserting a trivial segfault.
Created attachment 628552 [details] [diff] [review]
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.
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.
Created attachment 628975 [details] [diff] [review]
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.
Comment on attachment 628975 [details] [diff] [review]
Review of attachment 628975 [details] [diff] [review]:
Yay adapting for random OS corners. :-)