942027 - Assertion failure: isObject(), at dist/include/js/Value.h or Assertion failure: JSVAL_IS_DOUBLE_IMPL(l), at dist/include/js/Value.h or Assertion failure: v.isUndefined(), at jsnum.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stacks

function f(x) {
    Math.fround(Math.tan(0)) + (function() {})
}
for (var i = 0; i < 3; i++) {
    try {
        f(-Number.MAX_VALUE);
    } catch (e) {}
}

asserts js debug shell on m-c changeset c7cbfa315d46 with --ion-eager at Assertion failure: isObject(), at dist/include/js/Value.h on ARM Ubuntu Linux.

A variant:

function f(x) {
    Math.fround(Math.tan(0))(function() {})
}
for (var i = 0; i < 3; i++) {
    try {
        f(-Number.MAX_VALUE);
    } catch (e) {}
}

asserts at Assertion failure: JSVAL_IS_DOUBLE_IMPL(l), at dist/include/js/Value.h

Prior to reduction, the assertion was: Assertion failure: v.isUndefined(), at jsnum.cpp

My configure flags are:

CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh /home/fuzz5lin/Desktop/js-dbg-32-dm-ts-sfp-linux-mozilla-central-156775-c7cbfa315d46-6TujqD/compilePath/js/src/configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Guessing this is also related to bug 941381 or bug 918163, so I won't be running bisection here on this ARM board. (I could, but it takes a long time)

Comment 1

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

This does not seem to reproduce on 32-bit Mac js shells, though.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Is this specific to ARM? Can't reproduce this on either x32 or x64. Don't have any ARM devices, so it's hard to investigate.

Looking at the stack trace and the generated MIR, the addition is specialized as a string addition, and the encoding of LHS fails for some reason. I also see that a bailout occurs. I am not sure that this is an error due to the float32 math function, so a bisection range would help here.

Cc'ing Marty in case he gets bored and has some time to investigate this issue :)

Comment 3

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

This occurs fairly often, yes, I found this on ARM.

Benjamin, I can give you access to the ARM board if you so require.

Comment 4

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

> I am not sure that this is an
> error due to the float32 math function, so a bisection range would help here.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/f55bec181790
user:        Benjamin Bouvier
date:        Mon Oct 28 14:24:19 2013 +0100
summary:     Bug 930477: Specialize Math.floor for Float32; r=jandem,jonco

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: ARM Float result

The only change caused by the Math.floor patch is that it causes callWithABI to use a FLOAT result type, but ARM isn't specialized for this kind of result.

Gary, does this patch fix the issue?

Comment 6

[Douglas Crosher [:dougc]]

Attachment: Add float32 support to callWithABIPost

I was testing a similar patch for this and it does appear to fix this.  There is no need for a move in the case of the hard-float api, so it has been removed.

Would you like to request I review this one, or visa-versa?

I notice that the float32 argument passing is barely working too.  It seems to hold together for a single float32 argument, but really needs some work, and seems to depend on adding float32 support to the move resolver.

I am still seeing intermittent crashes on ARM jit-testing, but do not yet know the cause of the latest.

Comment 7

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

Comment on attachment 8342010 [details] [diff] [review]
ARM Float result

Yes, this patch does seem to.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Douglas Crosher [:dougc] from comment #6)
> Created attachment 8342032 [details] [diff] [review]
> Add float32 support to callWithABIPost
> 
> I was testing a similar patch for this and it does appear to fix this. 
> There is no need for a move in the case of the hard-float api, so it has
> been removed.
> 
> Would you like to request I review this one, or visa-versa?

Cool, thanks! As your patch eliminates some other code I don't know about, I think you should ask r? for Marty, as he is the ARM JIT specialist.

> 
> I notice that the float32 argument passing is barely working too.  It seems
> to hold together for a single float32 argument, but really needs some work,
> and seems to depend on adding float32 support to the move resolver.

The only way to have some Float32 argument to function calls is to call one of the MMathFunctions that can be specialized for Float32, unless I am missing something. In this case, registers just contain the float value and it seemed to work, at least locally with ARM emulation (which is not realistic). Are there some issues on devices you tested with?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3907208b88c5

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3907208b88c5