128057 - JS_GetPropertyDescArray does not check the return value of JS_GetPropertyDesc

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

nsPluginHostImpl::Observe "quit-application"
WEBSHELL- = 5
JS API usage error: the address passed to JS_AddNamedRoot currently holds an
invalid jsval.  This is usually caused by a missing call to JS_RemoveRoot.
The root's name is "JSDValue".
Assertion failure: root_points_to_gcArenaPool, at /home/timeless/mozilla/js/src/jsgc.c:947
Abort trap - core dumped

sorry, no stack, my freebsd system only pretends to dump core.

i was using ./mozilla -venkman, debug>stop on exceptions, tasks>mailnews, run [wallet], run [offline], [run [mypromptservice]], run [smime].

i have a bunch of patches that make mailnews not throw exceptions needlessly, but that's not very interesting.

and then i quit (ctrl-q i think)

i'll try to reproduce and get a stack...

Comment 1

[Robert Ginda]

still waiting for a stack.

Comment 2

[Robert Ginda]

Steps to reproduce...

1. Start venkman.
2. "loadd chrome://venkman/content/venkman-dev.js".
3. "treetest".
4. set a breakpoint in outliner-utils.js, line 1118, which should be the
rowCount getter.
5. click "toggle betty".
6. continue execution.
7. Repeat steps 5 and 6 until assertion (or crash in "random" place in GC if opt
build.)

I can get this to crash usually in less than 10 tries.  I hacked the
root_points_to_gcArenaPool check into jsd_NewValue (in jsd_val.c), and tracked
the problem down as far as JS_GetPropertyDescArray.  JS_GetPropertyDescArray
doesn't check the return value of JS_GetPropertyDesc, which leaves us with an
improperly initialized pd when it fails.  If the garbage in pd happens to test
positive for pd.flags & JSPD_ALIAS, we add a root that points to trash, which
eventually causes the assertion.

I suspect there is a larger issue here, but I'm not sure where to start looking.
Either way, adding the return value test fixes the assertion, and should clear
up most (if not all) of the venkman related talkback incidents.  I'd like to get
this in for 0.9.9 in order to collect further talkback data.

For the record, the object we're getting properties for has a constructor named
"BoxObject" (I suspect that means it is one of hyatt's nsIBoxObjects), and the
property we're failing to look up is called "view".

This is probably a dupe of 129235.

Comment 3

[Robert Ginda]

Attachment: proposed fix

return value check in jsdbgapi.c

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 72831 [details] [diff] [review]
proposed fix

Egad.  r=[e-mail].

/be

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 72831 [details] [diff] [review]
proposed fix

sr=shaver, nice catch.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 72831 [details] [diff] [review]
proposed fix

a=asa (on behalf of drivers) for checkin to the 0.9.9 branch and the 1.0 trunk

Comment 7

[Robert Ginda]

checked in to trunk and branch

Comment 8

[Brendan Eich [:brendan]]

Rob, do you know why the getProperty for "view" from that BoxObject is failing?

/be

Comment 9

[Robert Ginda]

No, I don't.  I had hopes that someone else might have an idea :(

Some possibly interesting datapoints are:
There are 29 properties on the BoxObject, "view" happens to be the last one. 
There is no "view" attribute on nsIBoxObject.

Comment 10

[Robert Ginda]

*** Bug 129481 has been marked as a duplicate of this bug. ***

Comment 11

[Robert Ginda]

The reason the property lookup is failing is due to a security manager veto. 
I'm following up via email.

Comment 12

[Robert Ginda]

*** Bug 129235 has been marked as a duplicate of this bug. ***

Comment 13

[Robert Ginda]

See bug 129503 for the security manager issue.