457335 - TM: we fail dom/tests/mochitest/ajax/jquery/test_jQuery.html

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Comment 1

[Jim Blandy :jimb]

Verified tests fail with JIT, pass with no JIT in rev b8dcfe8b5efb (Fri Sep 26 14:37:50 2008 -0400)

not ok - core module: .data() Check for original data expected: testroot actual: 1
not ok - core module: .data() Check for unmatched namespace expected: testroot actual: 1

Comment 2

[Andreas Gal :gal]

Might be a dupe of the test_Prototype failure:

https://bugzilla.mozilla.org/show_bug.cgi?id=457171

Suggest retesting with tip (where test_Prototype is fixed)

Comment 3

[Andreas Gal :gal]

Still failing jquery. I recommend all hands on deck for this one. We need the test suite to be green.

Comment 4

[Andreas Gal :gal]

Attachment: Patch, broken

Comment 5

[Boris Zbarsky [:bzbarsky]]

Do we by chance have a minimal-ish testcase here that's broken?

Comment 6

[Andreas Gal :gal]

We know where it fails. But I can't reproduce the test case in the shell. Help welcome.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Can you get together a minimal-ish browser testcase, smaller than the entirety of text_jQuery?  Or at least let the rest of us in on where in test_jQuery we fail, if you want us to help?  ;)

Comment 8

[Andreas Gal :gal]

Sure, I will post some info in a sec.

Comment 9

[Andreas Gal :gal]

Attachment: Modified jquery

Here is a modified jquery that shows the bug. Load test/index.html in the browser. Only once loop is traced (jquery:2066), and thats the one we don't compile right.

Comment 10

[Andreas Gal :gal]

recording starting from file:///Users/gal/workspace/jquery/dist/jquery.js:2066@143
globalObj=0x1e14a540, shape=24834
    trace

import vp=0xf07abc name=$global0 type=object flags=0
import vp=0xd21920 name=$callee0 type=object flags=0
import vp=0xd21924 name=$this0 type=object flags=0
import vp=0xd21928 name=$<anonymous>.event type=object flags=0
import vp=0xd21934 name=$<anonymous>.val type=boolean flags=0
import vp=0xd21938 name=$<anonymous>.ret type=boolean flags=0
import vp=0xd2193c name=$<anonymous>.namespace type=string flags=0
import vp=0xd21940 name=$<anonymous>.all type=boolean flags=0
import vp=0xd21944 name=$<anonymous>.handlers type=object flags=0
import vp=0xd21948 name=$<anonymous>.j type=string flags=0
import vp=0xd2194c name=$<anonymous>.handler type=object flags=0
import vp=0xd21950 name=$stack0 type=object flags=0
    state = param ecx
    param1 = param edx
    sp = ld state[0]
    rp = ld state[4]
    cx = ld state[12]
    gp = ld state[8]
    eos = ld state[JSVAL_ERROR_COOKIE]
    eor = ld state[20]
    $global0 = ld gp[408]
    $callee0 = ld sp[-80]
    $this0 = ld sp[-72]
    $<anonymous>.event = ld sp[-64]
    $<anonymous>.val = ld sp[-56]
    $<anonymous>.ret = ld sp[-48]
    $<anonymous>.namespace = ld sp[-40]
    $<anonymous>.all = ld sp[-32]
    $<anonymous>.handlers = ld sp[-24]
    $<anonymous>.j = ld sp[-16]
    $<anonymous>.handler = ld sp[-8]
    $stack0 = ld sp[0]
    FastCallIteratorNext1 = FastCallIteratorNext ( cx $stack0 )
    eq1 = eq FastCallIteratorNext1, JSVAL_ERROR_COOKIE
    xt1: xt eq1 -> 0xf0d31b sp+8 rp+0

    eq2 = eq FastCallIteratorNext1, JSVAL_HOLE
    eq3 = eq eq2, 0
    or1 = or $<anonymous>.j, JSVAL_STRING
    cmov1 = eq3 ? FastCallIteratorNext1 : or1
    and1 = and cmov1, JSVAL_TAGMASK
    eq4 = eq and1, JSVAL_STRING
    xf1: xf eq4 -> 0xf0d31b sp+8 rp+0

    and2 = and cmov1, ~JSVAL_TAGMASK
    sp[-16] = and2
    sp[8] = eq3
    eq5 = eq eq3, 1
    eq6 = eq eq5, 0
    xt2: xt eq5 -> 0xf0d31e sp+16 rp+0

    CloseIterator1 = CloseIterator ( cx $stack0 )
    eq7 = eq CloseIterator1, 0
    xt3: xt eq7 -> 0xf0d39c sp+8 rp+0

    sp[0] = $<anonymous>.val
    x1: x  -> 0xf0d3a0 sp+8 rp+0

Comment 11

[Andreas Gal :gal]

Attachment: Gross hack to fix it and get our tinderboxes green.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 341062 [details] [diff] [review]
Gross hack to fix it and get our tinderboxes green.

Toldya that would fix it :-P.

>+#ifdef JS_TRACER
>+                if (TRACE_RECORDER(cx)) {
>+                    js_AbortRecording(cx, regs.pc, "Untraceable iterator loop");

s/iterator/for-in/

r=me thanks!

/be

Comment 13

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/4edd86967da7

Comment 14

[Andreas Gal :gal]

Patch breaks us even worse in jquery. Backing out and will disable for-in until its rewritten in a sane manner.

Comment 15

[Andreas Gal :gal]

We are not jitting for in loops now. Still need better for-in code, but jquery no longer breaks.

Comment 16

[Brendan Eich [:brendan]]

(In reply to comment #15)
> We are not jitting for in loops now.

We are so! :-P

The only thing that changed is we abort a recording of JSOP_FOR* that is not going to iterate at all.

/be

Comment 17

[Bob Clary [:bc] (inactive)]

Is this really "fixed"? checked in and backed out? Or is it works for me?

Comment 18

[Aakash Desai [:aakashd]]

Seeing that there haven't been any duplicate sightings for this issue for 5
months, I'm going to go ahead and verify this unless someone has an issue with
that.