The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: gkw, Assigned: Igor Bukanov)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Mac OS X
assertion, fixed1.9.1.1, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking1.9.2 .7+, status1.9.2 .7-fixed, blocking1.9.1 .11+, status1.9.1 .11-fixed)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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.
(Reporter)

Comment 1

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

Comment 2

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

Comment 3

7 years ago
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.
(Reporter)

Comment 4

7 years ago
(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

Updated

7 years ago
Assignee: general → brendan

Updated

7 years ago
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey, [sg:critical]

Updated

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

Comment 8

7 years ago
I have not made this bug a dup of the bug 560471 since branches would need different one-liner patch.
(Assignee)

Comment 9

7 years ago
Created attachment 442062 [details] [diff] [review]
fix for 192
Attachment #442062 - Flags: review?(brendan)
(Assignee)

Comment 10

7 years ago
Created attachment 442063 [details] [diff] [review]
fix for 192 for real

The previous patch includes unrelated changes
Attachment #442062 - Attachment is obsolete: true
Attachment #442063 - Flags: review?(brendan)
Attachment #442062 - Flags: review?(brendan)
(Assignee)

Comment 11

7 years ago
http://hg.mozilla.org/tracemonkey/rev/22928ec2f2d9 - renaming AutoSaveRestoreWealRoots into AutoPreserveWeakRoots
Attachment #442063 - Flags: review?(brendan) → review+
(Assignee)

Updated

7 years ago
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+

Comment 12

7 years ago
http://hg.mozilla.org/mozilla-central/rev/22928ec2f2d9
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Attachment #442063 - Flags: approval1.9.0.19? → approval1.9.0.next?

Updated

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

Comment 14

7 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/90658753b7da

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f4310a06ef2c
status1.9.1: wanted → .11-fixed
status1.9.2: wanted → .6-fixed
Keywords: fixed1.9.1.1
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.