Add poisoning for nsRuleData::mValueOffsets. (Bug 636039, patch 19) r=bzbarsky

I tested manually that after:
 (a) removing the |ruleData.mValueOffsets[aSID] = 0;| in
     nsRuleNode::WalkRuleTree
 (b) removing the NS_ABORT_IF_FALSE(aRuleData->mValueOffsets[aSID] == 0,
     ...) from nsRuleNode::CheckSpecifiedProperties and
     UnsetPropertiesWithoutFlags
that we crash dereferencing the poison address in a SetCoord call inside
nsRuleNode::ComputeTextResetData

layout/base/nsPresArena.cpp

@@ -464,3 +464,9 @@ nsPresArena::FreeByCode(nsQueryFrame::FrameIID aCode, void* aPtr)
 {
   mState->Free(aCode, aPtr);
 }
+
+/* static */ PRUword
+nsPresArena::GetPoisonValue()
+{
+  return ARENA_POISON;
+}

layout/base/nsPresArena.h

@@ -76,6 +76,15 @@ public:
 
   PRUint32 Size();
 
+  /**
+   * Get the poison value that can be used to fill a memory space with
+   * an address that leads to a safe crash when dereferenced.
+   *
+   * The caller is responsible for ensuring that a pres shell has been
+   * initialized before calling this.
+   */
+  static PRUword GetPoisonValue();
+
 private:
   struct State;
   State* mState;

layout/style/nsRuleData.cpp

@@ -38,3 +38,46 @@
 
 #include "nsRuleData.h"
 #include "nsCSSProps.h"
+#include "nsPresArena.h"
+
+inline size_t
+nsRuleData::GetPoisonOffset()
+{
+  // Fill in mValueOffsets such that mValueStorage + mValueOffsets[i]
+  // will yield the frame poison value for all uninitialized value
+  // offsets.
+  PR_STATIC_ASSERT(sizeof(PRUword) == sizeof(size_t));
+  PR_STATIC_ASSERT(PRUword(-1) > PRUword(0));
+  PR_STATIC_ASSERT(size_t(-1) > size_t(0));
+  PRUword framePoisonValue = nsPresArena::GetPoisonValue();
+  return size_t(framePoisonValue - PRUword(mValueStorage)) /
+         sizeof(nsCSSValue);
+}
+
+nsRuleData::nsRuleData(PRUint32 aSIDs, nsCSSValue* aValueStorage,
+                       nsPresContext* aContext, nsStyleContext* aStyleContext)
+  : mSIDs(aSIDs),
+    mCanStoreInRuleTree(PR_TRUE),
+    mPresContext(aContext),
+    mStyleContext(aStyleContext),
+    mPostResolveCallback(nsnull),
+    mValueStorage(aValueStorage)
+{
+  size_t framePoisonOffset = GetPoisonOffset();
+  for (size_t i = 0; i < nsStyleStructID_Length; ++i) {
+    mValueOffsets[i] = framePoisonOffset;
+  }
+}
+
+#ifdef DEBUG
+nsRuleData::~nsRuleData()
+{
+  // assert nothing in mSIDs has poison value
+  size_t framePoisonOffset = GetPoisonOffset();
+  for (size_t i = 0; i < nsStyleStructID_Length; ++i) {
+    NS_ABORT_IF_FALSE(!(mSIDs & (1 << i)) ||
+                      mValueOffsets[i] != framePoisonOffset,
+                      "value in SIDs was left with poison offset");
+  }
+}
+#endif

layout/style/nsRuleData.h

