xhr js::BaseShape::getObjectFlags crash

RESOLVED FIXED in Firefox 14

Status

()

Core
DOM
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mcmanus, Assigned: peterv)

Tracking

({crash, regression, sec-critical})

14 Branch
mozilla15
crash, regression, sec-critical
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13 unaffected, firefox14+ fixed, firefox15 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [sg:critical][advisory-tracking-][qa-], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

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):

http://www.bankrate.com/finance/personal-finance/high-paying-college-majors-1.aspx

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

Updated

5 years ago
Blocks: 740069
e636439e342f was the tip cset of my build.

Updated

5 years ago
Keywords: crash
I went through the slideshow twice without hitting this.

Updated

5 years ago
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?
tracking-firefox14: --- → ?
I tried to reproduce this with a testcase like so:

  window.XMLHttpRequest.prototype = null;
  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");  
  Components.utils.forceGC();
  var xhr = new XMLHttpRequest();

but I don't get a crash there...
status-firefox14: --- → affected
tracking-firefox14: ? → +
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:

void
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;
        return;
    }
#endif

    if (XPCWrappedNativeScope *scope = XPCWrappedNativeScope::GetNativeScope(obj))
        scope->TraceDOMPrototypes(trc);
}

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

Comment 8

5 years ago
Created attachment 619034 [details] [diff] [review]
v1

(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;
  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");  
  Components.utils.forceGC();
  new XMLHttpRequest().upload;
Assignee: nobody → peterv
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #619034 - Flags: review?(bzbarsky)
Comment on attachment 619034 [details] [diff] [review]
v1

r=me, though should we do a chrome test to avoid enablePrivilege?
Attachment #619034 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 10

5 years ago
Created attachment 620275 [details] [diff] [review]
v1.1

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]
v1.1

r=me

We should ban use of JS_InitClass in DOM code.  It never does the right thing.  :(
Attachment #620275 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/17b552b979d5
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/17b552b979d5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox15: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 14

5 years ago
Comment on attachment 620275 [details] [diff] [review]
v1.1

[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]
v1.1

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

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/7d702f45c71a
status-firefox14: affected → fixed
Since this was never in a shipping build (not even beta) we don't need to keep this hidden.
Group: core-security
status-firefox-esr10: --- → unaffected
status-firefox13: --- → unaffected
Keywords: sec-critical
Whiteboard: [sg:critical]

Updated

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

Updated

5 years ago
Whiteboard: [sg:critical][advisory-tracking-] → [sg:critical][advisory-tracking-][qa-]
You need to log in before you can comment on or make changes to this bug.