Bug 1434965 - Don't store a JSContext pointer in UniqueStacks; pass it manually to the functions that need it. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Sat, 10 Feb 2018 19:17:05 -0500
changeset 754065 8b6e67d8fa47290d272e230aa9ffb51875aad2fb
parent 754064 a19d8f1d55841774fc94778b3f61d6698c6f8fc2
child 754066 850390a5b33080d6291f765feeed127007b6806c
push id98744
push userbmo:mstange@themasta.com
push dateMon, 12 Feb 2018 19:50:57 +0000
reviewersnjn
bugs1434965
milestone60.0a1
Bug 1434965 - Don't store a JSContext pointer in UniqueStacks; pass it manually to the functions that need it. r?njn This resolves the bug where we attempted to find information about JitReturnAddr entries which were collected with a new JSContext by asking an old JSContext. MozReview-Commit-ID: FQrnAuwwzHU
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ProfileBufferEntry.h
tools/profiler/core/ThreadInfo.cpp
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -304,18 +304,17 @@ uint32_t UniqueStacks::FrameKey::Hash() 
     hash = mozilla::AddToHash(hash, *mJITAddress);
     if (mJITDepth.isSome()) {
       hash = mozilla::AddToHash(hash, *mJITDepth);
     }
   }
   return hash;
 }
 
-UniqueStacks::UniqueStacks(JSContext* aContext)
- : mContext(aContext)
+UniqueStacks::UniqueStacks()
 {
   mFrameTableWriter.StartBareList();
   mStackTableWriter.StartBareList();
 }
 
 uint32_t UniqueStacks::GetOrAddStackIndex(const StackKey& aStack)
 {
   uint32_t index;
@@ -326,51 +325,52 @@ uint32_t UniqueStacks::GetOrAddStackInde
 
   index = mStackToIndexMap.Count();
   mStackToIndexMap.Put(aStack, index);
   StreamStack(aStack);
   return index;
 }
 
 MOZ_MUST_USE nsTArray<UniqueStacks::FrameKey>
