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...
: 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]
: Jason Orendorff [:jorendorff]
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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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

Description User image 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 User image 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 User image 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 User image 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 User image 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 User image 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:

Maybe add a comment that the values in slots may be optimized away in JITs?
Comment 6 User image Nicolas B. Pierron [:nbp] 2012-07-30 16:44:58 PDT

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