952062 - JS::AutoGCRooter::~AutoGCRooter crash when using union ArrayBufferViewOrBlobOrStringOrFormData

Status: RESOLVED FIXED

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

Description

[Doug Turner (:dougt)]

In bug 936340, I am using the union ArrayBufferViewOrBlobOrStringOrFormData.  For string value types there is no problem, but when I pass an ArrayBufferView or Blob, I see this crash:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
JS::AutoGCRooter::~AutoGCRooter (this=0x7fff5fbfbe30) at jsapi.h:96
96	        JS_ASSERT(this == *stackTop);
(gdb) bt
#0  JS::AutoGCRooter::~AutoGCRooter (this=0x7fff5fbfbe30) at jsapi.h:96
#1  0x0000000101fc3385 in JS::AutoVectorRooter<JS::Value>::~AutoVectorRooter (this=0x7fff5fbfbe30) at jsapi.h:226
#2  0x0000000101fc3321 in JS::AutoValueVector::~AutoValueVector (this=0x7fff5fbfbe30) at jsapi.h:559
#3  0x0000000101fb7285 in JS::AutoValueVector::~AutoValueVector (this=0x7fff5fbfbe30) at jsapi.h:559
#4  0x000000010608105c in js::InvokeArgs::~InvokeArgs (this=0x7fff5fbfbe18) at Stack.h:1090
#5  0x0000000106017e95 in js::InvokeArgs::~InvokeArgs (this=0x7fff5fbfbe18) at Stack.h:1090
#6  0x0000000105fbc4d1 in js::Invoke (cx=0x112ca51c0, thisv=@0x7fff5fbfbf70, fval=@0x7fff5fbfc010, argc=0, argv=0x7fff5fbfc188, rval={<js::MutableHandleBase<JS::Value>> = {<js::MutableValueOperations<JS::MutableHandle<JS::Value> >> = {<js::ValueOperations<JS::MutableHandle<JS::Value> >> = {<No data fields>}, <No data fields>}, <No data fields>}, ptr = 0x7fff5fbfbf90}) at Interpreter.cpp:517
#7  0x0000000105de7aac in JS_CallFunctionValue (cx=0x112ca51c0, objArg=0x127b3d740, fval={data = {asBits = 18445477441275990464, debugView = {payload47 = 4961636800, tag = JSVAL_TAG_OBJECT}, s = {payload = {i32 = 666669504, u32 = 666669504, why = 666669504}}, asDouble = -nan(0xb800127bc91c0), asPtr = 0xfffb800127bc91c0, asWord = 18445477441275990464, asUIntPtr = 18445477441275990464}}, argc=0, argv=0x7fff5fbfc188, rval=0x7fff5fbfc1f8) at jsapi.cpp:4993
#8  0x000000010282eb99 in mozilla::dom::Function::Call (this=0x11890e0a0, cx=0x112ca51c0, aThisObj={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbfc2b8}, arguments=@0x120e42d10, aRv=@0x7fff5fbfc4d8) at FunctionBinding.cpp:35
#9  0x000000010327b8cd in mozilla::dom::Function::Call<nsCOMPtr<nsISupports> > (this=0x11890e0a0, thisObj=@0x7fff5fbfc4f0, arguments=@0x120e42d10, aRv=@0x7fff5fbfc4d8, aExceptionHandling=mozilla::dom::CallbackObject::eReportExceptions) at FunctionBinding.h:54
#10 0x000000010325ca83 in nsGlobalWindow::RunTimeoutHandler (this=0x126d1c000, aTimeout=0x118443480, aScx=0x119977b80) at /Users/dougt/builds/mozilla-central/dom/base/nsGlobalWindow.cpp:11695
#11 0x000000010324cf53 in nsGlobalWindow::RunTimeout (this=0x126d1c000, aTimeout=0x118443480) at /Users/dougt/builds/mozilla-central/dom/base/nsGlobalWindow.cpp:11919
#12 0x000000010325c440 in nsGlobalWindow::TimerCallback (aTimer=0x120ce76a0, aClosure=0x118443480) at /Users/dougt/builds/mozilla-central/dom/base/nsGlobalWindow.cpp:12165
#13 0x0000000101669cf5 in nsTimerImpl::Fire (this=0x120ce76a0) at nsTimerImpl.cpp:551
#14 0x000000010166a101 in nsTimerEvent::Run (this=0x1140da110) at nsTimerImpl.cpp:635
#15 0x00000001016650b4 in nsThread::ProcessNextEvent (this=0x10032a360, mayWait=false, result=0x7fff5fbfcaf3) at nsThread.cpp:634
#16 0x000000010156e66b in NS_ProcessPendingEvents (thread=0x10032a360, timeout=20) at nsThreadUtils.cpp:210
#17 0x0000000102fcc51f in nsBaseAppShell::NativeEventCallback (this=0x110149a60) at nsBaseAppShell.cpp:95
#18 0x0000000102f59a0c in nsAppShell::ProcessGeckoEvents (aInfo=0x110149a60) at nsAppShell.mm:388
#19 0x00007fff88efb8f1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#20 0x00007fff88eed062 in __CFRunLoopDoSources0 ()
#21 0x00007fff88eec7ef in __CFRunLoopRun ()
#22 0x00007fff88eec275 in CFRunLoopRunSpecific ()
#23 0x00007fff8ba7af0d in RunCurrentEventLoopInMode ()
#24 0x00007fff8ba7acb7 in ReceiveNextEventCommon ()
#25 0x00007fff8ba7aabc in _BlockUntilNextEventMatchingListInModeWithFilter ()
#26 0x00007fff8950028e in _DPSNextEvent ()
#27 0x00007fff894ff8db in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#28 0x0000000102f58947 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x10031a980, _cmd=0x7fff89f317f3, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff780d4cd0, flag=1 '\001') at nsAppShell.mm:165
#29 0x00007fff894f39cc in -[NSApplication run] ()
#30 0x0000000102f5a4e1 in nsAppShell::Run (this=0x110149a60) at nsAppShell.mm:742
#31 0x0000000104bdec0c in nsAppStartup::Run (this=0x1107427e0) at nsAppStartup.cpp:268
#32 0x0000000104a9a5f9 in XREMain::XRE_mainRun (this=0x7fff5fbfec00) at /Users/dougt/builds/mozilla-central/toolkit/xre/nsAppRunner.cpp:4023
#33 0x0000000104a9adf9 in XREMain::XRE_main (this=0x7fff5fbfec00, argc=5, argv=0x7fff5fbff500, aAppData=0x7fff5fbfee98) at /Users/dougt/builds/mozilla-central/toolkit/xre/nsAppRunner.cpp:4091
#34 0x0000000104a9b26d in XRE_main (argc=5, argv=0x7fff5fbff500, aAppData=0x7fff5fbfee98, aFlags=0) at /Users/dougt/builds/mozilla-central/toolkit/xre/nsAppRunner.cpp:4331
#35 0x0000000100002077 in do_main (argc=5, argv=0x7fff5fbff500, xreDirectory=0x100327b00) at /Users/dougt/builds/mozilla-central/browser/app/nsBrowserApp.cpp:280
#36 0x00000001000015b1 in main (argc=5, argv=0x7fff5fbff500) at /Users/dougt/builds/mozilla-central/browser/app/nsBrowserApp.cpp:648
(gdb) 

