[compiler-rt] DO_NOT_MERGE (PR #66682)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 18 11:28:54 PDT 2023


https://github.com/vitalybuka created https://github.com/llvm/llvm-project/pull/66682

- [NFC][hwasan] Create *Report classes
- [NFC][hwasan] Use unnamed namespace and static
- [NFC][hwasan] Move Report classes together
- [NFC][hwasan] Extract BaseReport
- [NFC][hwasan] Move PrintAddressDescription
- [NFC][hwasan] Store thread id in SavedStackAllocations
- [NFC][hwasan] Add access_size into base report
- [NFC][hwasan] Make PrintAddressDescription method of BaseReport
- [NFC][hwasan] Collect heap related data early


>From 0087c44bcc6ef639bc7bdd4f4f9c1e9fe79a71f4 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 20:18:10 -0700
Subject: [PATCH 1/9] [NFC][hwasan] Create *Report classes

This prepare the code for rework to collect all nececcecary data before
symbolization. Symbolization as any untrivial computations may affect
hwasan metadata.
---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 82 ++++++++++++++++++++++--
 1 file changed, 75 insertions(+), 7 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index d318e26c6a86849..5a8bea78376a739 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -37,7 +37,7 @@ namespace __hwasan {
 
 class ScopedReport {
  public:
-  ScopedReport(bool fatal = false) : fatal(fatal) {
+  ScopedReport(bool fatal) : fatal(fatal) {
     Lock lock(&error_message_lock_);
     error_message_ptr_ = fatal ? &error_message_ : nullptr;
     ++hwasan_report_count;
@@ -569,9 +569,20 @@ uptr GetTopPc(StackTrace *stack) {
                      : 0;
 }
 
-void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) {
-  ScopedReport R(flags()->halt_on_error);
+namespace {
+class InvalidFreeReport {
+ public:
+  InvalidFreeReport(StackTrace *stack, uptr tagged_addr)
+      : stack(stack), tagged_addr(tagged_addr) {}
+  ~InvalidFreeReport();
 
+ private:
+  StackTrace *stack;
+  uptr tagged_addr;
+};
+
+InvalidFreeReport::~InvalidFreeReport() {
+  ScopedReport R(flags()->halt_on_error);
   uptr untagged_addr = UntagAddr(tagged_addr);
   tag_t ptr_tag = GetTagFromPointer(tagged_addr);
   tag_t *tag_ptr = nullptr;
@@ -610,9 +621,31 @@ void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) {
   MaybePrintAndroidHelpUrl();
   ReportErrorSummary(bug_type, stack);
 }
+}  // namespace
 
-void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size,
-                           const u8 *expected) {
+void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) {
+  InvalidFreeReport R(stack, tagged_addr);
+}
+
+namespace {
+class TailOverwrittenReport {
+ public:
+  explicit TailOverwrittenReport(StackTrace *stack, uptr tagged_addr,
+                                 uptr orig_size, const u8 *expected)
+      : stack(stack),
+        tagged_addr(tagged_addr),
+        orig_size(orig_size),
+        expected(expected) {}
+  ~TailOverwrittenReport();
+
+ private:
+  StackTrace *stack;
+  uptr tagged_addr;
+  uptr orig_size;
+  const u8 *expected;
+};
+
+TailOverwrittenReport::~TailOverwrittenReport() {
   uptr tail_size = kShadowAlignment - (orig_size % kShadowAlignment);
   u8 actual_expected[kShadowAlignment];
   internal_memcpy(actual_expected, expected, tail_size);
@@ -682,9 +715,37 @@ void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size,
   MaybePrintAndroidHelpUrl();
   ReportErrorSummary(bug_type, stack);
 }
+}  // namespace
 
-void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size,
-                       bool is_store, bool fatal, uptr *registers_frame) {
+void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size,
+                           const u8 *expected) {
+  TailOverwrittenReport R(stack, tagged_addr, orig_size, expected);
+}
+
+namespace {
+class TagMismatchReport {
+ public:
+  explicit TagMismatchReport(StackTrace *stack, uptr tagged_addr,
+                             uptr access_size, bool is_store, bool fatal,
+                             uptr *registers_frame)
+      : stack(stack),
+        tagged_addr(tagged_addr),
+        access_size(access_size),
+        is_store(is_store),
+        fatal(fatal),
+        registers_frame(registers_frame) {}
+  ~TagMismatchReport();
+
+ private:
+  StackTrace *stack;
+  uptr tagged_addr;
+  uptr access_size;
+  bool is_store;
+  bool fatal;
+  uptr *registers_frame;
+};
+
+TagMismatchReport::~TagMismatchReport() {
   ScopedReport R(fatal);
   SavedStackAllocations current_stack_allocations(
       GetCurrentThread()->stack_allocations());
@@ -753,6 +814,13 @@ void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size,
   MaybePrintAndroidHelpUrl();
   ReportErrorSummary(bug_type, stack);
 }
+}  // namespace
+
+void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size,
+                       bool is_store, bool fatal, uptr *registers_frame) {
+  TagMismatchReport R(stack, tagged_addr, access_size, is_store, fatal,
+                      registers_frame);
+}
 
 // See the frame breakdown defined in __hwasan_tag_mismatch (from
 // hwasan_tag_mismatch_{aarch64,riscv64}.S).

>From 16ca713e42896fb1e88e3d807cceec096c79bdd4 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 20:33:36 -0700
Subject: [PATCH 2/9] [NFC][hwasan] Use unnamed namespace and static

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 5a8bea78376a739..bf11462dc19ccc1 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -108,6 +108,7 @@ static void MaybePrintAndroidHelpUrl() {
 #endif
 }
 
+namespace {
 // A RAII object that holds a copy of the current thread stack ring buffer.
 // The actual stack buffer may change while we are iterating over it (for
 // example, Printf may call syslog() which can itself be built with hwasan).
@@ -143,6 +144,7 @@ class Decorator: public __sanitizer::SanitizerCommonDecorator {
   const char *Location() { return Green(); }
   const char *Thread() { return Green(); }
 };
+}  // namespace
 
 static bool FindHeapAllocation(HeapAllocationsRingBuffer *rb, uptr tagged_addr,
                                HeapAllocationRecord *har, uptr *ring_index,
@@ -373,7 +375,7 @@ static void ShowHeapOrGlobalCandidate(uptr untagged_addr, tag_t *candidate,
   }
 }
 
-void PrintAddressDescription(
+static void PrintAddressDescription(
     uptr tagged_addr, uptr access_size,
     StackAllocationsRingBuffer *current_stack_allocations) {
   Decorator d;
@@ -564,7 +566,7 @@ static void PrintTagsAroundAddr(tag_t *tag_ptr) {
       "description of short granule tags\n");
 }
 
-uptr GetTopPc(StackTrace *stack) {
+static uptr GetTopPc(StackTrace *stack) {
   return stack->size ? StackTrace::GetPreviousInstructionPc(stack->trace[0])
                      : 0;
 }

>From 895c5a9a8a14e47224bdd98233296ad44ab748a5 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 20:34:24 -0700
Subject: [PATCH 3/9] [NFC][hwasan] Move Report classes together

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index bf11462dc19ccc1..ccf7c10be994537 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -623,13 +623,7 @@ InvalidFreeReport::~InvalidFreeReport() {
   MaybePrintAndroidHelpUrl();
   ReportErrorSummary(bug_type, stack);
 }
-}  // namespace
-
-void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) {
-  InvalidFreeReport R(stack, tagged_addr);
-}
 
-namespace {
 class TailOverwrittenReport {
  public:
   explicit TailOverwrittenReport(StackTrace *stack, uptr tagged_addr,
@@ -717,14 +711,7 @@ TailOverwrittenReport::~TailOverwrittenReport() {
   MaybePrintAndroidHelpUrl();
   ReportErrorSummary(bug_type, stack);
 }
-}  // namespace
-
-void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size,
-                           const u8 *expected) {
-  TailOverwrittenReport R(stack, tagged_addr, orig_size, expected);
-}
 
-namespace {
 class TagMismatchReport {
  public:
   explicit TagMismatchReport(StackTrace *stack, uptr tagged_addr,
@@ -818,6 +805,15 @@ TagMismatchReport::~TagMismatchReport() {
 }
 }  // namespace
 
+void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) {
+  InvalidFreeReport R(stack, tagged_addr);
+}
+
+void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size,
+                           const u8 *expected) {
+  TailOverwrittenReport R(stack, tagged_addr, orig_size, expected);
+}
+
 void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size,
                        bool is_store, bool fatal, uptr *registers_frame) {
   TagMismatchReport R(stack, tagged_addr, access_size, is_store, fatal,

>From 487c559b58b9ade1d92fb64dab3a0ef5195758aa Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 21:37:14 -0700
Subject: [PATCH 4/9] [NFC][hwasan] Extract BaseReport

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 48 ++++++++++++------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index ccf7c10be994537..7c025471cbf2ff6 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -572,21 +572,33 @@ static uptr GetTopPc(StackTrace *stack) {
 }
 
 namespace {
-class InvalidFreeReport {
+class BaseReport {
+ public:
+  BaseReport(StackTrace *stack, bool fatal, uptr tagged_addr)
+      : scoped_report(fatal),
+        stack(stack),
+        tagged_addr(tagged_addr),
+        untagged_addr(UntagAddr(tagged_addr)),
+        ptr_tag(GetTagFromPointer(tagged_addr)) {}
+
+ protected:
+  ScopedReport scoped_report;
+  StackTrace *stack;
+  uptr tagged_addr;
+  uptr untagged_addr;
+  tag_t ptr_tag;
+};
+
+class InvalidFreeReport : public BaseReport {
  public:
   InvalidFreeReport(StackTrace *stack, uptr tagged_addr)
-      : stack(stack), tagged_addr(tagged_addr) {}
+      : BaseReport(stack, flags()->halt_on_error, tagged_addr) {}
   ~InvalidFreeReport();
 
  private:
-  StackTrace *stack;
-  uptr tagged_addr;
 };
 
 InvalidFreeReport::~InvalidFreeReport() {
-  ScopedReport R(flags()->halt_on_error);
-  uptr untagged_addr = UntagAddr(tagged_addr);
-  tag_t ptr_tag = GetTagFromPointer(tagged_addr);
   tag_t *tag_ptr = nullptr;
   tag_t mem_tag = 0;
   if (MemIsApp(untagged_addr)) {
@@ -624,19 +636,16 @@ InvalidFreeReport::~InvalidFreeReport() {
   ReportErrorSummary(bug_type, stack);
 }
 
-class TailOverwrittenReport {
+class TailOverwrittenReport : public BaseReport {
  public:
   explicit TailOverwrittenReport(StackTrace *stack, uptr tagged_addr,
                                  uptr orig_size, const u8 *expected)
-      : stack(stack),
-        tagged_addr(tagged_addr),
+      : BaseReport(stack, flags()->halt_on_error, tagged_addr),
         orig_size(orig_size),
         expected(expected) {}
   ~TailOverwrittenReport();
 
  private:
-  StackTrace *stack;
-  uptr tagged_addr;
   uptr orig_size;
   const u8 *expected;
 };
@@ -645,16 +654,13 @@ TailOverwrittenReport::~TailOverwrittenReport() {
   uptr tail_size = kShadowAlignment - (orig_size % kShadowAlignment);
   u8 actual_expected[kShadowAlignment];
   internal_memcpy(actual_expected, expected, tail_size);
-  tag_t ptr_tag = GetTagFromPointer(tagged_addr);
   // Short granule is stashed in the last byte of the magic string. To avoid
   // confusion, make the expected magic string contain the short granule tag.
   if (orig_size % kShadowAlignment != 0) {
     actual_expected[tail_size - 1] = ptr_tag;
   }
 
-  ScopedReport R(flags()->halt_on_error);
   Decorator d;
-  uptr untagged_addr = UntagAddr(tagged_addr);
   Printf("%s", d.Error());
   const char *bug_type = "allocation-tail-overwritten";
   Report("ERROR: %s: %s; heap object [%p,%p) of size %zd\n", SanitizerToolName,
@@ -712,35 +718,28 @@ TailOverwrittenReport::~TailOverwrittenReport() {
   ReportErrorSummary(bug_type, stack);
 }
 
-class TagMismatchReport {
+class TagMismatchReport : public BaseReport {
  public:
   explicit TagMismatchReport(StackTrace *stack, uptr tagged_addr,
                              uptr access_size, bool is_store, bool fatal,
                              uptr *registers_frame)
-      : stack(stack),
-        tagged_addr(tagged_addr),
+      : BaseReport(stack, fatal, tagged_addr),
         access_size(access_size),
         is_store(is_store),
-        fatal(fatal),
         registers_frame(registers_frame) {}
   ~TagMismatchReport();
 
  private:
-  StackTrace *stack;
-  uptr tagged_addr;
   uptr access_size;
   bool is_store;
-  bool fatal;
   uptr *registers_frame;
 };
 
 TagMismatchReport::~TagMismatchReport() {
-  ScopedReport R(fatal);
   SavedStackAllocations current_stack_allocations(
       GetCurrentThread()->stack_allocations());
 
   Decorator d;
-  uptr untagged_addr = UntagAddr(tagged_addr);
   // TODO: when possible, try to print heap-use-after-free, etc.
   const char *bug_type = "tag-mismatch";
   uptr pc = GetTopPc(stack);
@@ -754,7 +753,6 @@ TagMismatchReport::~TagMismatchReport() {
       __hwasan_test_shadow(reinterpret_cast<void *>(tagged_addr), access_size);
   CHECK_GE(offset, 0);
   CHECK_LT(offset, static_cast<sptr>(access_size));
-  tag_t ptr_tag = GetTagFromPointer(tagged_addr);
   tag_t *tag_ptr =
       reinterpret_cast<tag_t *>(MemToShadow(untagged_addr + offset));
   tag_t mem_tag = *tag_ptr;

>From b6f34e013537308c99b70582ceeba4aa76d83777 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 23:21:45 -0700
Subject: [PATCH 5/9] [NFC][hwasan] Move PrintAddressDescription

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 151 ++++++++++++-----------
 1 file changed, 76 insertions(+), 75 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 7c025471cbf2ff6..44285e90b00ebe3 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -375,6 +375,82 @@ static void ShowHeapOrGlobalCandidate(uptr untagged_addr, tag_t *candidate,
   }
 }
 
+void ReportStats() {}
+
+static void PrintTagInfoAroundAddr(tag_t *tag_ptr, uptr num_rows,
+                                   void (*print_tag)(InternalScopedString &s,
+                                                     tag_t *tag)) {
+  const uptr row_len = 16;  // better be power of two.
+  tag_t *center_row_beg = reinterpret_cast<tag_t *>(
+      RoundDownTo(reinterpret_cast<uptr>(tag_ptr), row_len));
+  tag_t *beg_row = center_row_beg - row_len * (num_rows / 2);
+  tag_t *end_row = center_row_beg + row_len * ((num_rows + 1) / 2);
+  InternalScopedString s;
+  for (tag_t *row = beg_row; row < end_row; row += row_len) {
+    s.Append(row == center_row_beg ? "=>" : "  ");
+    s.AppendF("%p:", (void *)ShadowToMem(reinterpret_cast<uptr>(row)));
+    for (uptr i = 0; i < row_len; i++) {
+      s.Append(row + i == tag_ptr ? "[" : " ");
+      print_tag(s, &row[i]);
+      s.Append(row + i == tag_ptr ? "]" : " ");
+    }
+    s.AppendF("\n");
+  }
+  Printf("%s", s.data());
+}
+
+static void PrintTagsAroundAddr(tag_t *tag_ptr) {
+  Printf(
+      "Memory tags around the buggy address (one tag corresponds to %zd "
+      "bytes):\n",
+      kShadowAlignment);
+  PrintTagInfoAroundAddr(tag_ptr, 17, [](InternalScopedString &s, tag_t *tag) {
+    s.AppendF("%02x", *tag);
+  });
+
+  Printf(
+      "Tags for short granules around the buggy address (one tag corresponds "
+      "to %zd bytes):\n",
+      kShadowAlignment);
+  PrintTagInfoAroundAddr(tag_ptr, 3, [](InternalScopedString &s, tag_t *tag) {
+    if (*tag >= 1 && *tag <= kShadowAlignment) {
+      uptr granule_addr = ShadowToMem(reinterpret_cast<uptr>(tag));
+      s.AppendF("%02x",
+                *reinterpret_cast<u8 *>(granule_addr + kShadowAlignment - 1));
+    } else {
+      s.AppendF("..");
+    }
+  });
+  Printf(
+      "See "
+      "https://clang.llvm.org/docs/"
+      "HardwareAssistedAddressSanitizerDesign.html#short-granules for a "
+      "description of short granule tags\n");
+}
+
+static uptr GetTopPc(StackTrace *stack) {
+  return stack->size ? StackTrace::GetPreviousInstructionPc(stack->trace[0])
+                     : 0;
+}
+
+namespace {
+class BaseReport {
+ public:
+  BaseReport(StackTrace *stack, bool fatal, uptr tagged_addr)
+      : scoped_report(fatal),
+        stack(stack),
+        tagged_addr(tagged_addr),
+        untagged_addr(UntagAddr(tagged_addr)),
+        ptr_tag(GetTagFromPointer(tagged_addr)) {}
+
+ protected:
+  ScopedReport scoped_report;
+  StackTrace *stack;
+  uptr tagged_addr;
+  uptr untagged_addr;
+  tag_t ptr_tag;
+};
+
 static void PrintAddressDescription(
     uptr tagged_addr, uptr access_size,
     StackAllocationsRingBuffer *current_stack_allocations) {
@@ -514,81 +590,6 @@ static void PrintAddressDescription(
   }
 }
 
-void ReportStats() {}
-
-static void PrintTagInfoAroundAddr(tag_t *tag_ptr, uptr num_rows,
-                                   void (*print_tag)(InternalScopedString &s,
-                                                     tag_t *tag)) {
-  const uptr row_len = 16;  // better be power of two.
-  tag_t *center_row_beg = reinterpret_cast<tag_t *>(
-      RoundDownTo(reinterpret_cast<uptr>(tag_ptr), row_len));
-  tag_t *beg_row = center_row_beg - row_len * (num_rows / 2);
-  tag_t *end_row = center_row_beg + row_len * ((num_rows + 1) / 2);
-  InternalScopedString s;
-  for (tag_t *row = beg_row; row < end_row; row += row_len) {
-    s.Append(row == center_row_beg ? "=>" : "  ");
-    s.AppendF("%p:", (void *)ShadowToMem(reinterpret_cast<uptr>(row)));
-    for (uptr i = 0; i < row_len; i++) {
-      s.Append(row + i == tag_ptr ? "[" : " ");
-      print_tag(s, &row[i]);
-      s.Append(row + i == tag_ptr ? "]" : " ");
-    }
-    s.AppendF("\n");
-  }
-  Printf("%s", s.data());
-}
-
-static void PrintTagsAroundAddr(tag_t *tag_ptr) {
-  Printf(
-      "Memory tags around the buggy address (one tag corresponds to %zd "
-      "bytes):\n", kShadowAlignment);
-  PrintTagInfoAroundAddr(tag_ptr, 17, [](InternalScopedString &s, tag_t *tag) {
-    s.AppendF("%02x", *tag);
-  });
-
-  Printf(
-      "Tags for short granules around the buggy address (one tag corresponds "
-      "to %zd bytes):\n",
-      kShadowAlignment);
-  PrintTagInfoAroundAddr(tag_ptr, 3, [](InternalScopedString &s, tag_t *tag) {
-    if (*tag >= 1 && *tag <= kShadowAlignment) {
-      uptr granule_addr = ShadowToMem(reinterpret_cast<uptr>(tag));
-      s.AppendF("%02x",
-                *reinterpret_cast<u8 *>(granule_addr + kShadowAlignment - 1));
-    } else {
-      s.AppendF("..");
-    }
-  });
-  Printf(
-      "See "
-      "https://clang.llvm.org/docs/"
-      "HardwareAssistedAddressSanitizerDesign.html#short-granules for a "
-      "description of short granule tags\n");
-}
-
-static uptr GetTopPc(StackTrace *stack) {
-  return stack->size ? StackTrace::GetPreviousInstructionPc(stack->trace[0])
-                     : 0;
-}
-
-namespace {
-class BaseReport {
- public:
-  BaseReport(StackTrace *stack, bool fatal, uptr tagged_addr)
-      : scoped_report(fatal),
-        stack(stack),
-        tagged_addr(tagged_addr),
-        untagged_addr(UntagAddr(tagged_addr)),
-        ptr_tag(GetTagFromPointer(tagged_addr)) {}
-
- protected:
-  ScopedReport scoped_report;
-  StackTrace *stack;
-  uptr tagged_addr;
-  uptr untagged_addr;
-  tag_t ptr_tag;
-};
-
 class InvalidFreeReport : public BaseReport {
  public:
   InvalidFreeReport(StackTrace *stack, uptr tagged_addr)

>From 6683603ba6b19987ca766f96519a3c222b7c5861 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 23:25:47 -0700
Subject: [PATCH 6/9] [NFC][hwasan] Store thread id in SavedStackAllocations

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 26 ++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 44285e90b00ebe3..3a44e8b02666d40 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -114,24 +114,39 @@ namespace {
 // example, Printf may call syslog() which can itself be built with hwasan).
 class SavedStackAllocations {
  public:
-  SavedStackAllocations(StackAllocationsRingBuffer *rb) {
+  SavedStackAllocations() = default;
+
+  explicit SavedStackAllocations(Thread *t) { CopyFrom(t); }
+
+  void CopyFrom(Thread *t) {
+    StackAllocationsRingBuffer *rb = t->stack_allocations();
     uptr size = rb->size() * sizeof(uptr);
     void *storage =
         MmapAlignedOrDieOnFatalError(size, size * 2, "saved stack allocations");
     new (&rb_) StackAllocationsRingBuffer(*rb, storage);
+    thread_id_ = t->unique_id();
   }
 
   ~SavedStackAllocations() {
-    StackAllocationsRingBuffer *rb = get();
-    UnmapOrDie(rb->StartOfStorage(), rb->size() * sizeof(uptr));
+    if (rb_) {
+      StackAllocationsRingBuffer *rb = get();
+      UnmapOrDie(rb->StartOfStorage(), rb->size() * sizeof(uptr));
+    }
+  }
+
+  const StackAllocationsRingBuffer *get() const {
+    return (const StackAllocationsRingBuffer *)&rb_;
   }
 
   StackAllocationsRingBuffer *get() {
     return (StackAllocationsRingBuffer *)&rb_;
   }
 
+  u32 thread_id() const { return thread_id_; }
+
  private:
-  uptr rb_;
+  uptr rb_ = 0;
+  u32 thread_id_;
 };
 
 class Decorator: public __sanitizer::SanitizerCommonDecorator {
@@ -737,8 +752,7 @@ class TagMismatchReport : public BaseReport {
 };
 
 TagMismatchReport::~TagMismatchReport() {
-  SavedStackAllocations current_stack_allocations(
-      GetCurrentThread()->stack_allocations());
+  SavedStackAllocations current_stack_allocations(GetCurrentThread());
 
   Decorator d;
   // TODO: when possible, try to print heap-use-after-free, etc.

>From db41857eb259d6e972bf086af7a337abc02493c6 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 23:29:35 -0700
Subject: [PATCH 7/9] [NFC][hwasan] Add access_size into base report

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 3a44e8b02666d40..acfe085556c338a 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -451,19 +451,21 @@ static uptr GetTopPc(StackTrace *stack) {
 namespace {
 class BaseReport {
  public:
-  BaseReport(StackTrace *stack, bool fatal, uptr tagged_addr)
+  BaseReport(StackTrace *stack, bool fatal, uptr tagged_addr, uptr access_size)
       : scoped_report(fatal),
         stack(stack),
         tagged_addr(tagged_addr),
+        access_size(access_size),
         untagged_addr(UntagAddr(tagged_addr)),
         ptr_tag(GetTagFromPointer(tagged_addr)) {}
 
  protected:
   ScopedReport scoped_report;
-  StackTrace *stack;
-  uptr tagged_addr;
-  uptr untagged_addr;
-  tag_t ptr_tag;
+  StackTrace *stack = nullptr;
+  uptr tagged_addr = 0;
+  uptr access_size = 0;
+  uptr untagged_addr = 0;
+  tag_t ptr_tag = 0;
 };
 
 static void PrintAddressDescription(
@@ -608,7 +610,7 @@ static void PrintAddressDescription(
 class InvalidFreeReport : public BaseReport {
  public:
   InvalidFreeReport(StackTrace *stack, uptr tagged_addr)
-      : BaseReport(stack, flags()->halt_on_error, tagged_addr) {}
+      : BaseReport(stack, flags()->halt_on_error, tagged_addr, 0) {}
   ~InvalidFreeReport();
 
  private:
@@ -656,7 +658,7 @@ class TailOverwrittenReport : public BaseReport {
  public:
   explicit TailOverwrittenReport(StackTrace *stack, uptr tagged_addr,
                                  uptr orig_size, const u8 *expected)
-      : BaseReport(stack, flags()->halt_on_error, tagged_addr),
+      : BaseReport(stack, flags()->halt_on_error, tagged_addr, 0),
         orig_size(orig_size),
         expected(expected) {}
   ~TailOverwrittenReport();
@@ -739,7 +741,7 @@ class TagMismatchReport : public BaseReport {
   explicit TagMismatchReport(StackTrace *stack, uptr tagged_addr,
                              uptr access_size, bool is_store, bool fatal,
                              uptr *registers_frame)
-      : BaseReport(stack, fatal, tagged_addr),
+      : BaseReport(stack, fatal, tagged_addr, access_size),
         access_size(access_size),
         is_store(is_store),
         registers_frame(registers_frame) {}

>From f8243463de41456e90338e8a8b7b4a3c573938c9 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sun, 17 Sep 2023 23:50:38 -0700
Subject: [PATCH 8/9] [NFC][hwasan] Make PrintAddressDescription method of
 BaseReport

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 80 ++++++++++++------------
 1 file changed, 41 insertions(+), 39 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index acfe085556c338a..c08bb57cb56dd0e 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -200,7 +200,7 @@ static bool FindHeapAllocation(HeapAllocationsRingBuffer *rb, uptr tagged_addr,
   return false;
 }
 
-static void PrintStackAllocations(StackAllocationsRingBuffer *sa,
+static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
                                   tag_t addr_tag, uptr untagged_addr) {
   uptr frames = Min((uptr)flags()->stack_history_size, sa->size());
   bool found_local = false;
@@ -457,23 +457,31 @@ class BaseReport {
         tagged_addr(tagged_addr),
         access_size(access_size),
         untagged_addr(UntagAddr(tagged_addr)),
-        ptr_tag(GetTagFromPointer(tagged_addr)) {}
+        ptr_tag(GetTagFromPointer(tagged_addr)) {
+    hwasanThreadList().VisitAllLiveThreads([&](Thread *t) {
+      if (stack_count < ARRAY_SIZE(stack_allocations) &&
+          t->AddrIsInStack(untagged_addr)) {
+        stack_allocations[stack_count++].CopyFrom(t);
+      }
+    });
+  }
 
  protected:
+  void PrintAddressDescription() const;
+
   ScopedReport scoped_report;
   StackTrace *stack = nullptr;
   uptr tagged_addr = 0;
   uptr access_size = 0;
   uptr untagged_addr = 0;
   tag_t ptr_tag = 0;
+  uptr stack_count = 0;
+  SavedStackAllocations stack_allocations[16];
 };
 
-static void PrintAddressDescription(
-    uptr tagged_addr, uptr access_size,
-    StackAllocationsRingBuffer *current_stack_allocations) {
+void BaseReport::PrintAddressDescription() const {
   Decorator d;
   int num_descriptions_printed = 0;
-  uptr untagged_addr = UntagAddr(tagged_addr);
 
   if (MemIsShadow(untagged_addr)) {
     Printf("%s%p is HWAsan shadow memory.\n%s", d.Location(), untagged_addr,
@@ -495,48 +503,44 @@ static void PrintAddressDescription(
            d.Default());
   }
 
-  tag_t addr_tag = GetTagFromPointer(tagged_addr);
-
-  bool on_stack = false;
   // Check stack first. If the address is on the stack of a live thread, we
   // know it cannot be a heap / global overflow.
-  hwasanThreadList().VisitAllLiveThreads([&](Thread *t) {
-    if (t->AddrIsInStack(untagged_addr)) {
-      on_stack = true;
-      // TODO(fmayer): figure out how to distinguish use-after-return and
-      // stack-buffer-overflow.
-      Printf("%s", d.Error());
-      Printf("\nCause: stack tag-mismatch\n");
-      Printf("%s", d.Location());
-      Printf("Address %p is located in stack of thread T%zd\n", untagged_addr,
-             t->unique_id());
-      Printf("%s", d.Default());
-      t->Announce();
-
-      auto *sa = (t == GetCurrentThread() && current_stack_allocations)
-                     ? current_stack_allocations
-                     : t->stack_allocations();
-      PrintStackAllocations(sa, addr_tag, untagged_addr);
-      num_descriptions_printed++;
-    }
-  });
+  if (stack_count) {
+    hwasanThreadList().VisitAllLiveThreads([&](Thread *t) {
+      for (auto &allocations : stack_allocations) {
+        if (allocations.thread_id() == t->unique_id()) {
+          // TODO(fmayer): figure out how to distinguish use-after-return and
+          // stack-buffer-overflow.
+          Printf("%s", d.Error());
+          Printf("\nCause: stack tag-mismatch\n");
+          Printf("%s", d.Location());
+          Printf("Address %p is located in stack of thread T%zd\n",
+                 untagged_addr, t->unique_id());
+          Printf("%s", d.Default());
+          t->Announce();
+
+          PrintStackAllocations(allocations.get(), ptr_tag, untagged_addr);
+          num_descriptions_printed++;
+        }
+      }
+    });
+  }
 
   // Check if this looks like a heap buffer overflow by scanning
   // the shadow left and right and looking for the first adjacent
-  // object with a different memory tag. If that tag matches addr_tag,
+  // object with a different memory tag. If that tag matches ptr_tag,
   // check the allocator if it has a live chunk there.
   tag_t *tag_ptr = reinterpret_cast<tag_t*>(MemToShadow(untagged_addr));
   tag_t *candidate = nullptr, *left = tag_ptr, *right = tag_ptr;
   uptr candidate_distance = 0;
   for (; candidate_distance < 1000; candidate_distance++) {
-    if (MemIsShadow(reinterpret_cast<uptr>(left)) &&
-        TagsEqual(addr_tag, left)) {
+    if (MemIsShadow(reinterpret_cast<uptr>(left)) && TagsEqual(ptr_tag, left)) {
       candidate = left;
       break;
     }
     --left;
     if (MemIsShadow(reinterpret_cast<uptr>(right)) &&
-        TagsEqual(addr_tag, right)) {
+        TagsEqual(ptr_tag, right)) {
       candidate = right;
       break;
     }
@@ -545,7 +549,8 @@ static void PrintAddressDescription(
 
   constexpr auto kCloseCandidateDistance = 1;
 
-  if (!on_stack && candidate && candidate_distance <= kCloseCandidateDistance) {
+  if (!stack_count && candidate &&
+      candidate_distance <= kCloseCandidateDistance) {
     ShowHeapOrGlobalCandidate(untagged_addr, candidate, left, right);
     num_descriptions_printed++;
   }
@@ -645,7 +650,7 @@ InvalidFreeReport::~InvalidFreeReport() {
 
   stack->Print();
 
-  PrintAddressDescription(tagged_addr, 0, nullptr);
+  PrintAddressDescription();
 
   if (tag_ptr)
     PrintTagsAroundAddr(tag_ptr);
@@ -754,8 +759,6 @@ class TagMismatchReport : public BaseReport {
 };
 
 TagMismatchReport::~TagMismatchReport() {
-  SavedStackAllocations current_stack_allocations(GetCurrentThread());
-
   Decorator d;
   // TODO: when possible, try to print heap-use-after-free, etc.
   const char *bug_type = "tag-mismatch";
@@ -806,8 +809,7 @@ TagMismatchReport::~TagMismatchReport() {
 
   stack->Print();
 
-  PrintAddressDescription(tagged_addr, access_size,
-                          current_stack_allocations.get());
+  PrintAddressDescription();
   t->Announce();
 
   PrintTagsAroundAddr(tag_ptr);

>From bf116b4cb5ada43402685bf627c3e83e4a4b7933 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Mon, 18 Sep 2023 00:15:28 -0700
Subject: [PATCH 9/9] [NFC][hwasan] Collect heap related data early

---
 compiler-rt/lib/hwasan/hwasan_report.cpp | 38 +++++++++++++++++-------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index c08bb57cb56dd0e..924371a2e90cc05 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -458,6 +458,17 @@ class BaseReport {
         access_size(access_size),
         untagged_addr(UntagAddr(tagged_addr)),
         ptr_tag(GetTagFromPointer(tagged_addr)) {
+    if (MemIsShadow(untagged_addr))
+      return;
+
+    HwasanChunkView chunk = FindHeapChunkByAddress(untagged_addr);
+    heap.begin = chunk.Beg();
+    if (heap.begin) {
+      heap.size = chunk.ActualSize();
+      heap.from_small_heap = chunk.FromSmallHeap();
+      heap.is_allocated = chunk.IsAllocated();
+    }
+
     hwasanThreadList().VisitAllLiveThreads([&](Thread *t) {
       if (stack_count < ARRAY_SIZE(stack_allocations) &&
           t->AddrIsInStack(untagged_addr)) {
@@ -475,8 +486,16 @@ class BaseReport {
   uptr access_size = 0;
   uptr untagged_addr = 0;
   tag_t ptr_tag = 0;
+
   uptr stack_count = 0;
   SavedStackAllocations stack_allocations[16];
+
+  struct {
+    uptr begin = 0;
+    uptr size = 0;
+    bool from_small_heap = false;
+    bool is_allocated = false;
+  } heap;
 };
 
 void BaseReport::PrintAddressDescription() const {
@@ -490,17 +509,14 @@ void BaseReport::PrintAddressDescription() const {
   }
 
   // Print some very basic information about the address, if it's a heap.
-  HwasanChunkView chunk = FindHeapChunkByAddress(untagged_addr);
-  if (uptr beg = chunk.Beg()) {
-    uptr size = chunk.ActualSize();
-    Printf("%s[%p,%p) is a %s %s heap chunk; "
-           "size: %zd offset: %zd\n%s",
-           d.Location(),
-           beg, beg + size,
-           chunk.FromSmallHeap() ? "small" : "large",
-           chunk.IsAllocated() ? "allocated" : "unallocated",
-           size, untagged_addr - beg,
-           d.Default());
+  if (heap.begin) {
+    Printf(
+        "%s[%p,%p) is a %s %s heap chunk; "
+        "size: %zd offset: %zd\n%s",
+        d.Location(), heap.begin, heap.begin + heap.size,
+        heap.from_small_heap ? "small" : "large",
+        heap.is_allocated ? "allocated" : "unallocated", heap.size,
+        untagged_addr - heap.begin, d.Default());
   }
 
   // Check stack first. If the address is on the stack of a live thread, we



More information about the llvm-commits mailing list