@@ -56,13 +56,13 @@ typedef void (*nsPostResolveFunc)(void* aStyleStruct, nsRuleData* aData);
 
 struct nsRuleData
 {
-  PRUint32 mSIDs;
+  const PRUint32 mSIDs;
   PRPackedBool mCanStoreInRuleTree;
   PRPackedBool mIsImportantRule;
   PRUint8 mLevel; // an nsStyleSet::sheetType
-  nsPresContext* mPresContext;
-  nsStyleContext* mStyleContext;
-  nsPostResolveFunc mPostResolveCallback;
+  nsPresContext* const mPresContext;
+  nsStyleContext* const mStyleContext;
+  const nsPostResolveFunc mPostResolveCallback;
 
   // We store nsCSSValues needed to compute the data for one or more
   // style structs (specified by the bitfield mSIDs).  These are stored
@@ -75,25 +75,17 @@ struct nsRuleData
   // nsRuleNode::HasAuthorSpecifiedRules; therefore some code that we
   // know is not called from HasAuthorSpecifiedRules assumes that the
   // mValueOffsets for the one struct in mSIDs is zero.
-  nsCSSValue* mValueStorage; // our user owns this array
+  nsCSSValue* const mValueStorage; // our user owns this array
   size_t mValueOffsets[nsStyleStructID_Length];
 
-  nsRuleData(PRUint32 aSIDs,
-             nsPresContext* aContext,
-             nsStyleContext* aStyleContext)
-    : mSIDs(aSIDs),
-      mCanStoreInRuleTree(PR_TRUE),
-      mPresContext(aContext),
-      mStyleContext(aStyleContext),
-      mPostResolveCallback(nsnull)
-  {
-    // FIXME: fill with poison value?
-  }
-  ~nsRuleData() {
-  #ifdef DEBUG
-    // FIXME: assert nothing in mSIDs has poison value
-  #endif
-  }
+  nsRuleData(PRUint32 aSIDs, nsCSSValue* aValueStorage,
+             nsPresContext* aContext, nsStyleContext* aStyleContext);
+
+#ifdef DEBUG
+  ~nsRuleData();
+#else
+  ~nsRuleData() {}
+#endif
 
   /**
    * Return a pointer to the value object within |this| corresponding
@@ -161,6 +153,9 @@ struct nsRuleData
   #undef CSS_PROP_DOMPROP_PREFIXED
   #undef CSS_PROP_BACKENDONLY
 
+private:
+  inline size_t GetPoisonOffset();
+
 };
 
 #endif

layout/style/nsRuleNode.cpp

@@ -1728,15 +1728,15 @@ const void*
 nsRuleNode::WalkRuleTree(const nsStyleStructID aSID,
                          nsStyleContext* aContext)
 {
-  nsRuleData ruleData(nsCachedStyleData::GetBitForSID(aSID),
-                      mPresContext, aContext);
   // use placement new[] on the result of alloca() to allocate a
   // variable-sized stack array, including execution of constructors,
   // and use an RAII class to run the destructors too.
   size_t nprops = nsCSSProps::PropertyCountInStruct(aSID);
   void* dataStorage = alloca(nprops * sizeof(nsCSSValue));
   AutoCSSValueArray dataArray(dataStorage, nprops);
-  ruleData.mValueStorage = dataArray.get();
+
+  nsRuleData ruleData(nsCachedStyleData::GetBitForSID(aSID),
+                      dataArray.get(), mPresContext, aContext);
   ruleData.mValueOffsets[aSID] = 0;
 
   // We start at the most specific rule in the tree.
@@ -2943,9 +2943,10 @@ nsRuleNode::SetGenericFont(nsPresContext* aPresContext,
 
   for (PRInt32 i = contextPath.Length() - 1; i >= 0; --i) {
     nsStyleContext* context = contextPath[i];
-    nsRuleData ruleData(NS_STYLE_INHERIT_BIT(Font), aPresContext, context);
     AutoCSSValueArray dataArray(dataStorage, nprops);
-    ruleData.mValueStorage = dataArray.get();
+
+    nsRuleData ruleData(NS_STYLE_INHERIT_BIT(Font), dataArray.get(),
+                        aPresContext, context);
     ruleData.mValueOffsets[eStyleStruct_Font] = 0;
 
     // Trimmed down version of ::WalkRuleTree() to re-apply the style rules
@@ -6483,31 +6484,42 @@ nsRuleNode::HasAuthorSpecifiedRules(nsStyleContext* aStyleContext,
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING)
     inheritBits |= NS_STYLE_INHERIT_BIT(Padding);
 
-  /* We're relying on the use of |aStyleContext| not mutating it! */
-  nsRuleData ruleData(inheritBits,
-                      aStyleContext->PresContext(), aStyleContext);
-
   // properties in the SIDS, whether or not we care about them
-  size_t nprops = 0;
+  size_t nprops = 0, backgroundOffset, borderOffset, paddingOffset;
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) {
-    ruleData.mValueOffsets[eStyleStruct_Background] = nprops;
+    backgroundOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Background);
   }
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) {
-    ruleData.mValueOffsets[eStyleStruct_Border] = nprops;
+    borderOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Border);
   }
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) {
-    ruleData.mValueOffsets[eStyleStruct_Padding] = nprops;
+    paddingOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Padding);
   }
 
   void* dataStorage = alloca(nprops * sizeof(nsCSSValue));
   AutoCSSValueArray dataArray(dataStorage, nprops);
-  ruleData.mValueStorage = dataArray.get();
+
+  /* We're relying on the use of |aStyleContext| not mutating it! */
+  nsRuleData ruleData(inheritBits, dataArray.get(),
+                      aStyleContext->PresContext(), aStyleContext);
+
+  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) {
+    ruleData.mValueOffsets[eStyleStruct_Background] = backgroundOffset;
+  }
+
+  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) {
+    ruleData.mValueOffsets[eStyleStruct_Border] = borderOffset;
+  }
+
+  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) {
+    ruleData.mValueOffsets[eStyleStruct_Padding] = paddingOffset;
+  }
 
   static const nsCSSProperty backgroundValues[] = {
     eCSSProperty_background_color,