It is fairly easy to reproduce.  Any ideas as to what is going on?

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We're screwing up a Maybe<RooterThing> somewhere.  Can you attach the generated C++ file.

Comment 2

[Doug Turner (:dougt)]

Attachment: BeaconBinding.cpp

Comment 3

[Doug Turner (:dougt)]

well, that isn't helpful is it?

Comment 4

[Doug Turner (:dougt)]

but that seriously is what is in objdir/dom/bindings/BeaconBinding.cpp

Comment 5

[Doug Turner (:dougt)]

the webidl is:

partial interface Navigator {
  [Throws, Pref="beacon.enabled"]

  void sendBeacon(DOMString url,
                 (ArrayBufferView or Blob or DOMString or FormData)? data);
};

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The interesting part will be the generated "sendBeacon" code in NavigatorBinding.cpp.

Comment 7

[Doug Turner (:dougt)]

static bool
sendBeacon(JSContext* cx, JS::Handle<JSObject*> obj, mozilla::dom::Navigator* self, const JSJitMethodCallArgs& args)
{

  if (args.length() < 2) {
    return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "Navigator.sendBeacon");
  }
  NonNull<nsAString> arg0;
  FakeDependentString arg0_holder;
  if (!ConvertJSValueToString(cx, args[0], args[0], eStringify, eStringify, arg0_holder)) {
    return false;
  }
  arg0 = &arg0_holder;
  Nullable<ArrayBufferViewOrBlobOrStringOrFormData > arg1;
  Maybe<ArrayBufferViewOrBlobOrStringOrFormDataArgument > arg1_holder;
  if (args[1].isNullOrUndefined()) {
    arg1.SetNull();
  } else {
    arg1_holder.construct(arg1.SetValue());
    {
      bool done = false, failed = false, tryNext;
      if (args[1].isObject()) {
        done = (failed = !arg1_holder.ref().TrySetToArrayBufferView(cx, args[1], args[1], tryNext)) || !tryNext ||
               (failed = !arg1_holder.ref().TrySetToBlob(cx, args[1], args[1], tryNext)) || !tryNext ||
               (failed = !arg1_holder.ref().TrySetToFormData(cx, args[1], args[1], tryNext)) || !tryNext;

      }
      if (!done) {
        do {
          done = (failed = !arg1_holder.ref().TrySetToString(cx, args[1], args[1], tryNext)) || !tryNext;
          break;
        } while (0);
      }
      if (failed) {
        return false;
      }
      if (!done) {
        ThrowErrorMessage(cx, MSG_NOT_IN_UNION, "Argument 2 of Navigator.sendBeacon", "ArrayBufferView, Blob, FormData");
        return false;
      }
    }
  }
  ErrorResult rv;
  self->SendBeacon(NonNullHelper(Constify(arg0)), Constify(arg1), rv);
  rv.WouldReportJSException();
  if (rv.Failed()) {
    return ThrowMethodFailedWithDetails(cx, rv, "Navigator", "sendBeacon");
  }
  args.rval().set(JSVAL_VOID);
  return true;
}

