Closed Bug 744772 Opened 10 years ago Closed 10 years ago

xhr js::BaseShape::getObjectFlags crash


(Core :: DOM: Core & HTML, defect)

14 Branch
Not set



Tracking Status
firefox13 --- unaffected
firefox14 + fixed
firefox15 --- fixed
firefox-esr10 --- unaffected


(Reporter: mcmanus, Assigned: peterv)



(Keywords: crash, regression, sec-critical, Whiteboard: [sg:critical][advisory-tracking-][qa-])

Crash Data


(1 file, 1 obsolete file)

Using a locally built x64 tree from m-c on 04-11 I get crashes fairly regularly at different points in this slideshow (just going through it a time or two is generally enough):

the crashes look to my unexperienced eye as a dom binding issue with XHR, but I didn't try and really narrow it down.

(gdb) bt
#0  0x00007ffff545e9be in js::BaseShape::getObjectFlags (this=0xdadadadadadadada) at ../../../pipemq/js/src/jsscope.h:352
#1  0x00007ffff54b40be in js::Shape::getObjectFlags (this=0x7fffd993d420) at ../../../pipemq/js/src/jsscope.h:609
#2  0x00007ffff5512e69 in JSObject::setFlag (this=0x7fffd993d200, cx=0x7fffcfe5a400, flag_=4096, generateShape=JSObject::GENERATE_NONE) at ../../../pipemq/js/src/jsscope.cpp:1112
#3  0x00007ffff547aab6 in JSObject::setNewTypeUnknown (this=0x7fffd993d200, cx=0x7fffcfe5a400) at ../../../pipemq/js/src/jsinfer.cpp:5603
#4  0x00007ffff53d8f17 in JS_NewObject (cx=0x7fffcfe5a400, jsclasp=0x7ffff677a160, proto=0x7fffd993d200, parent=0x7fffc9555880) at ../../../pipemq/js/src/jsapi.cpp:3285
#5  0x00007ffff4e3807f in mozilla::dom::bindings::CreateInterfacePrototypeObject (cx=0x7fffcfe5a400, global=0x7fffc9555880, parentProto=0x7fffd993d200, protoClass=0x7ffff677a160, methods=0x0,
    properties=0x7ffff677a320, constants=0x0) at ../../../pipemq/dom/bindings/Utils.cpp:72
#6  0x00007ffff4e3823e in mozilla::dom::bindings::CreateInterfaceObjects (cx=0x7fffcfe5a400, global=0x7fffc9555880, parentProto=0x7fffd993d200, protoClass=0x7ffff677a160, constructorClass=0x0,
    methods=0x0, properties=0x7ffff677a320, constants=0x0, staticMethods=0x0, name=0x0) at ../../../pipemq/dom/bindings/Utils.cpp:110
#7  0x00007ffff4e3556a in mozilla::dom::bindings::prototypes::XMLHttpRequestEventTarget::CreateInterfaceObjects (aCx=0x7fffcfe5a400, aGlobal=0x7fffc9555880)
    at XMLHttpRequestEventTargetBinding.cpp:482
#8  0x00007ffff4e2b137 in mozilla::dom::bindings::prototypes::XMLHttpRequestEventTarget::GetProtoObject (aCx=0x7fffcfe5a400, aGlobal=0x7fffc9555880)
    at ../../dist/include/mozilla/dom/bindings/XMLHttpRequestEventTargetBinding.h:73
#9  0x00007ffff4e2ec7a in mozilla::dom::bindings::prototypes::XMLHttpRequest::CreateInterfaceObjects (aCx=0x7fffcfe5a400, aGlobal=0x7fffc9555880) at XMLHttpRequestBinding.cpp:1282
#10 0x00007ffff4e2b0ab in mozilla::dom::bindings::prototypes::XMLHttpRequest::GetProtoObject (aCx=0x7fffcfe5a400, aGlobal=0x7fffc9555880)
    at ../../dist/include/mozilla/dom/bindings/XMLHttpRequestBinding.h:91
#11 0x00007ffff4e2f0fe in mozilla::dom::bindings::prototypes::XMLHttpRequest::DefineDOMInterface (aCx=0x7fffcfe5a400, aScope=0x7fffccbd6df0, aEnabled=0x7fffffff771e)
    at XMLHttpRequestBinding.cpp:1361
