Bug 1296642 - Avoid compiling {Read,Write}Sentinel calls in non-sentinel checking builds. r=billm, a=lizzard
authorNathan Froyd <froydnj@gmail.com>
Fri, 19 Aug 2016 21:22:28 -0400
changeset 342444 ffd7ae9b354a6fd5725bab8fdca0e6c6452f26aa
parent 342443 2b7afd7a12b637f6db6acdd78b627a6f7c60389c
child 342445 e66d206c851c7d6410a2eb858e45e4d85c77796c
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, lizzard
bugs1296642
milestone49.0
Bug 1296642 - Avoid compiling {Read,Write}Sentinel calls in non-sentinel checking builds. r=billm, a=lizzard 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
@@ -12,16 +12,20 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/string16.h"
 
 #include "mozilla/Attributes.h"
 #include "mozilla/BufferList.h"
 #include "mozilla/mozalloc.h"
 
+#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;
@@ -104,18 +108,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) {
     return WriteInt(value ? 1 : 0);
@@ -168,18 +178,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.
   };