Last Comment Bug 774624 - IonMonkey: Differential Testing: Getting different TypeErrors ((void 0) vs. 0) without/with ion.
: IonMonkey: Differential Testing: Getting different TypeErrors ((void 0) vs. 0...
Status: RESOLVED FIXED
[ion:p1:fx18]
: regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- critical (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on:
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-07-17 05:45 PDT by Christian Holler (:decoder)
Modified: 2012-07-30 16:44 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix DecompileValueGenerator. (8.48 KB, patch)
2012-07-30 15:51 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-07-17 05:45:57 PDT
The following testcase shows different behavior with options --ion -n -m --ion-eager vs. --no-ion on ionmonkey revision c4c50dc6317c:


function printStatus (msg) {
  msg = msg.toString();
}
Function.prototype.toString ^= function () f(apply);
printStatus ("foo");
printStatus(let (printStatus) function() {});


$ debug64/js --ion -n -m --ion-eager test.js
test.js:2: TypeError: 0 is not a function

$ debug64/js --no-ion test.js
test.js:2: TypeError: (void 0) is not a function
Comment 1 David Anderson [:dvander] 2012-07-17 17:03:48 PDT
On the given cset, I get: test.js:2: TypeError: msg.toString is not a function

Since the same error type is actually throwing in both cases, this is most likely a DecompileValueGenerator difference so we can consider this low priority.
Comment 2 Nicolas B. Pierron [:nbp] 2012-07-26 11:39:21 PDT
This error appear on tbpl (Column J with Linux opt/pgo) and is critical for landing.
Comment 3 David Anderson [:dvander] 2012-07-27 16:14:26 PDT
It sounds like we'll have to figure out how to preserve this error message, or make the tests more lenient.
Comment 4 Nicolas B. Pierron [:nbp] 2012-07-30 15:51:55 PDT
Created attachment 647330 [details] [diff] [review]
Fix DecompileValueGenerator.

- Make sure ReconstructPCStack can skip JSOP_THROWING by using JSOP_GOTO if possible.
- Add slow IonMonkey slot iteration to report nice error messages in case of failures. (when possible)
Comment 5 David Anderson [:dvander] 2012-07-30 16:12:04 PDT
Comment on attachment 647330 [details] [diff] [review]
Fix DecompileValueGenerator.

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

Good idea re: abstracting the slot iteration.

::: js/src/jsopcode.cpp
@@ +5792,5 @@
> +             *
> +             * In case of IonMonkey, we are using the snapshot which has
> +             * captured a previous state, so it is likely that we may not be
> +             * able to recover an expression of an error thrown from an
> +             * IonMonkey frame.

You might be able to remove this comment addition? We'll have guarded before any error that could cause js_DecompileValueGenerator to run, so the frame will be in the interpreter. For other cases, they should be effectful, and will thus have a recent snapshot at the safepoint.

::: js/src/vm/Stack.h
@@ +1822,5 @@
>      Value       thisv() const;
>  
> +    // These are only valid for the last frame.
> +    size_t      numDynSlots() const;
> +    Value       dynSlotValue(size_t index) const;

A few naming nits:
 s/last/top/
 s/numDynSlots/numFrameSlots/
 s/dynSlotValue/frameSlotValue/

Maybe add a comment that the values in slots may be optimized away in JITs?
Comment 6 Nicolas B. Pierron [:nbp] 2012-07-30 16:44:58 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/ec897e9588f4

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