Comment 8

[Boris Zbarsky [:bzbarsky]]

This is a very exciting bug.

The TrySetTo* functions construct the corresponding union member.  But then if the set fails non-fatally (as in, the argument is just not of the right type) they move on to the next TrySetTo* function without destroying the relevant union member first.

So if you pass a string to the code above, it will do:

    mUnion.mType = mUnion.eArrayBufferView;
    return mUnion.mValue.mArrayBufferView.SetValue(cx);

where mArrayBufferView is a RootedTypedArray<ArrayBufferView> but then discover that a string is not an ArrayBufferView and move on to constructing mUnion.mValue.mString.  The upshot is that the destructor of mUnion.mValue.mArrayBufferView never gets called.

So far I think we've been lucky and the union members we've had have all had trivial destructors...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix union codegen to properly destroy the union members if conversion to them fails.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix union codegen to properly destroy the union members if conversion to them fails.

Comment 11

[David Zbarsky (:dzbarsky)]

Comment on attachment 8350680 [details] [diff] [review]
Fix union codegen to properly destroy the union members if conversion to them fails.

Review of attachment 8350680 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming try is ok

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f84ac36ce832

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/620d511f837a for introducing OSX debug mochitest bustage: https://tbpl.mozilla.org/php/getParsedLog.php?id=32296676&tree=Mozilla-Inbound

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #13)
OSX was just the first to finish, Windows and Linux are also seeing this.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Ugh.  This was green on try at https://tbpl.mozilla.org/?tree=Try&rev=341fdc3a8f03 but apparently someone checked in more broken code in the meantime.  :(

Comment 16

[Boris Zbarsky [:bzbarsky]]

In particular, it was bug 882299.  And that's been backed out, so going to reland this: https://hg.mozilla.org/integration/mozilla-inbound/rev/321532cf56f8

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/321532cf56f8