Bug 1316527 - Return 0 when GetSerialNumber fails to find an existing serial number. r=froydnj
authorAndrew McCreight <continuation@gmail.com>
Wed, 16 Nov 2016 12:38:54 -0800
changeset 323046 811084dd7ec0b2d0049e570d7b6eac66a840ab99
parent 323045 5c0e1535fc906ba614f7092a2a735c33670331a0
child 323047 3560e9d6576b55c26bdbbd4c9aa1ae3f0cc0ecc0
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersfroydnj
bugs1316527, 1309051
milestone53.0a1
Bug 1316527 - Return 0 when GetSerialNumber fails to find an existing serial number. r=froydnj NS_LogCOMPtrAddRef and NS_LogCOMPtrRelease always pass false to GetSerialNumber, because they pass in everything they get without regard to whether it is being logged or not, so they don't want to create a serial number if none exists. This causes the assertions added in bug 1309051 to be hit. To work around this, I hoist the assertion into the other callers of this method. Two of them already had this check, but it was non-fatal. This also makes the asserts not happen in release builds, as I decided it doesn't really matter what happens if somebody tries to use it there.
xpcom/base/nsTraceRefcnt.cpp
--- a/xpcom/base/nsTraceRefcnt.cpp
+++ b/xpcom/base/nsTraceRefcnt.cpp
@@ -569,30 +569,30 @@ LogThisType(const char* aTypeName)
 }
 
 static PLHashNumber
 HashNumber(const void* aKey)
 {
   return PLHashNumber(NS_PTR_TO_INT32(aKey));
 }
 
-// This method uses MOZ_RELEASE_ASSERT in the unlikely event that
-// somebody uses this in a non-debug build.
 static intptr_t
 GetSerialNumber(void* aPtr, bool aCreate)
 {
   PLHashEntry** hep = PL_HashTableRawLookup(gSerialNumbers,
                                             HashNumber(aPtr),
                                             aPtr);
   if (hep && *hep) {
     MOZ_RELEASE_ASSERT(!aCreate, "If an object already has a serial number, we should be destroying it.");
     return static_cast<SerialNumberRecord*>((*hep)->value)->serialNumber;
   }
 
-  MOZ_RELEASE_ASSERT(aCreate, "If an object does not have a serial number, we should be creating it.");
+  if (!aCreate) {
+    return 0;
+  }
 
   SerialNumberRecord* record = new SerialNumberRecord();
   WalkTheStackSavingLocations(record->allocationStack);
   PL_HashTableRawAdd(gSerialNumbers, hep, HashNumber(aPtr),
                      aPtr, static_cast<void*>(record));
   return gNextSerialNumber;
 }
 
@@ -1048,19 +1048,19 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcn
 
     // Here's the case where MOZ_COUNT_CTOR was not used,
     // yet we still want to see creation information:
 
     bool loggingThisType = (!gTypesToLog || LogThisType(aClass));
     intptr_t serialno = 0;
     if (gSerialNumbers && loggingThisType) {
       serialno = GetSerialNumber(aPtr, aRefcnt == 1);
-      NS_ASSERTION(serialno != 0,
-                   "Serial number requested for unrecognized pointer!  "
-                   "Are you memmoving a refcounted object?");
+      MOZ_ASSERT(serialno != 0,
+                 "Serial number requested for unrecognized pointer!  "
+                 "Are you memmoving a refcounted object?");
       int32_t* count = GetRefCount(aPtr);
       if (count) {
         (*count)++;
       }
 
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
@@ -1098,19 +1098,19 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefc
         entry->Dtor();
       }
     }
 
     bool loggingThisType = (!gTypesToLog || LogThisType(aClass));
     intptr_t serialno = 0;
     if (gSerialNumbers && loggingThisType) {
       serialno = GetSerialNumber(aPtr, false);
-      NS_ASSERTION(serialno != 0,
-                   "Serial number requested for unrecognized pointer!  "
-                   "Are you memmoving a refcounted object?");
+      MOZ_ASSERT(serialno != 0,
+                 "Serial number requested for unrecognized pointer!  "
+                 "Are you memmoving a refcounted object?");
       int32_t* count = GetRefCount(aPtr);
       if (count) {
         (*count)--;
       }
 
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
@@ -1157,16 +1157,17 @@ NS_LogCtor(void* aPtr, const char* aType
       entry->Ctor();
     }
   }
 
   bool loggingThisType = (!gTypesToLog || LogThisType(aType));
   intptr_t serialno = 0;
   if (gSerialNumbers && loggingThisType) {
     serialno = GetSerialNumber(aPtr, true);
+    MOZ_ASSERT(serialno != 0, "GetSerialNumber should never return 0 when passed true");
   }
 
   bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
   if (gAllocLog && loggingThisType && loggingThisObject) {
     fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Ctor (%d)\n",
             aType, aPtr, serialno, aInstanceSize);
     WalkTheStackCached(gAllocLog);
   }
@@ -1193,16 +1194,19 @@ NS_LogDtor(void* aPtr, const char* aType
       entry->Dtor();
     }
   }
 
   bool loggingThisType = (!gTypesToLog || LogThisType(aType));
   intptr_t serialno = 0;
   if (gSerialNumbers && loggingThisType) {
     serialno = GetSerialNumber(aPtr, false);
+    MOZ_ASSERT(serialno != 0,
+               "Serial number requested for unrecognized pointer!  "
+               "Are you memmoving a MOZ_COUNT_CTOR-tracked object?");
     RecycleSerialNumberPtr(aPtr);
   }
 
   bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
 
   // (If we're on a losing architecture, don't do this because we'll be
   // using LogDeleteXPCOM instead to get file and line numbers.)
   if (gAllocLog && loggingThisType && loggingThisObject) {