[compiler-rt] r322941 - [XRay] [compiler-rt] fix heap overflow by computing record pointers correctly

[Martin Pelikan via llvm-commits]

Author: pelikan
Date: Fri Jan 19 05:18:40 2018
New Revision: 322941

URL: http://llvm.org/viewvc/llvm-project?rev=322941&view=rev
Log:
[XRay] [compiler-rt] fix heap overflow by computing record pointers correctly

Summary:
While there, unify InMemoryRawLog and InMemoryRawLogWithArg's coding style:
- swap libc's memcpy(3) for sanitizer's internal memcpy
- use basic pointer arithmetics to compute offsets from the first record
  entry in the pre-allocated buffer, which is always the appropriate type
  for the given function
- lose the local variable references as the TLD.* names fit just as well

Reviewers: eizan, kpw, dberris, dblaikie

Subscribers: #sanitizers, llvm-commits

Differential Revision: https://reviews.llvm.org/D42289

Modified:
    compiler-rt/trunk/lib/xray/xray_inmemory_log.cc

Modified: compiler-rt/trunk/lib/xray/xray_inmemory_log.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/xray/xray_inmemory_log.cc?rev=322941&r1=322940&r2=322941&view=diff
==============================================================================
--- compiler-rt/trunk/lib/xray/xray_inmemory_log.cc (original)
+++ compiler-rt/trunk/lib/xray/xray_inmemory_log.cc Fri Jan 19 05:18:40 2018
@@ -157,7 +157,6 @@ template <class RDTSC>
 void InMemoryRawLog(int32_t FuncId, XRayEntryType Type,
                     RDTSC ReadTSC) XRAY_NEVER_INSTRUMENT {
   auto &TLD = getThreadLocalData();
-  auto &InMemoryBuffer = TLD.InMemoryBuffer;
   int Fd = getGlobalFd();
   if (Fd == -1)
     return;
@@ -240,14 +239,12 @@ void InMemoryRawLog(int32_t FuncId, XRay
   R.TId = TLD.TID;
   R.Type = Type;
   R.FuncId = FuncId;
-  auto EntryPtr = static_cast<char *>(InMemoryBuffer) +
-                  (sizeof(__xray::XRayRecord) * TLD.BufferOffset);
-  __sanitizer::internal_memcpy(EntryPtr, &R, sizeof(R));
+  auto FirstEntry = reinterpret_cast<__xray::XRayRecord *>(TLD.InMemoryBuffer);
+  __sanitizer::internal_memcpy(FirstEntry + TLD.BufferOffset, &R, sizeof(R));
   if (++TLD.BufferOffset == TLD.BufferSize) {
     __sanitizer::SpinMutexLock L(&LogMutex);
-    auto RecordBuffer = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer);
-    retryingWriteAll(Fd, reinterpret_cast<char *>(RecordBuffer),
-                     reinterpret_cast<char *>(RecordBuffer + TLD.BufferOffset));
+    retryingWriteAll(Fd, reinterpret_cast<char *>(FirstEntry),
+                     reinterpret_cast<char *>(FirstEntry + TLD.BufferOffset));
     TLD.BufferOffset = 0;
     TLD.StackEntries = 0;
   }
@@ -257,8 +254,8 @@ template <class RDTSC>
 void InMemoryRawLogWithArg(int32_t FuncId, XRayEntryType Type, uint64_t Arg1,
                            RDTSC ReadTSC) XRAY_NEVER_INSTRUMENT {
   auto &TLD = getThreadLocalData();
-  auto &InMemoryBuffer = TLD.InMemoryBuffer;
-  auto &Offset = TLD.BufferOffset;
+  auto FirstEntry =
+      reinterpret_cast<__xray::XRayArgPayload *>(TLD.InMemoryBuffer);
   const auto &BuffLen = TLD.BufferSize;
   int Fd = getGlobalFd();
   if (Fd == -1)
@@ -267,12 +264,11 @@ void InMemoryRawLogWithArg(int32_t FuncI
   // First we check whether there's enough space to write the data consecutively
   // in the thread-local buffer. If not, we first flush the buffer before
   // attempting to write the two records that must be consecutive.
-  if (Offset + 2 > BuffLen) {
+  if (TLD.BufferOffset + 2 > BuffLen) {
     __sanitizer::SpinMutexLock L(&LogMutex);
-    auto RecordBuffer = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer);
-    retryingWriteAll(Fd, reinterpret_cast<char *>(RecordBuffer),
-                     reinterpret_cast<char *>(RecordBuffer + Offset));
-    Offset = 0;
+    retryingWriteAll(Fd, reinterpret_cast<char *>(FirstEntry),
+                     reinterpret_cast<char *>(FirstEntry + TLD.BufferOffset));
+    TLD.BufferOffset = 0;
     TLD.StackEntries = 0;
   }
 
@@ -290,15 +286,12 @@ void InMemoryRawLogWithArg(int32_t FuncI
   R.FuncId = FuncId;
   R.TId = TLD.TID;
   R.Arg = Arg1;
-  auto EntryPtr =
-      &reinterpret_cast<__xray::XRayArgPayload *>(&InMemoryBuffer)[Offset];
-  std::memcpy(EntryPtr, &R, sizeof(R));
-  if (++Offset == BuffLen) {
+  __sanitizer::internal_memcpy(FirstEntry + TLD.BufferOffset, &R, sizeof(R));
+  if (++TLD.BufferOffset == BuffLen) {
     __sanitizer::SpinMutexLock L(&LogMutex);
-    auto RecordBuffer = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer);
-    retryingWriteAll(Fd, reinterpret_cast<char *>(RecordBuffer),
-                     reinterpret_cast<char *>(RecordBuffer + Offset));
-    Offset = 0;
+    retryingWriteAll(Fd, reinterpret_cast<char *>(FirstEntry),
+                     reinterpret_cast<char *>(FirstEntry + TLD.BufferOffset));
+    TLD.BufferOffset = 0;
     TLD.StackEntries = 0;
   }
 }