1055762 - Assertion failure: conversion != MToDouble::NumbersOnly, at jit/Lowering.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

function g() {
    f(0);
}
function f(y) {
    return (undefined <= y);
}
try {
    g();
} catch (e) {}
(function() {
    f()()
})();

asserts js debug shell on m-c changeset 4d94eeca89f3 with --ion-offthread-compile=off --ion-eager at Assertion failure: conversion != MToDouble::NumbersOnly, at jit/Lowering.cpp

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-nspr-build

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140818122115" and the hash "a14312d5203e".
The "bad" changeset has the timestamp "20140818124517" and the hash "43494708df76".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a14312d5203e&tochange=43494708df76

Hannes, is bug 1054512 a likely regressor? Setting s-s and assuming sec-high because a similar assert (which has Symbol, but this one doesn't) as a start.

Comment 2

[Hannes Verschore [:h4writer]]

Owh, my bad. This is not a dupe of the other one.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: bug1055762

The issue is that the ToDoublePolicy didn't looked at the conversion() flag. As a result it allowed Undefined (not boxed), which isn't allowed for conversion kind NumbersOnly.

I updated ToDoublePolicy and ToInt32Policy to also look at those. (Though they are only used in MComparePolicy). So TypePolicy will now work with all the different given conversion kinds, without a hinge. No need for manual boxing here too.

Note: There is one algorithmic change. We used to box MIRType_Double and MIRType_Float32 with conversion flag "NumbersOnly". I removed this, since I think this is over-restrictive.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8476707 [details] [diff] [review]
bug1055762

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

::: js/src/jit/TypePolicy.cpp
@@ +525,5 @@
> +    if (ins->isToDouble()) {
> +        conversion = ins->toToDouble()->conversion();
> +    } else {
> +        conversion = ins->toToFloat32()->conversion();
> +    }

Nit: no {}

@@ +544,5 @@
> +      case MIRType_Boolean:
> +        // No need for boxing, when we will convert.
> +        if (conversion == MToFPInstruction::NonStringPrimitives)
> +            return true;
> +        // No need for boxing, when we will convert.

Nit: I think you can remove this comment and leave the fist one.

@@ +593,5 @@
> +      case MIRType_Boolean:
> +        // No need for boxing, when we will convert.
> +        if (conversion == MacroAssembler::IntConversion_Any)
> +            return true;
> +        // No need for boxing, when we will convert.

And here.

Comment 5

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/05c3110d8a10

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/05c3110d8a10

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.