e636439e342f was the tip cset of my build.
Keywords: crash
I went through the slideshow twice without hitting this.
Assignee: nobody → general
Crash Signature: [@ JSObject::setFlag]
Component: DOM: Core & HTML → JavaScript Engine
QA Contact: general → general
OK.  Just reproduced this crash, in a 32-bit debug build.

The lastProperty() of |this| in JSObject::setFlag is 0xdadadada.

The object in question is the proto we passed to JS_NewObject.  In fact, all its fields are 0xdadadada, which I assume means it got GCed.

Peter, that object came out of EventTarget::GetProtoObject.  Are we tracing the proto object array properly?
I tried to reproduce this with a testcase like so:

  window.XMLHttpRequest.prototype = null;"UniversalXPConnect");  
  var xhr = new XMLHttpRequest();

but I don't get a crash there...
Dead pointer, bad mojo.  Presumably this hasn't landed anywhere too awful yet, but as far as I know we customarily hide even temporarily-present security bugs (but we unhide them that much faster after they're fixed, as partial compensation).
Group: core-security
Comment 3 has the right question--this is a regression from bug 740069 caused by the proto array stored in DOM_PROTOTYPE_SLOT not being traced. This is TraceXPCGlobal:

TraceXPCGlobal(JSTracer *trc, JSObject *obj)
#ifdef DEBUG
    if (trc->callback == VerifyTraceXPCGlobalCalled) {
        // We don't do anything here, we only want to verify that TraceXPCGlobal
        // was called.
        reinterpret_cast<VerifyTraceXPCGlobalCalledTracer*>(trc)->ok = true;

    if (XPCWrappedNativeScope *scope = XPCWrappedNativeScope::GetNativeScope(obj))

It wasn't changed by bug 740069 (or anything after that, either).
Assignee: general → nobody
Component: JavaScript Engine → DOM
QA Contact: general → general
What I don't understand is why my testcase from comment 4 didn't crash.  Hard to test a fix if I can't reproduce the crash reliably.  :(
Attached patch v1 (obsolete) — Splinter Review
(In reply to Boris Zbarsky (:bz) from comment #7)
>   window.XMLHttpRequest.prototype = null;

.prototype is { [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false }. This crashes for me:

  delete window.XMLHttpRequestUpload;"UniversalXPConnect");  
  new XMLHttpRequest().upload;
Assignee: nobody → peterv
Attachment #619034 - Flags: review?(bzbarsky)
Comment on attachment 619034 [details] [diff] [review]

r=me, though should we do a chrome test to avoid enablePrivilege?
Attachment #619034 - Flags: review?(bzbarsky) → review+
Attached patch v1.1Splinter Review
With a mochitest.

Turns out there was also a bug in the worker scope's trace, we need to move the trace call inside the check for a real wrapper, so we don't run it for a DedicatedWorkerGlobalScope's prototype object (which uses the same JSClass because worker code calls JS_InitClass).
Attachment #619034 - Attachment is obsolete: true
Attachment #620275 - Flags: review?(bzbarsky)
Comment on attachment 620275 [details] [diff] [review]


We should ban use of JS_InitClass in DOM code.  It never does the right thing.  :(
Attachment #620275 - Flags: review?(bzbarsky) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 620275 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): bug 740069
User impact if declined: crashes
Testing completed (on m-c, etc.): landed on m-c (and has a crashtest)
Risk to taking this patch (and alternatives if risky): low risk, we just started tracing JS objects so that we don't end up with stale pointers to them
String changes made by this patch: None
Attachment #620275 - Flags: approval-mozilla-aurora?
Comment on attachment 620275 [details] [diff] [review]

[Triage Comment]
Low risk crash fix that may become worse after release based upon browsing habits. Approving for Aurora 14.
Attachment #620275 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Since this was never in a shipping build (not even beta) we don't need to keep this hidden.
Group: core-security
Keywords: sec-critical
Whiteboard: [sg:critical]
Crash Signature: [@ JSObject::setFlag] → [@ JSObject::setFlag] [@ JSObject::setFlag(JSContext*, unsigned int, JSObject::GenerateShape)]
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [sg:critical] → [sg:critical][advisory-tracking-]
Depends on: 765034
Whiteboard: [sg:critical][advisory-tracking-] → [sg:critical][advisory-tracking-][qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.