Bug 1296642 - Avoid compiling {Read,Write}Sentinel calls in non-sentinel checking builds. r=billm, a=ritu
authorNathan Froyd <froydnj@gmail.com>
Fri, 19 Aug 2016 21:22:28 -0400
changeset 349914 8a0abccd95639178abc9c36aebd9e0c7cf9ea5d5
parent 349913 cb7323bec3e585dbcf1ec19dc636c76dce96c599
child 349915 57ab6cc9d420e4a04b61a364754891f2298f0ae0
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, ritu
bugs1296642
milestone50.0a2
Bug 1296642 - Avoid compiling {Read,Write}Sentinel calls in non-sentinel checking builds. r=billm, a=ritu Pickle::{Read,Write}Sentinel were introduced as a way of catching problems with corrupted IPDL messages at the point of message serialization, rather than the point of use of the bad data. The checking itself is only done on debug or non-release builds, but the calls to the functions are compiled in regardless of whether checking is done. While LTO could plausibly optimize away all the calls, we don't have LTO on all of our platforms, particularly mobile. Therefore, we should move the non-checking versions of the calls inline, so the compiler can eliminate those calls entirely, even in non-LTO builds.
ipc/chromium/src/base/pickle.cc
ipc/chromium/src/base/pickle.h
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -14,20 +14,16 @@
 #include <stdlib.h>
 
 #include <limits>
 #include <string>
 #include <algorithm>
 
 #include "nsDebug.h"
 
-#if !defined(RELEASE_BUILD) || defined(DEBUG)
-#define SENTINEL_CHECKING
-#endif
-
 //------------------------------------------------------------------------------
 
 static_assert(MOZ_ALIGNOF(Pickle::memberAlignmentType) >= MOZ_ALIGNOF(uint32_t),
               "Insufficient alignment");
 
 static const uint32_t kHeaderSegmentCapacity = 64;
 
 static const uint32_t kDefaultSegmentCapacity = 4096;
@@ -432,35 +428,29 @@ bool Pickle::ReadBytesInto(PickleIterato
 
   if (!buffers_.ReadBytes(iter->iter_, reinterpret_cast<char*>(data), length)) {
     return false;
   }
 
   return iter->iter_.AdvanceAcrossSegments(buffers_, AlignInt(length) - length);
 }
 
+#ifdef MOZ_PICKLE_SENTINEL_CHECKING
 bool Pickle::ReadSentinel(PickleIterator* iter, uint32_t sentinel) const {
-#ifdef SENTINEL_CHECKING
   uint32_t found;
   if (!ReadUInt32(iter, &found)) {
     return false;
   }
   return found == sentinel;
-#else
-  return true;
-#endif
 }
 
 bool Pickle::WriteSentinel(uint32_t sentinel) {
-#ifdef SENTINEL_CHECKING
   return WriteUInt32(sentinel);
-#else
-  return true;
+}
 #endif
-}
 
 void Pickle::EndRead(PickleIterator& iter) const {
   DCHECK(iter.iter_.Done());
 }
 
 void Pickle::BeginWrite(uint32_t length, uint32_t alignment) {
   DCHECK(alignment % 4 == 0) << "Must be at least 32-bit aligned!";
 
--- a/ipc/chromium/src/base/pickle.h
+++ b/ipc/chromium/src/base/pickle.h
@@ -17,16 +17,20 @@
 #include "mozilla/BufferList.h"
 #include "mozilla/mozalloc.h"
 
 #ifdef MOZ_FAULTY
 #include "base/singleton.h"
 #include "mozilla/ipc/Faulty.h"
 #endif
 
+#if !defined(RELEASE_BUILD) || defined(DEBUG)
+#define MOZ_PICKLE_SENTINEL_CHECKING
+#endif
+
 class Pickle;
 
 class PickleIterator {
 public:
   explicit PickleIterator(const Pickle& pickle);
 
 private:
   friend class Pickle;
@@ -109,17 +113,24 @@ class Pickle {
   MOZ_MUST_USE bool ReadBytesInto(PickleIterator* iter, void* data, uint32_t length) const;
   MOZ_MUST_USE bool FlattenBytes(PickleIterator* iter, const char** data, uint32_t length,
                                  uint32_t alignment = sizeof(memberAlignmentType));
 
   // Safer version of ReadInt() checks for the result not being negative.
   // Use it for reading the object sizes.
   MOZ_MUST_USE bool ReadLength(PickleIterator* iter, int* result) const;
 
-  MOZ_MUST_USE bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const;
+  MOZ_MUST_USE bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const
+#ifdef MOZ_PICKLE_SENTINEL_CHECKING
+    ;
+#else
+  {
+    return true;
+  }
+#endif
 
   void EndRead(PickleIterator& iter) const;
 
   // Methods for adding to the payload of the Pickle.  These values are
   // appended to the end of the Pickle's payload.  When reading values from a
   // Pickle, it is important to read them in the order in which they were added
   // to the Pickle.
   bool WriteBool(bool value) {
@@ -212,17 +223,24 @@ class Pickle {
     return WriteBytes(&value, sizeof(value));
   }
   bool WriteString(const std::string& value);
   bool WriteWString(const std::wstring& value);
   bool WriteData(const char* data, uint32_t length);
   bool WriteBytes(const void* data, uint32_t data_len,
                   uint32_t alignment = sizeof(memberAlignmentType));
 
-  bool WriteSentinel(uint32_t sentinel);
+  bool WriteSentinel(uint32_t sentinel)
+#ifdef MOZ_PICKLE_SENTINEL_CHECKING
+    ;
+#else
+  {
+    return true;
+  }
+#endif
 
   int32_t* GetInt32PtrForTest(uint32_t offset);
 
   void InputBytes(const char* data, uint32_t length);
 
   // Payload follows after allocation of Header (header size is customizable).
   struct Header {
     uint32_t payload_size;  // Specifies the size of the payload.