Last Comment Bug 561539 - "Assertion failure: isQName(), at ../jsobjinlines.h" or "Assertion failure: obj->getClass() == &js_NamespaceClass.base || IsQNameClass(obj->getClass()), at ../jsxml.cpp" with e4x
: "Assertion failure: isQName(), at ../jsobjinlines.h" or "Assertion failure: o...
Status: RESOLVED FIXED
fixed-in-tracemonkey, [sg:critical][c...
: assertion, fixed1.9.1.1, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Igor Bukanov
:
:
Mentors:
Depends on:
Blocks: jsfunfuzz 560471
  Show dependency treegraph
 
Reported: 2010-04-24 02:13 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-03-18 11:17 PDT (History)
10 users (show)
choller: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.7+
.7-fixed
.11+
.11-fixed


Attachments
fix for 192 (1.53 KB, patch)
2010-04-28 05:17 PDT, Igor Bukanov
no flags Details | Diff | Splinter Review
fix for 192 for real (651 bytes, patch)
2010-04-28 05:19 PDT, Igor Bukanov
brendan: review+
dveditz: approval1.9.2.7+
dveditz: approval1.9.1.11+
dveditz: approval1.9.0.next+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2010-04-24 02:13:19 PDT
gczeal(1)
x = <x>></x>
print(x..x)

asserts js debug shell on TM tip without -j at Assertion failure: isQName(), at ../jsobjinlines.h:333

Tested on 64-bit Mac 10.6.3. Setting s-s because this involves gczeal.
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2010-04-24 02:47:07 PDT
This also asserts in 32-bit shells.

Another assert message can be Assertion failure: obj->getClass() == &js_NamespaceClass.base || IsQNameClass(obj->getClass()), at ../jsxml.cpp:161

autoBisect shows this is probably related to bug 560471:

The first bad revision is:
changeset:   41143:2d3e3a6cfba3
user:        Igor Bukanov
date:        Sat Apr 24 00:15:42 2010 +0200
summary:     bug 560471 - remove GC_LAST_DITCH and GC_KEEP_ATOMS. r=jorendorff
Comment 2 Igor Bukanov 2010-04-24 11:32:21 PDT
This bug is sort-of regression from the bug 366975. That bug introduced a protection against wiping out weak roots in the last ditch GC when the END_GC callback allocates new GC things. But that was incomplete fix as in the patch I forgot to restore the weak roots back to the saved values. This was harmless as the browser callback does not allocate XML things.

The bug 560471 has started to use that functionality (now expressed in the form of AutoSaveWeakRoots class) on any path involving the last ditch GC. As such the last ditch GC would wipe out all weak roots after marking a copy. Thus a sequence that was safe previously like allocating XML thing and a string would be no longer a safe one.
Comment 3 Igor Bukanov 2010-04-24 11:43:55 PDT
I have landed the fix for the bug as a followup for the bug 560471. But the rooting problem theoretically could exists on branches even if that is very unlikely. So we should fix this there using a trivial one-line of changes patch.
Comment 4 Gary Kwong [:gkw] [:nth10sd] 2010-04-24 22:10:18 PDT
(In reply to comment #3)
> I have landed the fix for the bug as a followup for the bug 560471. But the
> rooting problem theoretically could exists on branches even if that is very
> unlikely. So we should fix this there using a trivial one-line of changes
> patch.

Landed on TM by Igor.

http://hg.mozilla.org/tracemonkey/rev/8e1084fb0cff

Setting branch flags for nomination...
Comment 5 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-26 10:10:39 PDT
Can we get a roll-up patch either here on on bug 560471 and both bugs nominated if we think we need them on the branches? Thanks!
Comment 6 Brendan Eich [:brendan] 2010-04-26 18:47:25 PDT
Bug 560358 uses "Preserve" as the single verb for what this bug's patch calls "SaveRestore". rs=me on renaming s/SaveRestore/Preserve/g.

/be
Comment 7 Brendan Eich [:brendan] 2010-04-27 21:45:40 PDT
Sayrer, this is Igor's bug 560471, or a dup of it.

/be
Comment 8 Igor Bukanov 2010-04-28 05:13:15 PDT
I have not made this bug a dup of the bug 560471 since branches would need different one-liner patch.
Comment 9 Igor Bukanov 2010-04-28 05:17:03 PDT
Created attachment 442062 [details] [diff] [review]
fix for 192
Comment 10 Igor Bukanov 2010-04-28 05:19:55 PDT
Created attachment 442063 [details] [diff] [review]
fix for 192 for real

The previous patch includes unrelated changes
Comment 11 Igor Bukanov 2010-04-28 05:35:12 PDT
http://hg.mozilla.org/tracemonkey/rev/22928ec2f2d9 - renaming AutoSaveRestoreWealRoots into AutoPreserveWeakRoots
Comment 13 Daniel Veditz [:dveditz] 2010-06-07 10:33:15 PDT
Comment on attachment 442063 [details] [diff] [review]
fix for 192 for real

Approved for 1.9.2.5 and 1.9.1.11, a=dveditz for release-drivers
Comment 15 Carsten Book [:Tomcat] 2010-07-01 14:01:47 PDT
Gary: can i use this testcase somehow in a Debug Build to verify this fix in 1.9.2/1.9.1 ?
Comment 16 Daniel Veditz [:dveditz] 2010-07-22 19:15:34 PDT
Comment on attachment 442063 [details] [diff] [review]
fix for 192 for real

Approved for 1.9.0.20, a=dveditz

Note You need to log in before you can comment on or make changes to this bug.