Last Comment Bug 742222 - Execution of request.onreadystatechange crashes Firefox
: Execution of request.onreadystatechange crashes Firefox
Status: VERIFIED FIXED
: crash, regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Windows Vista
: -- critical (vote)
: mozilla14
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 740069
  Show dependency treegraph
 
Reported: 2012-04-03 23:30 PDT by Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08
Modified: 2012-04-10 04:07 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Firebug test build (1.68 MB, application/x-xpinstall)
2012-04-03 23:30 PDT, Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08
no flags Details
Make sure to JS_WrapValue values being returned for callback types and 'any' types. (1.27 KB, patch)
2012-04-04 07:46 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review

Description Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08 2012-04-03 23:30:51 PDT
Created attachment 612113 [details]
Firebug test build

This report is based on the following thread:
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/49c2d30c38c40504/81252a5b7b76b655?hl=en#81252a5b7b76b655


Firebug is overriding <xhr-request>.onreadystatechange and when the
callback is executed it calls the original handler, see here: 

https://github.com/firebug/firebug/blob/572bc4883aa895b2bcac53e555e6f6434aa11ec1/extension/content/firebug/net/spy.js#L819

onreadystatechange is now just a Function (since Fx14) so, it's executed using 'call':

var originalHandler = request.onreadystatechange;
originalHandler.call(request, event);   // <- causes crash

Execution of the second line causes Firefox crash.

Crash Report:
https://crash-stats.mozilla.com/report/index/bp-2fcfa090-a26b-43d6-bbcc-45aef2120403

The first Nightly where the bug appeared:
http://hg.mozilla.org/mozilla-central/rev/3e46009daea3

It still appears in today's Nightly:
http://hg.mozilla.org/mozilla-central/rev/c410b2d6d570

STR:

1) Install attached Firebug build
2) Enable the Console panel
3) Load www.google.com
4) Type something into the search box -> CRASH

Honza
Comment 1 Boris Zbarsky [:bz] 2012-04-03 23:47:02 PDT
Peter, are we possibly ending up returning the content function to chrome code without putting a cross-compartment wrapper on it?
Comment 2 Boris Zbarsky [:bz] 2012-04-04 07:35:52 PDT
Yup.  In a debug build, I get:

Assertion failure: compartment mismatched, at ../../../mozilla/js/src/jscntxtinlines.h:156

(gdb) frame 7
#7  0x135c8876 in js::CallJSNative (cx=0x22c529d0, native=0x16179ec0 <get_onreadystatechange>, args=@0xbfff65b4) at jscntxtinlines.h:316
316             assertSameCompartment(cx, args.rval());
Comment 3 Boris Zbarsky [:bz] 2012-04-04 07:46:26 PDT
Created attachment 612189 [details] [diff] [review]
Make sure to JS_WrapValue values being returned for callback types and 'any' types.
Comment 4 Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08 2012-04-04 23:15:52 PDT
Perhaps this one is related?
Bug 742631 - Crash [@ js::ContextStack::currentScriptWithDiagnostics(unsigned char**)] 

Honza
Comment 5 Peter Van der Beken [:peterv] - away till Aug 1st 2012-04-05 05:15:16 PDT
Comment on attachment 612189 [details] [diff] [review]
Make sure to JS_WrapValue values being returned for callback types and 'any' types.

Would be nice to have a test.
Comment 6 Boris Zbarsky [:bz] 2012-04-05 10:17:16 PDT
http://hg.mozilla.org/mozilla-central/rev/1907c8bf3cae with test.
Comment 8 Matt Brubeck (:mbrubeck) 2012-04-06 11:31:43 PDT
https://hg.mozilla.org/mozilla-central/rev/1907c8bf3cae
Comment 9 Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08 2012-04-10 04:07:19 PDT
My STR from comment #0 is fixed in
http://hg.mozilla.org/mozilla-central/rev/9ca66ce2672f

Thanks!

Honza

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