Closed Bug 1195588 Opened 9 years ago Closed 9 years ago

Assertion failure: num.isNumber(), at jit/Recover.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: gkw, Assigned: h4writer)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files, 1 obsolete file)

function m(f) {
    for (var k = 0; k < 2; ++k) {
        try {
            f()
        } catch (e) {}
    }
}
function g(i) {
    x
}
m(g)
function h() {
    g(Math.sqrt(+((function() {}) < 1)))
}
m(h)

asserts js debug shell on m-c changeset 9673c75864be with --fuzzing-safe --no-threads --ion-eager at Assertion failure: num.isNumber(), at jit/Recover.cpp

Configure options:

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

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r 9673c75864be

=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150814025439" and the hash "2db399cd414f8ca2e70b41d4c36d50fa0a9a8d87".
The "bad" changeset has the timestamp "20150814034639" and the hash "d791ba00bf065740fc504329a1075f5132cdc800".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2db399cd414f8ca2e70b41d4c36d50fa0a9a8d87&tochange=d791ba00bf065740fc504329a1075f5132cdc800

Hannes, is bug 1171945 a likely regressor?
Flags: needinfo?(hv1989)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x9c2b0, 0x00000001006a0085 js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::RSqrt::recover(this=<unavailable>, cx=<unavailable>, iter=<unavailable>) const + 261 at Recover.cpp:839, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001006a0085 js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::RSqrt::recover(this=<unavailable>, cx=<unavailable>, iter=<unavailable>) const + 261 at Recover.cpp:839
    frame #1: 0x00000001005d74e3 js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::SnapshotIterator::computeInstructionResults(this=<unavailable>, cx=0x000000010284c400, results=0x00007fff5fbfe0a8) const + 307 at JitFrames.cpp:2243
    frame #2: 0x00000001005d695e js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::SnapshotIterator::initInstructionResults(this=0x00007fff5fbfd200, fallback=0x00007fff5fbfd0f0) + 542 at JitFrames.cpp:2197
    frame #3: 0x0000000100478cf1 js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::JitFrameIterator&, bool, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*) [inlined] SnapshotIteratorForBailout::init(this=<unavailable>, cx=<unavailable>) + 60 at BaselineBailouts.cpp:437
    frame #4: 0x0000000100478cb5 js-dbg-64-dm-nsprBuild-darwin-9673c75864be`js::jit::BailoutIonToBaseline(cx=<unavailable>, activation=<unavailable>, iter=0x00007fff5fbfd368, invalidate=true, bailoutInfo=0x00007fff5fbfd360, excInfo=0x00007fff5fbfdb88) + 645 at BaselineBailouts.cpp:1489
(lldb)
Attached patch Fix ToDouble recover (obsolete) — Splinter Review
Apparently ToDouble wasn't casting the bool to double type. Which seems like an issue predating the the regression range?

Though I haven't found why we now suddenly decide to bailout and as consequence trigger recover instructions to get executed. Before my patch the same MIRs (and RIRs) were created, but we didn't bailed. Patch should have been a NOP, which it apparently isn't. Still investigating.
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8649745 - Flags: review?(nicolas.b.pierron)
*removed debug statements
Attachment #8649745 - Attachment is obsolete: true
Attachment #8649745 - Flags: review?(nicolas.b.pierron)
Attachment #8649748 - Flags: review?(nicolas.b.pierron)
(In reply to Hannes Verschore [:h4writer] from comment #2)
> Created attachment 8649745 [details] [diff] [review]
> Fix ToDouble recover
> 
> Apparently ToDouble wasn't casting the bool to double type. Which seems like
> an issue predating the the regression range?
> 
> Though I haven't found why we now suddenly decide to bailout and as
> consequence trigger recover instructions to get executed. Before my patch
> the same MIRs (and RIRs) were created, but we didn't bailed. Patch should
> have been a NOP, which it apparently isn't. Still investigating.

Ok, I just found the place this isn't a NOP. "int + bool" (for any binary operation) used to get boxed. Now we do double specialization. Could get improved by doing int32 specialization for "int + bool" and "int + null"
Comment on attachment 8649748 [details] [diff] [review]
Fix ToDouble recover

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

::: js/src/jit/Recover.cpp
@@ +1111,5 @@
> +    double dbl;
> +    if (!ToNumber(cx, v, &dbl))
> +        return false;
> +
> +    result.set(DoubleValue(dbl));

nit: use  result.setDouble(dbl) .
Attachment #8649748 - Flags: review?(nicolas.b.pierron) → review+
You probably didn't want to land those printfs
(In reply to :Ms2ger from comment #7)
> You probably didn't want to land those printfs

Indeed! Somehow I still had the older patch version in my queue. Strange. Gonna reland the good version when tree is green again.
Flags: needinfo?(hv1989)
https://hg.mozilla.org/mozilla-central/rev/a9d2ba227725
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: