Last Comment Bug 759020 - jstests.py swallows some seg faults
: jstests.py swallows some seg faults
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Terrence Cole [:terrence]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-27 21:54 PDT by Nicholas Nethercote [:njn]
Modified: 2012-06-02 12:01 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
crash-causing patch (1.14 KB, patch)
2012-05-30 18:00 PDT, Nicholas Nethercote [:njn]
no flags Details | Diff | Splinter Review
v0 (1.31 KB, patch)
2012-05-31 16:52 PDT, Terrence Cole [:terrence]
dmandelin: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2012-05-27 21:54:05 PDT
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.
Comment 1 Terrence Cole [:terrence] 2012-05-30 15:01:36 PDT
What platform?  I was not able to reproduce this by inserting a trivial segfault.
Comment 2 Nicholas Nethercote [:njn] 2012-05-30 18:00:52 PDT
Created attachment 628552 [details] [diff] [review]
crash-causing patch

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.
Comment 3 Terrence Cole [:terrence] 2012-05-31 15:54:17 PDT
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.
Comment 4 Terrence Cole [:terrence] 2012-05-31 16:52:33 PDT
Created attachment 628975 [details] [diff] [review]
v0

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 5 David Mandelin [:dmandelin] 2012-06-01 11:16:43 PDT
Comment on attachment 628975 [details] [diff] [review]
v0

Review of attachment 628975 [details] [diff] [review]:
-----------------------------------------------------------------

Yay adapting for random OS corners. :-)
Comment 6 Terrence Cole [:terrence] 2012-06-01 11:44:42 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/b178b0631bf0

Note You need to log in before you can comment on or make changes to this bug.