TM: Wrong number with |let|, JSVAL_INT_MAX, type-unstable loop, global

RESOLVED WORKSFORME

Status

()

P1
normal
RESOLVED WORKSFORME
11 years ago
10 years ago

People

(Reporter: jruderman, Assigned: dvander)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla1.9.1b3
testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

11 years ago
for each (b in [0x3FFFFFFF, 0x3FFFFFFF, 0x3FFFFFFF]) {
  for each (let e in [{}, {}, {}, "", {}]) { 
    b = (b | 0x40000000) + 1;
    print('' + b); 
  }
}
delete b;

The last number printed is wrong with -j:

2147483648
-1073741823
-1073741822
-1073741821
-1073741820
2147483648
-1073741823
-1073741822
-1073741821
-1073741820
2147483648
-1073741823
-1073741822
-1073741821
-1073741695    <-- wrong (off by 125)
I stepped through in a debugger which I left open overnight (and promptly lost when I accidentally turned off the switch powering the outlet where my laptop was plugged), and there seems to have been a missing (?) i2f somewhere, because the value passed to js_BoxInt32 (I *think* that was the builtin being called) was approximately $xmm0.v2_double[1] == -1073741656.000004, but $xmm0.v4_int32[3] == -1073741821 (or thereabouts, going from memory, last digit might have been 0).

Updated

11 years ago
Flags: blocking1.9.1+
I'm pretty close on this, but I don't have time to figure out the last piece right now. What happens is that the generated code in the last trace run is trying to load the global 'b', and expects it to be a double, because the type map says so. But it's really an int. And that's because the second-to-last trace run stores an int there. The generated LIR:

              add eax,1                       eax(or1) ecx(gp) ebx($stack3) esi(sp) edi(cx)
              jo 0x292f79                     eax(add1) ecx(gp) ebx($stack3) esi(sp) edi(cx)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x292f79:
                                              merging registers (intersect) with existing edge
              mov ecx,-4(ebp)                
              mov eax,2658520                
              mov esp,ebp                    
        0x292f83:
              jmp 0x291ff9                   
--------------------------------------- end exit block 0x289124
    i2f2 = i2f add1
    st gp[656] = add1
    ld8 = ld cx[152]
    ld9 = ld ld8[60]
    sti sp[40] = PCVAL_TO_OBJECT(pcval)
    sti sp[48] = ld9
    sti sp[56] = ATOM_TO_STRING(atom)
    sti sp[64] = add1
    js_NumberToString1 = js_NumberToString ( cx i2f2 )
    eq1 = eq js_NumberToString1, 0
    xt2: xt eq1 -> 0:100 sp+72 rp+0
              cvtsi2sd xmm0,eax               eax(add1) ecx(gp) ebx($stack3) esi(sp) edi(cx)
              mov 656(ecx),eax                eax(add1) ecx(gp) ebx($stack3) esi(sp) edi(cx) xmm0(i2f2)

At the very end you see that it converts the value (eax) to a double in xmm0, but then just stores the int eax anyway. (xmm0 is used later on down to convert to a string and print out, so it's not dead here even in this wrong code.) 

I don't know where the code is that does the bad thing. I thought it would be in record_SetPropHit because that's where JSOP_SETNAME seems to be handled, but in fact the debug printouts for this LIR don't seem to appear until later. But maybe that's due to a caching effect in the way debug output is generated.
The problem seems to be in js_JoinPeersIfCompatible. This function links (via assm()->patch()) exit A from trace 1 to the header of trace 2 iff the stack typemap at exit A matches the stack typemap at trace 2 header. But this is wrong if the global typemaps don't match.

js_JoinPeersIfCompatible gets the stack typemap for trace 2 header from the TreeInfo. TreeInfo currently has no field for the global type map. Do we need to make TreeInfo hold the full type map? Please advise.
Mapping global types per tree is (IIUC) happening over in bug 469044. We need it for sure.

Dave, you willing to own this pending progress in bug 469044?

/be
Depends on: 469044
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b3
(In reply to comment #3) 
> TreeInfo currently has no field for the global type map. Do we need
> to make TreeInfo hold the full type map? Please advise.

Yes, but in the meantime a global typemap change should be flushing the JIT cache.
Assignee: general → danderson

Comment 6

10 years ago
WFM in TM tip (blocking bug 469044 has been fixed).
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME

Comment 8

10 years ago
http://hg.mozilla.org/mozilla-central/rev/e9226dd6c073
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.