774624 - IonMonkey: Differential Testing: Getting different TypeErrors ((void 0) vs. 0) without/with ion.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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] - inactive, e-mail if emergency]

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]]

This error appear on tbpl (Column J with Linux opt/pgo) and is critical for landing.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

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]]

Attachment: 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] - inactive, e-mail if emergency]

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]]

https://hg.mozilla.org/projects/ionmonkey/rev/ec897e9588f4