Closed Bug 561539 Opened 14 years ago Closed 14 years ago

"Assertion failure: isQName(), at ../jsobjinlines.h" or "Assertion failure: obj->getClass() == &js_NamespaceClass.base || IsQNameClass(obj->getClass()), at ../jsxml.cpp" with e4x

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking1.9.2 --- .7+
status1.9.2 --- .7-fixed
blocking1.9.1 --- .11+
status1.9.1 --- .11-fixed

People

(Reporter: gkw, Assigned: igor)

References

Details

(4 keywords, Whiteboard: fixed-in-tracemonkey, [sg:critical][critsmash:resolved])

Attachments

(1 file, 1 obsolete file)

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.
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
Blocks: 560471
Summary: "Assertion failure: isQName(), at ../jsobjinlines.h" with e4x → "Assertion failure: isQName(), at ../jsobjinlines.h" or "Assertion failure: obj->getClass() == &js_NamespaceClass.base || IsQNameClass(obj->getClass()), at ../jsxml.cpp" with e4x
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.
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.
(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...
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Whiteboard: fixed-in-tracemonkey
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!
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
Assignee: general → brendan
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey, [sg:critical]
Whiteboard: fixed-in-tracemonkey, [sg:critical] → fixed-in-tracemonkey, [sg:critical][critsmash:patch]
Sayrer, this is Igor's bug 560471, or a dup of it.

/be
Assignee: brendan → igor
I have not made this bug a dup of the bug 560471 since branches would need different one-liner patch.
Attached patch fix for 192 (obsolete) — Splinter Review
Attachment #442062 - Flags: review?(brendan)
The previous patch includes unrelated changes
Attachment #442062 - Attachment is obsolete: true
Attachment #442063 - Flags: review?(brendan)
Attachment #442062 - Flags: review?(brendan)
http://hg.mozilla.org/tracemonkey/rev/22928ec2f2d9 - renaming AutoSaveRestoreWealRoots into AutoPreserveWeakRoots
Attachment #442063 - Flags: review?(brendan) → review+
Attachment #442063 - Flags: approval1.9.2.5?
Attachment #442063 - Flags: approval1.9.1.11?
Attachment #442063 - Flags: approval1.9.0.19?
blocking1.9.1: ? → .11+
blocking1.9.2: ? → .5+
http://hg.mozilla.org/mozilla-central/rev/22928ec2f2d9
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #442063 - Flags: approval1.9.0.19? → approval1.9.0.next?
Whiteboard: fixed-in-tracemonkey, [sg:critical][critsmash:patch] → fixed-in-tracemonkey, [sg:critical][critsmash:resolved]
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
Attachment #442063 - Flags: approval1.9.2.5?
Attachment #442063 - Flags: approval1.9.2.5+
Attachment #442063 - Flags: approval1.9.1.11?
Attachment #442063 - Flags: approval1.9.1.11+
blocking1.9.2: .5+ → .6+
Attachment #442063 - Flags: approval1.9.2.5+ → approval1.9.2.6+
Gary: can i use this testcase somehow in a Debug Build to verify this fix in 1.9.2/1.9.1 ?
Comment on attachment 442063 [details] [diff] [review]
fix for 192 for real

Approved for 1.9.0.20, a=dveditz
Attachment #442063 - Flags: approval1.9.0.next? → approval1.9.0.next+
Group: core-security
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: