504516 - TM: Crash with a "divide by zero" error (really -2147483648 / -1)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

for (d in [0, 0]) {
    const a = (d -= (++d).toString())
    for each(b in [Number(1) << d, 0, 0xC]) {
        b / a
    }
}

crashes with TM branch js opt shell with -j with a DIVIDE BY ZERO error. Doesn't seem to occur in 1.9.1.

Exception Type:  EXC_ARITHMETIC (SIGFPE)
Exception Codes: EXC_I386_DIV (divide by zero)
Crashed Thread:  0

Thread 0 Crashed:
0   ???                           	0x0017ff51 0 + 1572689
1   ???                           	0xbffff228 0 + 3221221928
2   js-opt-tm-intelmac            	0x000f0e7e js_MonitorLoopEdge(JSContext*, unsigned int&) + 1326
3   js-opt-tm-intelmac            	0x00054ffa js_Interpret + 44266
4   js-opt-tm-intelmac            	0x0005a0c7 js_Execute + 407
5   js-opt-tm-intelmac            	0x0000e54c JS_ExecuteScript + 60
6   js-opt-tm-intelmac            	0x00004100 Process(JSContext*, JSObject*, char*, int) + 1616
7   js-opt-tm-intelmac            	0x000077cf main + 879
8   js-opt-tm-intelmac            	0x000022db _start + 209
9   js-opt-tm-intelmac            	0x00002209 start + 41

Comment 1

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

autoBisect shows this is probably related to bug 474443 :

The first bad revision is:
changeset:   28973:812a94dc7dd5
user:        Andreas Gal <[e-mail]>
date:        Fri Jun 12 08:33:32 2009 -0700
summary:     Implement oracle-based speculative fmod/fdiv/fmul demotion (474443, r=graydon,dvander).

Comment 2

[Jesse Ruderman]

Still crashes for me.

Comment 3

[Andreas Gal :gal]

Attachment: patch

Comment 4

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

Comment on attachment 395550 [details] [diff] [review]
patch

The reason this happens is that it's dividing -2147483648 / -1, the result of which is 2147483648. But that doesn't fit in a 32-bit signed integer so the CPU bizarrely throws the "divide by zero" exception.

Might want to add a comment about that in the patch, or change the comment above the line.

Comment 5

[Andreas Gal :gal]

Attachment: patch

Comment 6

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

Comment on attachment 396364 [details] [diff] [review]
patch


>-        if (!d1->isconst())

Should we keep this check?

Comment 7

[Andreas Gal :gal]

Attachment: patch

Comment 8

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/48c039f7ac4f

Comment 9

[Andreas Gal :gal]

Backed out. As much as I can't believe I am guilty it looks like I made us go orange.

http://hg.mozilla.org/tracemonkey/rev/dc2598d08078

Comment 10

[Jesse Ruderman]

The orange that happened with this checkin looked random.  The serious permaorange was clearly caused by bug 471214.

Comment 11

[Andreas Gal :gal]

Testing Jesse's theory. Brendan backed out 471214.

http://hg.mozilla.org/tracemonkey/rev/dc2598d08078

Comment 12

[Andreas Gal :gal]

Added testcase

http://hg.mozilla.org/tracemonkey/rev/9ad5876ecea2

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/48c039f7ac4f

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/3a8acec84491 (for real, apparently)

Comment 15

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/706eab21428e

Comment 16

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929