Bug 1484382 - Use mozilla::ScopeExit in js/src r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Tue, 21 Aug 2018 08:59:31 +0000
changeset 487753 ed07516ab49e79cdfaf85785bc8f572aee1de55a
parent 487752 20a3b24e500abc7bf403466c8beb8c38abbaeb6a
child 487754 8d88272387d127eaf0abd274bdba7dd5787c07bc
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1484382
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1484382 - Use mozilla::ScopeExit in js/src r=jandem Bug 1484382 - Use mozilla::ScopeExit in jit/JitFrames.cpp Bug 1484382 - Use mozilla::ScopeExit in vm/TypeInference.cpp Bug 1484382 - Use mozilla::ScopeExit in jit/JitcodeMap.cpp Bug 1484382 - Use mozilla::ScopeExit in jit/JitFrames.cpp Differential Revision: https://phabricator.services.mozilla.com/D3685
js/src/jit/JitFrames.cpp
js/src/jit/JitcodeMap.cpp
js/src/jit/JitcodeMap.h
js/src/proxy/CrossCompartmentWrapper.cpp
js/src/vm/TypeInference.cpp
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -1,16 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jit/JitFrames-inl.h"
 
+#include "mozilla/ScopeExit.h"
+
 #include "jsutil.h"
 
 #include "gc/Marking.h"
 #include "jit/BaselineDebugModeOSR.h"
 #include "jit/BaselineFrame.h"
 #include "jit/BaselineIC.h"
 #include "jit/BaselineJIT.h"
 #include "jit/Ion.h"
@@ -565,63 +567,38 @@ HandleExceptionBaseline(JSContext* cx, c
         frameOk = Debugger::onLeaveFrame(cx, frame.baselineFrame(), pc, frameOk);
     } else if (script->hasTrynotes()) {
         CloseLiveIteratorsBaselineForUncatchableException(cx, frame, pc);
     }
 
     OnLeaveBaselineFrame(cx, frame, pc, rfe, frameOk);
 }
 
