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

VERIFIED FIXED in mozilla1.9.1b1

Status

()

P1
critical
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: gal, Assigned: gal)

Tracking

({verified1.9.1})

unspecified
mozilla1.9.1b1
x86
Mac OS X
verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

10 years ago
Severity: normal → critical
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b1

Comment 1

10 years ago
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
(Assignee)

Comment 2

10 years ago
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)
(Assignee)

Comment 3

10 years ago
Still failing jquery. I recommend all hands on deck for this one. We need the test suite to be green.
(Assignee)

Comment 4

10 years ago
Created attachment 340703 [details] [diff] [review]
Patch, broken
Do we by chance have a minimal-ish testcase here that's broken?
(Assignee)

Comment 6

10 years ago
We know where it fails. But I can't reproduce the test case in the shell. Help welcome.
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?  ;)
(Assignee)

Comment 8

10 years ago
Sure, I will post some info in a sec.
(Assignee)

Updated

10 years ago
Assignee: jim → gal
(Assignee)

Comment 9

10 years ago
Created attachment 340859 [details]
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.
(Assignee)

Comment 10

10 years ago
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
(Assignee)

Comment 11

10 years ago
Created attachment 341062 [details] [diff] [review]
Gross hack to fix it and get our tinderboxes green.
Attachment #340703 - Attachment is obsolete: true
Attachment #341062 - Flags: review?(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
Attachment #341062 - Flags: review?(brendan) → review+
(Assignee)

Comment 14

10 years ago
Patch breaks us even worse in jquery. Backing out and will disable for-in until its rewritten in a sane manner.
(Assignee)

Comment 15

10 years ago
We are not jitting for in loops now. Still need better for-in code, but jquery no longer breaks.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(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

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

Updated

10 years ago
Flags: in-testsuite+
Flags: in-litmus-

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.