[compiler-rt] d19af2f - [GWP-ASan] Crash handler API returns sizeof(collected trace)

[Mitch Phillips via llvm-commits]

Author: Mitch Phillips
Date: 2020-07-27T10:51:55-07:00
New Revision: d19af2f2476b5e13a65d5283cce9859e2c1ef763

URL: https://github.com/llvm/llvm-project/commit/d19af2f2476b5e13a65d5283cce9859e2c1ef763
DIFF: https://github.com/llvm/llvm-project/commit/d19af2f2476b5e13a65d5283cce9859e2c1ef763.diff

LOG: [GWP-ASan] Crash handler API returns sizeof(collected trace)

Summary:
Fix up a slight bug with the crash handler API, where we say that we
return the size of the collected trace (instead of the size of the trace
that's returned) when the return buffer is too small, and the result is
truncated.

Also, as a result, patch up a small uninitialized memory bug.

Reviewers: morehouse, eugenis

Reviewed By: eugenis

Subscribers: #sanitizers

Tags: #sanitizers

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

Added: 
    

Modified: 
    compiler-rt/lib/gwp_asan/common.cpp
    compiler-rt/lib/gwp_asan/crash_handler.cpp
    compiler-rt/lib/gwp_asan/tests/backtrace.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/gwp_asan/common.cpp b/compiler-rt/lib/gwp_asan/common.cpp
index 3438c4b91893..483694d57b7e 100644
--- a/compiler-rt/lib/gwp_asan/common.cpp
+++ b/compiler-rt/lib/gwp_asan/common.cpp
@@ -34,6 +34,9 @@ const char *ErrorToString(const Error &E) {
   __builtin_trap();
 }
 
+constexpr size_t AllocationMetadata::kStackFrameStorageBytes;
+constexpr size_t AllocationMetadata::kMaxTraceLengthToCollect;
+
 void AllocationMetadata::RecordAllocation(uintptr_t AllocAddr,
                                           size_t AllocSize) {
   Addr = AllocAddr;

diff  --git a/compiler-rt/lib/gwp_asan/crash_handler.cpp b/compiler-rt/lib/gwp_asan/crash_handler.cpp
index c3b9e1467bd9..3c640256706f 100644
--- a/compiler-rt/lib/gwp_asan/crash_handler.cpp
+++ b/compiler-rt/lib/gwp_asan/crash_handler.cpp
@@ -10,6 +10,7 @@
 #include "gwp_asan/stack_trace_compressor.h"
 
 #include <assert.h>
+#include <string.h>
 
 using AllocationMetadata = gwp_asan::AllocationMetadata;
 using Error = gwp_asan::Error;
@@ -112,9 +113,15 @@ uint64_t __gwp_asan_get_allocation_thread_id(
 size_t __gwp_asan_get_allocation_trace(
     const gwp_asan::AllocationMetadata *AllocationMeta, uintptr_t *Buffer,
     size_t BufferLen) {
-  return gwp_asan::compression::unpack(
+  uintptr_t UncompressedBuffer[AllocationMetadata::kMaxTraceLengthToCollect];
+  size_t UnpackedLength = gwp_asan::compression::unpack(
       AllocationMeta->AllocationTrace.CompressedTrace,
-      AllocationMeta->AllocationTrace.TraceSize, Buffer, BufferLen);
+      AllocationMeta->AllocationTrace.TraceSize, UncompressedBuffer,
+      AllocationMetadata::kMaxTraceLengthToCollect);
+  if (UnpackedLength < BufferLen)
+    BufferLen = UnpackedLength;
+  memcpy(Buffer, UncompressedBuffer, BufferLen * sizeof(*Buffer));
+  return UnpackedLength;
 }
 
 bool __gwp_asan_is_deallocated(
@@ -130,9 +137,15 @@ uint64_t __gwp_asan_get_deallocation_thread_id(
 size_t __gwp_asan_get_deallocation_trace(
     const gwp_asan::AllocationMetadata *AllocationMeta, uintptr_t *Buffer,
     size_t BufferLen) {
-  return gwp_asan::compression::unpack(
+  uintptr_t UncompressedBuffer[AllocationMetadata::kMaxTraceLengthToCollect];
+  size_t UnpackedLength = gwp_asan::compression::unpack(
       AllocationMeta->DeallocationTrace.CompressedTrace,
-      AllocationMeta->DeallocationTrace.TraceSize, Buffer, BufferLen);
+      AllocationMeta->DeallocationTrace.TraceSize, UncompressedBuffer,
+      AllocationMetadata::kMaxTraceLengthToCollect);
+  if (UnpackedLength < BufferLen)
+    BufferLen = UnpackedLength;
+  memcpy(Buffer, UncompressedBuffer, BufferLen * sizeof(*Buffer));
+  return UnpackedLength;
 }
 
 #ifdef __cplusplus

diff  --git a/compiler-rt/lib/gwp_asan/tests/backtrace.cpp b/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
index b3d44270bb2a..95150653ff61 100644
--- a/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
+++ b/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
@@ -8,6 +8,7 @@
 
 #include <string>
 
+#include "gwp_asan/common.h"
 #include "gwp_asan/crash_handler.h"
 #include "gwp_asan/tests/harness.h"
 
@@ -76,9 +77,46 @@ TEST(Backtrace, Short) {
 TEST(Backtrace, ExceedsStorableLength) {
   gwp_asan::AllocationMetadata Meta;
   Meta.AllocationTrace.RecordBacktrace(
-      [](uintptr_t * /* TraceBuffer */, size_t /* Size */) -> size_t {
-        return SIZE_MAX; // Wow, that's big!
+      [](uintptr_t *TraceBuffer, size_t Size) -> size_t {
+        // Need to inintialise the elements that will be packed.
+        memset(TraceBuffer, 0u, Size * sizeof(*TraceBuffer));
+
+        // Indicate that there were more frames, and we just didn't have enough
+        // room to store them.
+        return Size * 2;
+      });
+  // Retrieve a frame from the collected backtrace, make sure it works E2E.
+  uintptr_t TraceOutput;
+  EXPECT_EQ(gwp_asan::AllocationMetadata::kMaxTraceLengthToCollect,
+            __gwp_asan_get_allocation_trace(&Meta, &TraceOutput, 1));
+}
+
+TEST(Backtrace, ExceedsRetrievableAllocLength) {
+  gwp_asan::AllocationMetadata Meta;
+  constexpr size_t kNumFramesToStore = 3u;
+  Meta.AllocationTrace.RecordBacktrace(
+      [](uintptr_t *TraceBuffer, size_t /* Size */) -> size_t {
+        memset(TraceBuffer, kNumFramesToStore,
+               kNumFramesToStore * sizeof(*TraceBuffer));
+        return kNumFramesToStore;
+      });
+  uintptr_t TraceOutput;
+  // Ask for one element, get told that there's `kNumFramesToStore` available.
+  EXPECT_EQ(kNumFramesToStore,
+            __gwp_asan_get_allocation_trace(&Meta, &TraceOutput, 1));
+}
+
+TEST(Backtrace, ExceedsRetrievableDeallocLength) {
+  gwp_asan::AllocationMetadata Meta;
+  constexpr size_t kNumFramesToStore = 3u;
+  Meta.DeallocationTrace.RecordBacktrace(
+      [](uintptr_t *TraceBuffer, size_t /* Size */) -> size_t {
+        memset(TraceBuffer, kNumFramesToStore,
+               kNumFramesToStore * sizeof(*TraceBuffer));
+        return kNumFramesToStore;
       });
   uintptr_t TraceOutput;
-  EXPECT_EQ(1u, __gwp_asan_get_allocation_trace(&Meta, &TraceOutput, 1));
+  // Ask for one element, get told that there's `kNumFramesToStore` available.
+  EXPECT_EQ(kNumFramesToStore,
+            __gwp_asan_get_deallocation_trace(&Meta, &TraceOutput, 1));
 }