Note: There are a few cases of duplicates in user autocompletion which are being worked on.

jstests.py swallows some seg faults

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: terrence)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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

5 years ago
What platform?  I was not able to reproduce this by inserting a trivial segfault.
(Reporter)

Comment 2

5 years ago
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.
Attachment #628552 - Flags: feedback?(terrence)
(Assignee)

Comment 3

5 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

5 years ago
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.
Assignee: n.nethercote → terrence
Attachment #628975 - Flags: review?(dmandelin)
(Reporter)

Updated

5 years ago
Attachment #628552 - Flags: feedback?(terrence)
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

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b178b0631bf0
https://hg.mozilla.org/mozilla-central/rev/b178b0631bf0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.