Bug 1148069 - Set SyncProfiles' buffers to an invalid generation. (r=djvj)
☠☠ backed out by f00797aa6356 ☠ ☠
authorShu-yu Guo <shu@rfrn.org>
Fri, 27 Mar 2015 16:39:25 -0700
changeset 265182 c68a6ebe60838c61b84699e3314d99ca18a6ff2d
parent 265181 654dcfdaccca33ea15ef6aa167ea1c8d0955a578
child 265183 8183fed6b8a2da12231c6ad5e67c3b0c83a46944
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1148069
milestone39.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 1148069 - Set SyncProfiles' buffers to an invalid generation. (r=djvj)
tools/profiler/ProfileEntry.cpp
tools/profiler/ProfileEntry.h
tools/profiler/SyncProfile.cpp
tools/profiler/TableTicker.cpp
--- a/tools/profiler/ProfileEntry.cpp
+++ b/tools/profiler/ProfileEntry.cpp
@@ -96,36 +96,41 @@ void* ProfileEntry::get_tagPtr() {
 
 // END ProfileEntry
 ////////////////////////////////////////////////////////////////////////
 
 
 ////////////////////////////////////////////////////////////////////////
 // BEGIN ProfileBuffer
 
-ProfileBuffer::ProfileBuffer(int aEntrySize)
+ProfileBuffer::ProfileBuffer(int aEntrySize, uint32_t aGeneration)
   : mEntries(MakeUnique<ProfileEntry[]>(aEntrySize))
   , mWritePos(0)
   , mReadPos(0)
   , mEntrySize(aEntrySize)
-  , mGeneration(0)
+  , mGeneration(aGeneration)
 {
 }
 
 ProfileBuffer::~ProfileBuffer()
 {
-  mGeneration = INT_MAX;
+  mGeneration = UINT32_MAX;
   deleteExpiredStoredMarkers();
 }
 
 // Called from signal, call only reentrant functions
 void ProfileBuffer::addTag(const ProfileEntry& aTag)
 {
   mEntries[mWritePos++] = aTag;
   if (mWritePos == mEntrySize) {
+    // Wrapping around may result in things referenced in the buffer (e.g.,
+    // JIT code addresses and markers) being incorrectly collected. 2 is
+    // subtracted to assert that we do not leak stored markers in
+    // ~ProfileBuffer.
+    MOZ_ASSERT(mGeneration != UINT32_MAX - 2);
     mGeneration++;
     mWritePos = 0;
   }
   if (mWritePos == mReadPos) {
     // Keep one slot open.
     mEntries[mReadPos] = ProfileEntry();
     mReadPos = (mReadPos + 1) % mEntrySize;
   }
--- a/tools/profiler/ProfileEntry.h
+++ b/tools/profiler/ProfileEntry.h
@@ -93,17 +93,17 @@ class UniqueJITOptimizations {
   mozilla::Vector<OptimizationKey> mOpts;
   std::map<OptimizationKey, unsigned> mOptToIndexMap;
 };
 
 class ProfileBuffer {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ProfileBuffer)
 
-  explicit ProfileBuffer(int aEntrySize);
+  explicit ProfileBuffer(int aEntrySize, uint32_t aGeneration = 0);
 
   void addTag(const ProfileEntry& aTag);
   void IterateTagsForThread(IterateTagsCallback aCallback, int aThreadId);
   void StreamSamplesToJSObject(JSStreamWriter& b, int aThreadId, JSRuntime* rt,
                                UniqueJITOptimizations& aUniqueOpts);
   void StreamMarkersToJSObject(JSStreamWriter& b, int aThreadId);
   void DuplicateLastSample(int aThreadId);
 
@@ -126,17 +126,17 @@ public:
 
   // Points to the entry at which we can start reading.
   int mReadPos;
 
   // The number of entries in our buffer.
   int mEntrySize;
 
   // How many times mWritePos has wrapped around.
-  int mGeneration;
+  uint32_t mGeneration;
 
   // Markers that marker entries in the buffer might refer to.
   ProfilerMarkerLinkedList mStoredMarkers;
 };
 
 class ThreadProfile
 {
 public:
@@ -173,17 +173,16 @@ public:
   ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
   void SetPendingDelete()
   {
     mPseudoStack = nullptr;
     mPlatformData = nullptr;
   }
 
   uint32_t bufferGeneration() const {
-    MOZ_ASSERT(mBuffer->mGeneration >= 0);
     return mBuffer->mGeneration;
   }
 
 private:
   FRIEND_TEST(ThreadProfile, InsertOneTag);
   FRIEND_TEST(ThreadProfile, InsertOneTagWithTinyBuffer);
   FRIEND_TEST(ThreadProfile, InsertTagsNoWrap);
   FRIEND_TEST(ThreadProfile, InsertTagsWrap);
--- a/tools/profiler/SyncProfile.cpp
+++ b/tools/profiler/SyncProfile.cpp
@@ -3,17 +3,17 @@
 /* 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 "SyncProfile.h"
 #include "UnwinderThread2.h"
 
 SyncProfile::SyncProfile(ThreadInfo* aInfo, int aEntrySize)
-  : ThreadProfile(aInfo, new ProfileBuffer(aEntrySize))
+  : ThreadProfile(aInfo, new ProfileBuffer(aEntrySize, /* aGeneration = */ UINT32_MAX))
   , mOwnerState(REFERENCED)
   , mUtb(nullptr)
 {
   MOZ_COUNT_CTOR(SyncProfile);
 }
 
 SyncProfile::~SyncProfile()
 {
--- a/tools/profiler/TableTicker.cpp
+++ b/tools/profiler/TableTicker.cpp
@@ -630,24 +630,23 @@ void mergeStacksIntoProfile(ThreadProfil
     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
     MOZ_ASSERT(nativeStackAddr);
     MOZ_ASSERT(nativeIndex >= 0);
     aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
     nativeIndex--;
   }
 
-  MOZ_ASSERT(aProfile.bufferGeneration() >= startBufferGen);
-  uint32_t lapCount = aProfile.bufferGeneration() - startBufferGen;
-
   // Update the JS runtime with the current profile sample buffer generation.
   //
   // Do not do this for synchronous sampling, which create their own
   // ProfileBuffers.
   if (!aSample->isSamplingCurrentThread && pseudoStack->mRuntime) {
+    MOZ_ASSERT(aProfile.bufferGeneration() >= startBufferGen);
+    uint32_t lapCount = aProfile.bufferGeneration() - startBufferGen;
     JS::UpdateJSRuntimeProfilerSampleBufferGen(pseudoStack->mRuntime,
                                                aProfile.bufferGeneration(),
                                                lapCount);
   }
 }
 
 #ifdef USE_NS_STACKWALK
 static