-struct AutoDeleteDebugModeOSRInfo
-{
-    BaselineFrame* frame;
-    explicit AutoDeleteDebugModeOSRInfo(BaselineFrame* frame) : frame(frame) { MOZ_ASSERT(frame); }
-    ~AutoDeleteDebugModeOSRInfo() { frame->deleteDebugModeOSRInfo(); }
-};
-
-struct AutoResetLastProfilerFrameOnReturnFromException
-{
-    JSContext* cx;
-    ResumeFromException* rfe;
+static void*
+GetLastProfilingFrame(ResumeFromException* rfe) {
+    switch (rfe->kind) {
+      case ResumeFromException::RESUME_ENTRY_FRAME:
+      case ResumeFromException::RESUME_WASM:
+        return nullptr;
 
-    AutoResetLastProfilerFrameOnReturnFromException(JSContext* cx, ResumeFromException* rfe)
-      : cx(cx), rfe(rfe) {}
+      // The following all return into baseline frames.
+      case ResumeFromException::RESUME_CATCH:
+      case ResumeFromException::RESUME_FINALLY:
+      case ResumeFromException::RESUME_FORCED_RETURN:
+        return rfe->framePointer + BaselineFrame::FramePointerOffset;
 
-    ~AutoResetLastProfilerFrameOnReturnFromException() {
-        if (!cx->runtime()->jitRuntime()->isProfilerInstrumentationEnabled(cx->runtime()))
-            return;
-
-        MOZ_ASSERT(cx->jitActivation == cx->profilingActivation());
-
-        void* lastProfilingFrame = getLastProfilingFrame();
-        cx->jitActivation->setLastProfilingFrame(lastProfilingFrame);
+      // When resuming into a bailed-out ion frame, use the bailout info to
+      // find the frame we are resuming into.
+      case ResumeFromException::RESUME_BAILOUT:
+        return rfe->bailoutInfo->incomingStack;
     }
 
-    void* getLastProfilingFrame() {
-        switch (rfe->kind) {
-          case ResumeFromException::RESUME_ENTRY_FRAME:
-          case ResumeFromException::RESUME_WASM:
-            return nullptr;
-
-          // The following all return into baseline frames.
-          case ResumeFromException::RESUME_CATCH:
-          case ResumeFromException::RESUME_FINALLY:
-          case ResumeFromException::RESUME_FORCED_RETURN:
-            return rfe->framePointer + BaselineFrame::FramePointerOffset;
-
-          // When resuming into a bailed-out ion frame, use the bailout info to
-          // find the frame we are resuming into.
-          case ResumeFromException::RESUME_BAILOUT:
-            return rfe->bailoutInfo->incomingStack;
-        }
-
-        MOZ_CRASH("Invalid ResumeFromException type!");
-        return nullptr;
-    }
-};
+    MOZ_CRASH("Invalid ResumeFromException type!");
+    return nullptr;
+}
 
 void
 HandleExceptionWasm(JSContext* cx, wasm::WasmFrameIter* iter, ResumeFromException* rfe)
 {
     MOZ_ASSERT(cx->activation()->asJit()->hasWasmExitFP());
     rfe->kind = ResumeFromException::RESUME_WASM;
     rfe->framePointer = (uint8_t*) wasm::FailFP;
     rfe->stackPointer = (uint8_t*) wasm::HandleThrow(cx, *iter);
@@ -629,17 +606,25 @@ HandleExceptionWasm(JSContext* cx, wasm:
 }
 
 void
 HandleException(ResumeFromException* rfe)
 {
     JSContext* cx = TlsContext.get();
     TraceLoggerThread* logger = TraceLoggerForCurrentThread(cx);
 
-    AutoResetLastProfilerFrameOnReturnFromException profFrameReset(cx, rfe);
+    auto resetProfilerFrame = mozilla::MakeScopeExit([=] {
+        if (!cx->runtime()->jitRuntime()->isProfilerInstrumentationEnabled(cx->runtime()))
+            return;
+
+        MOZ_ASSERT(cx->jitActivation == cx->profilingActivation());
+
+        void* lastProfilingFrame = GetLastProfilingFrame(rfe);
+        cx->jitActivation->setLastProfilingFrame(lastProfilingFrame);
+    });
 
     rfe->kind = ResumeFromException::RESUME_ENTRY_FRAME;
 
     JitSpew(JitSpew_IonInvalidate, "handling exception");
 
     // Clear any Ion return override that's been set.
     // This may happen if a callVM function causes an invalidation (setting the
     // override), and then fails, bypassing the bailout handlers that would
@@ -751,17 +736,19 @@ HandleException(ResumeFromException* rfe
             AutoBaselineHandlingException handlingException(frame.baselineFrame(), pc);
 
             HandleExceptionBaseline(cx, frame, rfe, pc);
 
             // If we are propagating an exception through a frame with
             // on-stack recompile info, we should free the allocated
             // RecompileInfo struct before we leave this block, as we will not
             // be returning to the recompile handler.
-            AutoDeleteDebugModeOSRInfo deleteDebugModeOSRInfo(frame.baselineFrame());
+            auto deleteDebugModeOSRInfo = mozilla::MakeScopeExit([=] {
+                frame.baselineFrame()->deleteDebugModeOSRInfo();
+            });
 
             if (rfe->kind != ResumeFromException::RESUME_ENTRY_FRAME &&
                 rfe->kind != ResumeFromException::RESUME_FORCED_RETURN)
             {
                 return;
             }
 
             TraceLogStopEvent(logger, TraceLogger_Baseline);
--- a/js/src/jit/JitcodeMap.cpp
+++ b/js/src/jit/JitcodeMap.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jit/JitcodeMap.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/Sprintf.h"
 
 #include "gc/Marking.h"
 #include "gc/Statistics.h"
 #include "jit/BaselineJIT.h"
 #include "jit/JitRealm.h"
 #include "jit/JitSpewer.h"
 #include "js/Vector.h"
@@ -1420,68 +1421,53 @@ JitcodeRegionEntry::findPcOffset(uint32_
         if (queryNativeOffset <= curNativeOffset + nativeDelta)
             break;
         curNativeOffset += nativeDelta;
         curPcOffset += pcDelta;
     }
     return curPcOffset;
 }
 
-typedef js::Vector<char*, 32, SystemAllocPolicy> ProfilingStringVector;
-
-struct AutoFreeProfilingStrings {
-    ProfilingStringVector& profilingStrings_;
-    bool keep_;
-    explicit AutoFreeProfilingStrings(ProfilingStringVector& vec)
-        : profilingStrings_(vec),
-          keep_(false)
-    {}
-
-    void keepStrings() { keep_ = true; }
-
-    ~AutoFreeProfilingStrings() {
-        if (keep_)
-            return;
-        for (size_t i = 0; i < profilingStrings_.length(); i++)
-            js_free(profilingStrings_[i]);
-    }
-};
-
 bool
 JitcodeIonTable::makeIonEntry(JSContext* cx, JitCode* code,
                               uint32_t numScripts, JSScript** scripts,
                               JitcodeGlobalEntry::IonEntry& out)
 {
     typedef JitcodeGlobalEntry::IonEntry::SizedScriptList SizedScriptList;
 
     MOZ_ASSERT(numScripts > 0);
 
     // Create profiling strings for script, within vector.
     typedef js::Vector<char*, 32, SystemAllocPolicy> ProfilingStringVector;
 
     ProfilingStringVector profilingStrings;
     if (!profilingStrings.reserve(numScripts))
         return false;
 
-    AutoFreeProfilingStrings autoFreeProfilingStrings(profilingStrings);
+    // Cleanup allocations on failure.
+    auto autoFreeProfilingStrings = mozilla::MakeScopeExit([&] {
+        for (auto elem: profilingStrings)
+            js_free(elem);
+    });
+
     for (uint32_t i = 0; i < numScripts; i++) {
         char* str = JitcodeGlobalEntry::createScriptString(cx, scripts[i]);
         if (!str)
             return false;
         if (!profilingStrings.append(str))
             return false;
     }
 
     // Create SizedScriptList
     void* mem = (void*)cx->pod_malloc<uint8_t>(SizedScriptList::AllocSizeFor(numScripts));
     if (!mem)
         return false;
 
-    // Keep allocated profiling strings on destruct.
-    autoFreeProfilingStrings.keepStrings();
+    // Keep allocated profiling strings.
+    autoFreeProfilingStrings.release();
 
     SizedScriptList* scriptList = new (mem) SizedScriptList(numScripts, scripts,
                                                             &profilingStrings[0]);
     out.init(code, code->raw(), code->rawEnd(), scriptList, this);
     return true;
 }
 
 uint32_t
--- a/js/src/jit/JitcodeMap.h
+++ b/js/src/jit/JitcodeMap.h
@@ -139,17 +139,16 @@ class JitcodeGlobalEntry
     JS_STATIC_ASSERT(LIMIT <= 8);
 
     struct BytecodeLocation {
         JSScript* script;
         jsbytecode* pc;
         BytecodeLocation(JSScript* script, jsbytecode* pc) : script(script), pc(pc) {}
     };
     typedef Vector<BytecodeLocation, 0, SystemAllocPolicy> BytecodeLocationVector;
-    typedef Vector<const char*, 0, SystemAllocPolicy> ProfileStringVector;
 
     struct BaseEntry
     {
         static const uint64_t kNoSampleInBuffer = UINT64_MAX;
 
         JitCode* jitcode_;
         void* nativeStartAddr_;
         void* nativeEndAddr_;
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -1,14 +1,16 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "mozilla/ScopeExit.h"
+
 #include "gc/PublicIterators.h"
 #include "js/Wrapper.h"
 #include "proxy/DeadObjectProxy.h"
 #include "vm/Iteration.h"
 #include "vm/WrapperObject.h"
 
 #include "gc/Nursery-inl.h"
 #include "vm/Compartment-inl.h"
@@ -258,72 +260,50 @@ CrossCompartmentWrapper::getOwnEnumerabl
  * allows fast iteration over objects across a compartment boundary.
  */
 static bool
 CanReify(HandleObject obj)
 {
     return obj->is<PropertyIteratorObject>();
 }
 
-struct AutoCloseIterator
+static JSObject*
+Reify(JSContext* cx, JS::Compartment* origin, HandleObject iter)
 {
-    AutoCloseIterator(JSContext* cx, PropertyIteratorObject* obj) : obj(cx, obj) {}
+    // Ensure iterator gets closed.
+    auto autoCloseIterator = mozilla::MakeScopeExit([=] {
+        CloseIterator(iter);
+    });
+
+    NativeIterator* ni = iter->as<PropertyIteratorObject>().getNativeIterator();
+    RootedObject obj(cx, ni->objectBeingIterated());
+
+    // Wrap iteratee.
+    if (!origin->wrap(cx, &obj))
+        return nullptr;
 
-    ~AutoCloseIterator() {
-        if (obj)
-            CloseIterator(obj);
+    // Wrap the elements in the iterator's snapshot.
+    size_t length = ni->numKeys();
+    AutoIdVector keys(cx);
+    if (length > 0) {
+        if (!keys.reserve(length))
+            return nullptr;
+        RootedId id(cx);
+        RootedValue v(cx);
+        for (size_t i = 0; i < length; ++i) {
+            v.setString(ni->propertiesBegin()[i]);
+            if (!ValueToId<CanGC>(cx, v, &id))
+                return nullptr;
+            cx->markId(id);
+            keys.infallibleAppend(id);
+        }
     }
 
-    void clear() { obj = nullptr; }
-
-  private:
-    Rooted<PropertyIteratorObject*> obj;
-};
-
-static JSObject*
-Reify(JSContext* cx, JS::Compartment* origin, HandleObject objp)
-{
-    Rooted<PropertyIteratorObject*> iterObj(cx, &objp->as<PropertyIteratorObject>());
-    NativeIterator* ni = iterObj->getNativeIterator();
-
-    RootedObject obj(cx, ni->objectBeingIterated());
-    {
-        AutoCloseIterator close(cx, iterObj);
-
-        /* Wrap the iteratee. */
-        if (!origin->wrap(cx, &obj))
-            return nullptr;
-
-        /*
-         * Wrap the elements in the iterator's snapshot.
-         * N.B. the order of closing/creating iterators is important due to the
-         * implicit cx->enumerators state.
-         */
-        size_t length = ni->numKeys();
-        AutoIdVector keys(cx);
-        if (length > 0) {
-            if (!keys.reserve(length))
-                return nullptr;
-            RootedId id(cx);
-            RootedValue v(cx);
-            for (size_t i = 0; i < length; ++i) {
-                v.setString(ni->propertiesBegin()[i]);
-                if (!ValueToId<CanGC>(cx, v, &id))
-                    return nullptr;
-                cx->markId(id);
-                keys.infallibleAppend(id);
-            }
-        }
-
-        close.clear();
-        CloseIterator(iterObj);
-
-        obj = EnumeratedIdVectorToIterator(cx, obj, keys);
-    }
-    return obj;
+    // Return iterator in current compartment.
+    return EnumeratedIdVectorToIterator(cx, obj, keys);
 }
 
 JSObject*
 CrossCompartmentWrapper::enumerate(JSContext* cx, HandleObject wrapper) const
 {
     RootedObject res(cx);
     {
         AutoRealm call(cx, wrappedObject(wrapper));
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -6,16 +6,17 @@
 
 #include "vm/TypeInference-inl.h"
 
 #include "mozilla/DebugOnly.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
 #include "mozilla/PodOperations.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/Sprintf.h"
 
 #include <new>
 
 #include "jsapi.h"
 #include "builtin/String.h"
 
 #include "gc/HashUtil.h"
@@ -3813,35 +3814,16 @@ ChangeObjectFixedSlotCount(JSContext* cx
     Shape* newShape = ReshapeForAllocKind(cx, obj->lastProperty(), obj->taggedProto(), allocKind);
     if (!newShape)
         return false;
 
     obj->setLastPropertyShrinkFixedSlots(newShape);
     return true;
 }
 
-namespace {
-
-struct DestroyTypeNewScript
-{
-    JSContext* cx;
-    ObjectGroup* group;
-
-    DestroyTypeNewScript(JSContext* cx, ObjectGroup* group)
-      : cx(cx), group(group)
-    {}
-
-    ~DestroyTypeNewScript() {
-        if (group)
-            group->clearNewScript(cx);
-    }
-};
-
-} // namespace
-
 bool
 TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, bool force)
 {
     // Perform the new script properties analysis if necessary, returning
     // whether the new group table was updated and group needs to be refreshed.
 
     // Make sure there aren't dead references in preliminaryObjects. This can
     // clear out the new script information on OOM.
@@ -3863,17 +3845,20 @@ TypeNewScript::maybeAnalyze(JSContext* c
     // Don't perform the analyses until sufficient preliminary objects have
     // been allocated.
     if (!force && !preliminaryObjects->full())
         return true;
 
     AutoEnterAnalysis enter(cx);
 
     // Any failures after this point will clear out this TypeNewScript.
-    DestroyTypeNewScript destroyNewScript(cx, group);
+    auto destroyNewScript = mozilla::MakeScopeExit([&] {
+        if (group)
+            group->clearNewScript(cx);
+    });
 
     // Compute the greatest common shape prefix and the largest slot span of
     // the preliminary objects.
     Shape* prefixShape = nullptr;
     size_t maxSlotSpan = 0;
     for (size_t i = 0; i < PreliminaryObjectArray::COUNT; i++) {
         JSObject* objBase = preliminaryObjects->get(i);
         if (!objBase)
@@ -3998,17 +3983,17 @@ TypeNewScript::maybeAnalyze(JSContext* c
 
     js_delete(preliminaryObjects);
     preliminaryObjects = nullptr;
 
     if (group->maybeUnboxedLayout(sweep)) {
         // An unboxed layout was constructed for the group, and this has already
         // been hooked into it.
         MOZ_ASSERT(group->unboxedLayout(sweep).newScript() == this);
-        destroyNewScript.group = nullptr;
+        destroyNewScript.release();
 
         // Clear out the template object, which is not used for TypeNewScripts
         // with an unboxed layout. Currently it is a mutant object with a
         // non-native group and native shape, so make it safe for GC by changing
         // its group to the default for its prototype.
         AutoEnterOOMUnsafeRegion oomUnsafe;
         ObjectGroup* plainGroup = ObjectGroup::defaultNewGroup(cx, &PlainObject::class_,
                                                                group->proto());
@@ -4021,17 +4006,17 @@ TypeNewScript::maybeAnalyze(JSContext* c
     }
 
     if (prefixShape->slotSpan() == templateObject()->slotSpan()) {
         // The definite properties analysis found exactly the properties that
         // are held in common by the preliminary objects. No further analysis
         // is needed.
         group->addDefiniteProperties(cx, templateObject()->lastProperty());
 
-        destroyNewScript.group = nullptr;
+        destroyNewScript.release();
         return true;
     }
 
     // There are more properties consistently added to objects of this group
     // than were discovered by the definite properties analysis. Use the
     // existing group to represent fully initialized objects with all
     // definite properties in the prefix shape, and make a new group to
     // represent partially initialized objects.
@@ -4060,17 +4045,17 @@ TypeNewScript::maybeAnalyze(JSContext* c
 
     // prefixShape was read via a weak pointer, so we need a read barrier before
     // we store it into the heap.
     Shape::readBarrier(prefixShape);
 
     initializedShape_ = prefixShape;
     initializedGroup_ = group;
 
-    destroyNewScript.group = nullptr;
+    destroyNewScript.release();
 
     if (regenerate)
         *regenerate = true;
     return true;
 }
 
 bool
 TypeNewScript::rollbackPartiallyInitializedObjects(JSContext* cx, ObjectGroup* group)