Bogus hazard in AutoJSContext::AutoJSContext

RESOLVED FIXED in mozilla29

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

unspecified
mozilla29
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

Hazard:

Function 'uint8 nsScriptSecurityManager::ScriptAllowed(JSObject*)' has unrooted 'aGlobal' of type 'JSObject*' live across GC call 'void mozilla::AutoJSContext::AutoJSContext(mozilla::detail::GuardObjectNotifier*)' at caps/src/nsScriptSecurityManager.cpp:1605
GC Function: void mozilla::AutoJSContext::AutoJSContext(mozilla::detail::GuardObjectNotifier*)
    mozilla::Maybe<T>::~Maybe() [with T = mozilla::AutoCxPusher]
    void mozilla::AutoCxPusher::~AutoCxPusher(int32)
    JSContext* XPCJSContextStack::Pop()
    void JS_RestoreFrameChain(JSContext*)
    void JSContext::restoreFrameChain()
    void JSContext::wrapPendingException()
    uint8 JSCompartment::wrap(JSContext*, class JS::MutableHandle<JS::Value>, class JS::Handle<JSObject*
>)
    uint8 JSCompartment::wrap(JSContext*, JSString**)
    JSFlatString* js_NewStringCopyN(js::ExclusiveContext*, uint16*, uint64) [with js::AllowGC allowGC = (js::AllowGC)1u; jschar = char16_t; size_t = long unsigned int]
    ...

The question is, why would the AutoJSContext constructor call the *destructor* of Maybe? That call does not show up in the generated code (as disassembled by gdb.)

A portion of the gimple (from -fdump-tree-gimple) is:

mozilla::AutoJSContext::AutoJSContext(const mozilla::detail::GuardObjectNotifier&) (struct AutoJSContext * const this, const struct GuardObjectNotifier & _notifier)
{
  struct Maybe * D.311281;
  struct GuardObjectNotificationReceiver * D.311282;
  struct GuardObjectNotificationReceiver * D.311283;
  struct Maybe * D.311284;

  this->mCx = 0B;
  D.311281 = &this->mPusher;
  mozilla::Maybe<mozilla::AutoCxPusher>::Maybe (D.311281);
  try
    {
      D.311282 = &this->_mCheckNotUsedAsTemporary;
      mozilla::detail::GuardObjectNotificationReceiver::GuardObjectNotificationReceiver (D.311282);
      try
        {
          mozilla::AutoJSContext::Init (this, 0, _notifier);
        }
      catch
        {
          D.311283 = &this->_mCheckNotUsedAsTemporary;
          mozilla::detail::GuardObjectNotificationReceiver::~GuardObjectNotificationReceiver (D.311283);
        }
    }
  catch
    {
      D.311284 = &this->mPusher;
      mozilla::Maybe<mozilla::AutoCxPusher>::~Maybe (D.311284);
    }
}

I don't know if the gimple try/catch stuff is for C++ exceptions, and it emits it even with you compile with -fno-exceptions, or if it is for some internal use, but it really does seem to be considering a case where AutoJSContext::Init "fails" in some way, so that it would need to call ~Maybe.
Blocks: 898606
Created attachment 8345622 [details] [diff] [review]
AutoJSContext cannot GC

Sadly, it appears to me that AutoPushJSContext really can GC. But we can still annotate the other two.
Attachment #8345622 - Flags: review?(terrence)
Assignee: nobody → sphink
Comment on attachment 8345622 [details] [diff] [review]
AutoJSContext cannot GC

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

r=me
Attachment #8345622 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/907b38d40e79
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.