[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

diff --git a/compiler-rt/lib/gwp_asan/common.cpp b/compiler-rt/lib/gwp_asan/common.cpp
index 3438c4b..483694d 100644 (file)
--- 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 c3b9e14..3c64025 100644 (file)
--- 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 b3d4427..9515065 100644 (file)
--- 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));
 }