657881 - TI: Make type failures abort like assertions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Patch

Currently, the TI branch uses the function [TypeFailure] to print a custom "infer failure" message and to terminate the engine afterwards with a crash. I propose to change this behavior: Instead of printing a custom message and crashing, I suggest to use [JS_Assert] to print an assertion instead and cause an abort. The main benefits are:

* Standard tools (e.g. fuzzers) can use the assertion message to differentiate between several failures more easily
* JS_Assert contains code for multiple OSes

Note that JS_Assert is active during release builds.

I've made a small patch that shows how this could be done. Please let me know what needs to be changed in order to get this into TI. What I'm not fully sure about is this statement:

cx->compartment->types.print(cx, cx->compartment);

I've never seen this output something, so I don't know what its purpose is.

With the patch applied, the message looks for example like this:

$ ./js -n -a test.js 
Assertion failure: [infer failure] Missing type at #2:00021 pushed 0: string, at jsinfer.cpp:285
Aborted

Comment 1

[Brian Hackett [Laid off!]]

Comment on attachment 533209 [details] [diff] [review]
Patch

Looks good, do you need this pushed?

If the INFERFLAGS environment variable is set to 'result' or 'full', the cx->compartment->types.print dumps the current type state of all scripts and type objects to stdout.

Comment 2

[Christian Holler (:decoder)]

Attachment: Improved patch

Attached is an improved patch which changes the following things:

- It first prints out the regular failure message (without the [infer failure] though) with all its details
- It then aborts with an assertion that has the same message minus stuff that I consider too unique for the assertion. In this case I removed the pc and index variables from the message.
- The "unknown bytecode" message should now include the unknown byte in the message and assertion.


Example with bug 657193:

$ ./js -m -n -a test.js
Missing type at #3:00029 pushed 0: void
Assertion failure: [infer failure] Missing type, pushed 0: void, at js/src/jsinfer.cpp:287
Aborted


Is it ok like that? :)

Comment 3

[Brian Hackett [Laid off!]]

Looks good, will push within the next couple hours.

Comment 4

[Brian Hackett [Laid off!]]

http://hg.mozilla.org/projects/jaegermonkey/rev/11714be33655