Bug 1301377 - Disallow GC while using ProfilingFrameIterator r=jandem
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 14 Oct 2016 17:13:47 +0100
changeset 318098 5cdbe9d9ea62f4a452fc0a18712003f256124288
parent 318097 af1735c43825bbab57abcfbb5d06e4a09ed92907
child 318099 493604f175c858dc059b28e6496e9fc13bd938b4
push id33211
push usercbook@mozilla.com
push dateMon, 17 Oct 2016 09:38:38 +0000
treeherderautoland@e4ef6fa03aa8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1301377
milestone52.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 1301377 - Disallow GC while using ProfilingFrameIterator r=jandem
js/public/ProfilingFrameIterator.h
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/gc/bug-1301377.js
js/src/vm/Stack.cpp
--- a/js/public/ProfilingFrameIterator.h
+++ b/js/public/ProfilingFrameIterator.h
@@ -6,16 +6,17 @@
 
 #ifndef js_ProfilingFrameIterator_h
 #define js_ProfilingFrameIterator_h
 
 #include "mozilla/Alignment.h"
 #include "mozilla/Maybe.h"
 
 #include "jsbytecode.h"
+#include "js/GCAPI.h"
 #include "js/TypeDecls.h"
 #include "js/Utility.h"
 
 struct JSContext;
 struct JSRuntime;
 class JSScript;
 
 namespace js {
@@ -34,27 +35,33 @@ namespace JS {
 
 struct ForEachTrackedOptimizationAttemptOp;
 struct ForEachTrackedOptimizationTypeInfoOp;
 
 // This iterator can be used to walk the stack of a thread suspended at an
 // arbitrary pc. To provide acurate results, profiling must have been enabled
 // (via EnableRuntimeProfilingStack) before executing the callstack being
 // unwound.
+//
+// Note that the caller must not do anything that could cause GC to happen while
+// the iterator is alive, since this could invalidate Ion code and cause its
+// contents to become out of date.
 class JS_PUBLIC_API(ProfilingFrameIterator)
 {
     JSRuntime* rt_;
     uint32_t sampleBufferGen_;
     js::Activation* activation_;
 
     // When moving past a JitActivation, we need to save the prevJitTop
     // from it to use as the exit-frame pointer when the next caller jit
     // activation (if any) comes around.
     void* savedPrevJitTop_;
 
+    JS::AutoCheckCannotGC nogc_;
+
     static const unsigned StorageSpace = 8 * sizeof(void*);
     mozilla::AlignedStorage<StorageSpace> storage_;
     js::wasm::ProfilingFrameIterator& asmJSIter() {
         MOZ_ASSERT(!done());
         MOZ_ASSERT(isAsmJS());
         return *reinterpret_cast<js::wasm::ProfilingFrameIterator*>(storage_.addr());
     }
     const js::wasm::ProfilingFrameIterator& asmJSIter() const {
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1634,80 +1634,107 @@ ReadSPSProfilingStack(JSContext* cx, uns
 
     // If profiler sampling has been suppressed, return an empty
     // stack.
     if (!cx->runtime()->isProfilerSamplingEnabled()) {
       args.rval().setObject(*stack);
       return true;
     }
 
-    RootedObject inlineStack(cx);
-    RootedObject inlineFrameInfo(cx);
-    RootedString frameKind(cx);
-    RootedString frameLabel(cx);
-    RootedId idx(cx);
+    struct InlineFrameInfo
+    {
+        InlineFrameInfo(const char* kind, UniqueChars&& label)
+          : kind(kind), label(mozilla::Move(label)) {}
+        const char* kind;
+        UniqueChars label;
+    };
+
+    Vector<Vector<InlineFrameInfo, 0, TempAllocPolicy>, 0, TempAllocPolicy> frameInfo(cx);
 
     JS::ProfilingFrameIterator::RegisterState state;
-    uint32_t physicalFrameNo = 0;
-    const unsigned propAttrs = JSPROP_ENUMERATE;
-    for (JS::ProfilingFrameIterator i(cx, state); !i.done(); ++i, ++physicalFrameNo) {
+    for (JS::ProfilingFrameIterator i(cx, state); !i.done(); ++i) {
         MOZ_ASSERT(i.stackAddress() != nullptr);
 
-        // Array holding all inline frames in a single physical jit stack frame.
-        inlineStack = NewDenseEmptyArray(cx);
-        if (!inlineStack)
+        if (!frameInfo.emplaceBack(cx))
             return false;
 
-        JS::ProfilingFrameIterator::Frame frames[16];
-        uint32_t nframes = i.extractStack(frames, 0, 16);
-        for (uint32_t inlineFrameNo = 0; inlineFrameNo < nframes; inlineFrameNo++) {
-
-            // Object holding frame info.
-            inlineFrameInfo = NewBuiltinClassInstance<PlainObject>(cx);
-            if (!inlineFrameInfo)
-                return false;
-
+        const size_t MaxInlineFrames = 16;
+        JS::ProfilingFrameIterator::Frame frames[MaxInlineFrames];
+        uint32_t nframes = i.extractStack(frames, 0, MaxInlineFrames);
+        MOZ_ASSERT(nframes <= MaxInlineFrames);
+        for (uint32_t i = 0; i < nframes; i++) {
             const char* frameKindStr = nullptr;
-            switch (frames[inlineFrameNo].kind) {
+            switch (frames[i].kind) {
               case JS::ProfilingFrameIterator::Frame_Baseline:
                 frameKindStr = "baseline";
                 break;
               case JS::ProfilingFrameIterator::Frame_Ion:
                 frameKindStr = "ion";
                 break;
               case JS::ProfilingFrameIterator::Frame_AsmJS:
                 frameKindStr = "asmjs";
                 break;
               default:
                 frameKindStr = "unknown";
             }
-            frameKind = NewStringCopyZ<CanGC>(cx, frameKindStr);
+
+            if (!frameInfo.back().emplaceBack(frameKindStr, mozilla::Move(frames[i].label)))
+                return false;
+        }
+    }
+
+    RootedObject inlineFrameInfo(cx);
+    RootedString frameKind(cx);
+    RootedString frameLabel(cx);
+    RootedId idx(cx);
+
+    const unsigned propAttrs = JSPROP_ENUMERATE;
+
+    uint32_t physicalFrameNo = 0;
+    for (auto& frame : frameInfo) {
+        // Array holding all inline frames in a single physical jit stack frame.
+        RootedObject inlineStack(cx, NewDenseEmptyArray(cx));
+        if (!inlineStack)
+            return false;
+
+        uint32_t inlineFrameNo = 0;
+        for (auto& inlineFrame : frame) {
+            // Object holding frame info.
+            RootedObject inlineFrameInfo(cx, NewBuiltinClassInstance<PlainObject>(cx));
+            if (!inlineFrameInfo)
+                return false;
+
+            frameKind = NewStringCopyZ<CanGC>(cx, inlineFrame.kind);
             if (!frameKind)
                 return false;
 
             if (!JS_DefineProperty(cx, inlineFrameInfo, "kind", frameKind, propAttrs))
                 return false;
 
-            auto chars = frames[inlineFrameNo].label.release();
+            auto chars = inlineFrame.label.release();
             frameLabel = NewString<CanGC>(cx, reinterpret_cast<Latin1Char*>(chars), strlen(chars));
             if (!frameLabel)
                 return false;
 
             if (!JS_DefineProperty(cx, inlineFrameInfo, "label", frameLabel, propAttrs))
                 return false;
 
             idx = INT_TO_JSID(inlineFrameNo);
             if (!JS_DefinePropertyById(cx, inlineStack, idx, inlineFrameInfo, 0))
                 return false;
+
+            ++inlineFrameNo;
         }
 
         // Push inline array into main array.
         idx = INT_TO_JSID(physicalFrameNo);
         if (!JS_DefinePropertyById(cx, stack, idx, inlineStack, 0))
             return false;
+
+        ++physicalFrameNo;
     }
 
     args.rval().setObject(*stack);
     return true;
 }
 
 static bool
 EnableOsiPointRegisterChecks(JSContext*, unsigned argc, Value* vp)
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1301377.js
@@ -0,0 +1,12 @@
+var lfLogBuffer = `
+  gczeal(14);
+  enableSPSProfiling();
+  gczeal(15,3);
+  var s = "";
+  for (let i = 0; i != 30; i+=2) {}
+  readSPSProfilingStack(s, "c0d1c0d1c0d1c0d1c0d1c0d1c0d1c0");
+`;
+loadFile(lfLogBuffer);
+function loadFile(lfVarx) {
+  evaluate(lfVarx);
+}
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1718,17 +1718,18 @@ ActivationIterator::settle()
         activation_ = activation_->prev();
 }
 
 JS::ProfilingFrameIterator::ProfilingFrameIterator(JSContext* cx, const RegisterState& state,
                                                    uint32_t sampleBufferGen)
   : rt_(cx),
     sampleBufferGen_(sampleBufferGen),
     activation_(nullptr),
-    savedPrevJitTop_(nullptr)
+    savedPrevJitTop_(nullptr),
+    nogc_(cx)
 {
     if (!cx->spsProfiler.enabled())
         MOZ_CRASH("ProfilingFrameIterator called when spsProfiler not enabled for runtime.");
 
     if (!cx->profilingActivation())
         return;
 
     // If profiler sampling is not enabled, skip.