Bug 1410276 - Add a canary field to nsStringBuffer r=bz

This version of the patch hopefully causes fewer performance regressions.
It might be good to apply this and catch some more assertions.

--HG--
extra : rebase_source : b8674308d581bed4baf6f64a9dd23f2cf995028b

xpcom/string/nsStringBuffer.h

@@ -12,6 +12,22 @@
 
 template<class T> struct already_AddRefed;
 
+/*
+ * Add a canary field to protect against double-frees of nsStringBuffer and
+ * other potential heap corruptions.  We intend to back this out before 58 hits
+ * beta.
+ */
+#if (defined(DEBUG) || defined(NIGHTLY_BUILD)) && !defined(MOZ_ASAN)
+# define STRING_BUFFER_CANARY 1
+#endif
+
+#ifdef STRING_BUFFER_CANARY
+enum nsStringBufferCanary : uint32_t {
+  CANARY_OK = 0xaf57c8fa,
+  CANARY_POISON = 0x534dc0f5
+};
+#endif
+
 /**
  * This structure precedes the string buffers "we" allocate.  It may be the
  * case that nsTAString::mData does not point to one of these special
@@ -29,6 +45,10 @@ private:
   std::atomic<uint32_t> mRefCount;
   uint32_t mStorageSize;
 
+#ifdef STRING_BUFFER_CANARY
+  uint32_t mCanary;
+#endif
+
 public:
 
   /**
@@ -78,7 +98,12 @@ public:
    */
   static nsStringBuffer* FromData(void* aData)
   {
-    return reinterpret_cast<nsStringBuffer*>(aData) - 1;
+    nsStringBuffer* sb = reinterpret_cast<nsStringBuffer*>(aData) - 1;
+#ifdef STRING_BUFFER_CANARY
+    if (MOZ_UNLIKELY(sb->mCanary != CANARY_OK))
+      sb->FromDataCanaryCheckFailed();
+#endif
+    return sb;
   }
 
   /**
@@ -175,6 +200,15 @@ public:
    * double-counting.
    */
   size_t SizeOfIncludingThisEvenIfShared(mozilla::MallocSizeOf aMallocSizeOf) const;
+
+#ifdef STRING_BUFFER_CANARY
+  /*
+   * Called by FromData if the canary check failed.  This is out-of-line in
+   * nsSubstring.cpp so that MOZ_CRASH_UNSAFE_PRINTF is available via #includes.
+   * It is not available in FromData due to #include-order.
+   */
+  void FromDataCanaryCheckFailed() const;
+#endif
 };
 
 #endif /* !defined(nsStringBuffer_h__ */

xpcom/string/nsSubstring.cpp

@@ -20,6 +20,7 @@
 #include "nsString.h"
 #include "nsStringBuffer.h"
 #include "nsDependentString.h"
+#include "nsPrintfCString.h"
 #include "nsMemory.h"
 #include "prprf.h"
 #include "nsStaticAtom.h"
@@ -36,6 +37,19 @@
 #include <unistd.h>
 #endif
 
+#ifdef STRING_BUFFER_CANARY
+#define CHECK_STRING_BUFFER_CANARY(c)                                     \
+  do {                                                                    \
+    if ((c) != CANARY_OK) {                                               \
+      MOZ_CRASH_UNSAFE_PRINTF("Bad canary value %d", c);                  \
+    }                                                                     \
+  } while(0)
+#else
+#define CHECK_STRING_BUFFER_CANARY(c)                                     \
+  do {                                                                    \
+  } while(0)
+#endif
+
 using mozilla::Atomic;
 
 // ---------------------------------------------------------------------------
@@ -205,6 +219,8 @@ nsStringBuffer::AddRef()
 void
 nsStringBuffer::Release()
 {
+  CHECK_STRING_BUFFER_CANARY(mCanary);
+
   // Since this may be the last release on this thread, we need
   // release semantics so that prior writes on this thread are visible
   // to the thread that destroys the object when it reads mValue with
@@ -217,6 +233,9 @@ nsStringBuffer::Release()
     // the last release on other threads, that is, to ensure that
     // writes prior to that release are now visible on this thread.
     count = mRefCount.load(std::memory_order_acquire);
+#ifdef STRING_BUFFER_CANARY
+    mCanary = CANARY_POISON;
+#endif
 
     STRING_STAT_INCREMENT(Free);
     free(this); // we were allocated with |malloc|
@@ -241,6 +260,9 @@ nsStringBuffer::Alloc(size_t aSize)
 
     hdr->mRefCount = 1;
     hdr->mStorageSize = aSize;
+#ifdef STRING_BUFFER_CANARY
+    hdr->mCanary = CANARY_OK;
+#endif
     NS_LOG_ADDREF(hdr, 1, "nsStringBuffer", sizeof(*hdr));
   }
   return dont_AddRef(hdr);
@@ -251,6 +273,7 @@ nsStringBuffer::Realloc(nsStringBuffer* aHdr, size_t aSize)
 {
   STRING_STAT_INCREMENT(Realloc);
 
+  CHECK_STRING_BUFFER_CANARY(aHdr->mCanary);
   NS_ASSERTION(aSize != 0, "zero capacity allocation not allowed");
   NS_ASSERTION(sizeof(nsStringBuffer) + aSize <= size_t(uint32_t(-1)) &&
                sizeof(nsStringBuffer) + aSize > aSize,
@@ -349,6 +372,14 @@ nsStringBuffer::SizeOfIncludingThisEvenIfShared(mozilla::MallocSizeOf aMallocSiz
   return aMallocSizeOf(this);
 }
 
+#ifdef STRING_BUFFER_CANARY
+void
+nsStringBuffer::FromDataCanaryCheckFailed() const
+{
+  MOZ_CRASH_UNSAFE_PRINTF("Bad canary value %d in FromData", mCanary);
+}
+#endif
+
 // ---------------------------------------------------------------------------
 
 // define nsAString