Closed Bug 657881 Opened 9 years ago Closed 9 years ago
TI: Make type failures abort like assertions
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
Attachment #533209 - Flags: review?(bhackett1024)
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.
Attachment #533209 - Flags: review?(bhackett1024) → review+
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? :)
Attachment #536916 - Flags: review?(bhackett1024) → review+
Looks good, will push within the next couple hours.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.