-UniqueStacks::GetOrAddJITFrameKeysForAddress(void* aJITAddress)
+UniqueStacks::GetOrAddJITFrameKeysForAddress(JSContext* aContext,
+                                             void* aJITAddress)
 {
   nsTArray<FrameKey>& frameKeys =
     *mAddressToJITFrameKeysMap.LookupOrAdd(aJITAddress);
 
   if (frameKeys.IsEmpty()) {
-    MOZ_RELEASE_ASSERT(mContext);
-    for (JS::ProfiledFrameHandle handle : JS::GetProfiledFrames(mContext,
+    for (JS::ProfiledFrameHandle handle : JS::GetProfiledFrames(aContext,
                                                                 aJITAddress)) {
       // JIT frames with the same canonical address should be treated as the
       // same frame, so set the frame key's address to the canonical address.
       FrameKey frameKey(handle.canonicalAddress(), frameKeys.Length());
-      MaybeAddJITFrameIndex(frameKey, handle);
+      MaybeAddJITFrameIndex(aContext, frameKey, handle);
       frameKeys.AppendElement(frameKey);
     }
     MOZ_ASSERT(frameKeys.Length() > 0);
   }
 
   // Return a copy of the array.
   return nsTArray<FrameKey>(frameKeys);
 }
 
 void
-UniqueStacks::MaybeAddJITFrameIndex(const FrameKey& aFrame,
+UniqueStacks::MaybeAddJITFrameIndex(JSContext* aContext,
+                                    const FrameKey& aFrame,
                                     const JS::ProfiledFrameHandle& aJITFrame)
 {
   uint32_t index;
   if (mFrameToIndexMap.Get(aFrame, &index)) {
     MOZ_ASSERT(index < mFrameToIndexMap.Count());
     return;
   }
 
   index = mFrameToIndexMap.Count();
   mFrameToIndexMap.Put(aFrame, index);
-  StreamJITFrame(aJITFrame);
+  StreamJITFrame(aContext, aJITFrame);
 }
 
 uint32_t
 UniqueStacks::GetOrAddFrameIndex(const FrameKey& aFrame)
 {
   uint32_t index;
   if (mFrameToIndexMap.Get(aFrame, &index)) {
     MOZ_ASSERT(index < mFrameToIndexMap.Count());
@@ -520,17 +520,18 @@ StreamJITFrameOptimizations(SpliceableJS
     line = JS_PCToLineNumber(script, pc, &column);
     aWriter.IntProperty("line", line);
     aWriter.IntProperty("column", column);
   }
   aWriter.EndObject();
 }
 
 void
-UniqueStacks::StreamJITFrame(const JS::ProfiledFrameHandle& aJITFrame)
+UniqueStacks::StreamJITFrame(JSContext* aContext,
+                             const JS::ProfiledFrameHandle& aJITFrame)
 {
   enum Schema : uint32_t {
     LOCATION = 0,
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
     CATEGORY = 4
   };
@@ -545,17 +546,17 @@ UniqueStacks::StreamJITFrame(const JS::P
   writer.StringElement(IMPLEMENTATION,
                         frameKind == JS::ProfilingFrameIterator::Frame_Ion
                         ? "ion"
                         : "baseline");
 
   if (aJITFrame.hasTrackedOptimizations()) {
     writer.FreeFormElement(OPTIMIZATIONS,
       [&](SpliceableJSONWriter& aWriter, UniqueJSONStrings& aUniqueStrings) {
-        StreamJITFrameOptimizations(aWriter, aUniqueStrings, mContext,
+        StreamJITFrameOptimizations(aWriter, aUniqueStrings, aContext,
                                     aJITFrame);
       });
   }
 }
 
 struct ProfileSample
 {
   uint32_t mStack;
@@ -868,20 +869,23 @@ ProfileBuffer::StreamSamplesToJSON(Splic
           e.Next();
         }
 
         stack = aUniqueStacks.AppendFrame(stack, frameKey);
 
       } else if (e.Get().IsJitReturnAddr()) {
         numFrames++;
 
+        // We can only process JitReturnAddr entries if we have a JSContext.
+        MOZ_RELEASE_ASSERT(aContext);
+
         // A JIT frame may expand to multiple frames due to inlining.
         void* pc = e.Get().u.mPtr;
         nsTArray<UniqueStacks::FrameKey> frameKeys =
-          aUniqueStacks.GetOrAddJITFrameKeysForAddress(pc);
+          aUniqueStacks.GetOrAddJITFrameKeysForAddress(aContext, pc);
         for (const UniqueStacks::FrameKey& frameKey : frameKeys) {
           stack = aUniqueStacks.AppendFrame(stack, frameKey);
         }
 
         e.Next();
 
       } else {
         break;
--- a/tools/profiler/core/ProfileBufferEntry.h
+++ b/tools/profiler/core/ProfileBufferEntry.h
@@ -213,50 +213,50 @@ public:
     {
       return mHash < aOther.mHash;
     }
 
   private:
     uint32_t mHash;
   };
 
-  explicit UniqueStacks(JSContext* aContext);
+  explicit UniqueStacks();
 
   // Return a StackKey for aFrame as the stack's root frame (no prefix).
   MOZ_MUST_USE StackKey BeginStack(const FrameKey& aFrame);
 
   // Return a new StackKey that is obtained by appending aFrame to aStack.
   MOZ_MUST_USE StackKey AppendFrame(const StackKey& aStack,
                                     const FrameKey& aFrame);
 
   MOZ_MUST_USE nsTArray<FrameKey>
-  GetOrAddJITFrameKeysForAddress(void* aJITAddress);
+  GetOrAddJITFrameKeysForAddress(JSContext* aContext, void* aJITAddress);
 
   MOZ_MUST_USE uint32_t GetOrAddFrameIndex(const FrameKey& aFrame);
   MOZ_MUST_USE uint32_t GetOrAddStackIndex(const StackKey& aStack);
 
   void SpliceFrameTableElements(SpliceableJSONWriter& aWriter);
   void SpliceStackTableElements(SpliceableJSONWriter& aWriter);
 
 private:
   // Make sure that there exists a frame index for aFrame, and if there isn't
   // one already, create one and call StreamJITFrame for the frame.
-  void MaybeAddJITFrameIndex(const FrameKey& aFrame,
+  void MaybeAddJITFrameIndex(JSContext* aContext,
+                             const FrameKey& aFrame,
                              const JS::ProfiledFrameHandle& aJITFrame);
 
   void StreamNonJITFrame(const FrameKey& aFrame);
-  void StreamJITFrame(const JS::ProfiledFrameHandle& aJITFrame);
+  void StreamJITFrame(JSContext* aContext,
+                      const JS::ProfiledFrameHandle& aJITFrame);
   void StreamStack(const StackKey& aStack);
 
 public:
   UniqueJSONStrings mUniqueStrings;
 
 private:
-  JSContext* mContext;
-
   // To avoid incurring JitcodeGlobalTable lookup costs for every JIT frame,
   // we cache the frame keys of frames keyed by JIT code address. All FrameKeys
   // in mAddressToJITFrameKeysMap are guaranteed to be in mFrameToIndexMap.
   nsClassHashtable<nsPtrHashKey<void>, nsTArray<FrameKey>> mAddressToJITFrameKeysMap;
 
   SpliceableChunkedJSONWriter mFrameTableWriter;
   nsDataHashtable<nsGenericHashKey<FrameKey>, uint32_t> mFrameToIndexMap;
 
--- a/tools/profiler/core/ThreadInfo.cpp
+++ b/tools/profiler/core/ThreadInfo.cpp
@@ -76,17 +76,17 @@ void
 ThreadInfo::StreamJSON(const ProfileBuffer& aBuffer,
                        SpliceableJSONWriter& aWriter,
                        const TimeStamp& aProcessStartTime, double aSinceTime)
 {
   UniquePtr<PartialThreadProfile> partialProfile = Move(mPartialProfile);
 
   UniquePtr<UniqueStacks> uniqueStacks = partialProfile
     ? Move(partialProfile->mUniqueStacks)
-    : MakeUnique<UniqueStacks>(mContext);
+    : MakeUnique<UniqueStacks>();
 
   UniquePtr<char[]> partialSamplesJSON;
   UniquePtr<char[]> partialMarkersJSON;
   if (partialProfile) {
     partialSamplesJSON = Move(partialProfile->mSamplesJSON);
     partialMarkersJSON = Move(partialProfile->mMarkersJSON);
   }
 
@@ -242,17 +242,17 @@ ThreadInfo::FlushSamplesAndMarkers(const
   // aWriter.{Start,End}BareList. The result string will be a comma-separated
   // list of JSON object literals that will prepended by StreamJSObject into
   // an existing array.
   //
   // Note that the UniqueStacks instance is persisted so that the frame-index
   // mapping is stable across JS shutdown.
   UniquePtr<UniqueStacks> uniqueStacks = mPartialProfile
     ? Move(mPartialProfile->mUniqueStacks)
-    : MakeUnique<UniqueStacks>(mContext);
+    : MakeUnique<UniqueStacks>();
 
   UniquePtr<char[]> samplesJSON;
   UniquePtr<char[]> markersJSON;
 
   {
     SpliceableChunkedJSONWriter b;
     b.StartBareList();
     bool haveSamples = false;