Closed Bug 942027 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: gkw, Assigned: dougc)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker])

Attachments

(2 files, 1 obsolete file)

Attached file 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)
Flags: needinfo?(benj)
This does not seem to reproduce on 32-bit Mac js shells, though.
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 :)
Flags: needinfo?(benj)
This occurs fairly often, yes, I found this on ARM.

Benjamin, I can give you access to the ARM board if you so require.
Flags: needinfo?(mrosenberg)
Whiteboard: [fuzzblocker]
> 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
Blocks: 930477
Attached patch ARM Float result (obsolete) — Splinter Review
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?
Attachment #8342010 - Flags: feedback?(gary)
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 on attachment 8342010 [details] [diff] [review]
ARM Float result

Yes, this patch does seem to.
Attachment #8342010 - Flags: feedback?(gary) → feedback+
(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?
Attachment #8342032 - Flags: review?(mrosenberg)
Attachment #8342032 - Flags: review?(mrosenberg) → review+
Keywords: checkin-needed
Attachment #8342010 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/3907208b88c5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Assignee: general → dtc-moz
You need to log in before you can comment on or make changes to this bug.