Last Comment Bug 697643 - Crash [@ JS_IsExceptionPending] modifying a live NodeList
: Crash [@ JS_IsExceptionPending] modifying a live NodeList
Status: RESOLVED FIXED
: crash, regression, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla10
Assigned To: Peter Van der Beken [:peterv]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 326633 648801
  Show dependency treegraph
 
Reported: 2011-10-26 19:20 PDT by Jesse Ruderman
Modified: 2012-01-05 13:28 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
testcase (crashes Firefox when loaded) (105 bytes, text/html)
2011-10-26 19:20 PDT, Jesse Ruderman
no flags Details
stack trace (debug) (8.74 KB, text/plain)
2011-10-26 19:21 PDT, Jesse Ruderman
no flags Details
v1 (6.85 KB, patch)
2011-10-27 13:56 PDT, Peter Van der Beken [:peterv]
bzbarsky: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-10-26 19:20:33 PDT
Created attachment 569887 [details]
testcase (crashes Firefox when loaded)
Comment 1 Jesse Ruderman 2011-10-26 19:21:20 PDT
Created attachment 569888 [details]
stack trace (debug)

(Opt: bp-e8b730d4-a820-409a-ad90-ccfae2111026)
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-10-26 19:29:56 PDT
HTMLOptionsCollectionWrapper::setItemAt has:

  return NS_SUCCEEDED(rv) ? true : Throw(nsnull, rv);

but Throw() does XPCThrower::Throw(rv, cx); which does JS_IsExceptionPending(cx) which dereferences cx.

Peter, can we just pass in a JSContext to setItemAt?  The only caller seems to have a JSContext.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-10-26 20:10:43 PDT
That's what generates the code in question, yes.
Comment 5 Peter Van der Beken [:peterv] 2011-10-27 13:56:54 PDT
Created attachment 570078 [details] [diff] [review]
v1
Comment 6 Peter Van der Beken [:peterv] 2011-10-28 00:59:26 PDT
Comment on attachment 570078 [details] [diff] [review]
v1

Ideally we'd throw when unwrapping, for that we should probably sprinkle more builtinclass around. But we need to deal correctly with errors from setItemAt anyway.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-10-28 09:52:00 PDT
Comment on attachment 570078 [details] [diff] [review]
v1

r=me
Comment 8 Peter Van der Beken [:peterv] 2011-11-07 09:19:56 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ab4f0ac68dd
Comment 9 Ed Morley [:emorley] 2011-11-07 17:26:42 PST
https://hg.mozilla.org/mozilla-central/rev/7ab4f0ac68dd

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