hwasan: Teach the runtime to identify the local variable being accessed in UAR reports.

Each function's PC is recorded in the ring buffer. From there we can access
the function's local variables and reconstruct the tag of each one with the
help of the information printed by llvm-symbolizer's new FRAME command. We
can then find the variable that was likely being accessed by matching the
pointer's tag against the reconstructed tag.

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

llvm-svn: 364607

compiler-rt/lib/hwasan/hwasan.cpp

@@ -193,36 +193,6 @@ void UpdateMemoryUsage() {
 void UpdateMemoryUsage() {}
 #endif
 
-struct FrameDescription {
-  uptr PC;
-  const char *Descr;
-};
-
-struct FrameDescriptionArray {
-  FrameDescription *beg, *end;
-};
-
-static InternalMmapVectorNoCtor<FrameDescriptionArray> AllFrames;
-
-void InitFrameDescriptors(uptr b, uptr e) {
-  FrameDescription *beg = reinterpret_cast<FrameDescription *>(b);
-  FrameDescription *end = reinterpret_cast<FrameDescription *>(e);
-  if (beg == end)
-    return;
-  AllFrames.push_back({beg, end});
-  if (Verbosity())
-    for (FrameDescription *frame_descr = beg; frame_descr < end; frame_descr++)
-      Printf("Frame: %p %s\n", frame_descr->PC, frame_descr->Descr);
-}
-
-const char *GetStackFrameDescr(uptr pc) {
-  for (uptr i = 0, n = AllFrames.size(); i < n; i++)
-    for (auto p = AllFrames[i].beg; p < AllFrames[i].end; p++)
-      if (p->PC == pc)
-        return p->Descr;
-  return nullptr;
-}
-
 // Prepare to run instrumented code on the main thread.
 void InitInstrumentation() {
   if (hwasan_instrumentation_inited) return;
@@ -267,9 +237,9 @@ using namespace __hwasan;
 
 uptr __hwasan_shadow_memory_dynamic_address;  // Global interface symbol.
 
-void __hwasan_init_frames(uptr beg, uptr end) {
-  InitFrameDescriptors(beg, end);
-}
+// This function was used by the old frame descriptor mechanism. We keep it
+// around to avoid breaking ABI.
+void __hwasan_init_frames(uptr beg, uptr end) {}
 
 void __hwasan_init_static() {
   InitShadowGOT();

compiler-rt/lib/hwasan/hwasan.h

@@ -44,6 +44,11 @@ const uptr kAddressTagMask = 0xFFUL << kAddressTagShift;
 // for threads and stack histories. This is an ABI constant.
 const unsigned kShadowBaseAlignment = 32;
 
+const unsigned kRecordAddrBaseTagShift = 3;
+const unsigned kRecordFPShift = 48;
+const unsigned kRecordFPLShift = 4;
+const unsigned kRecordFPModulus = 1 << (64 - kRecordFPShift + kRecordFPLShift);
+
 static inline tag_t GetTagFromPointer(uptr p) {
   return p >> kAddressTagShift;
 }
@@ -93,9 +98,6 @@ void hwasan_free(void *ptr, StackTrace *stack);
 void InstallTrapHandler();
 void InstallAtExitHandler();
 
-const char *GetStackOriginDescr(u32 id, uptr *pc);
-const char *GetStackFrameDescr(uptr pc);
-
 void EnterSymbolizer();
 void ExitSymbolizer();
 bool IsInSymbolizer();

compiler-rt/lib/hwasan/hwasan_report.cpp

@@ -139,6 +139,75 @@ uptr FindHeapAllocation(HeapAllocationsRingBuffer *rb,
   return 0;
 }
 
+static void PrintStackAllocations(StackAllocationsRingBuffer *sa,
+                                  tag_t addr_tag, uptr untagged_addr) {
+  uptr frames = Min((uptr)flags()->stack_history_size, sa->size());
+  bool found_local = false;
+  for (uptr i = 0; i < frames; i++) {
+    const uptr *record_addr = &(*sa)[i];
+    uptr record = *record_addr;
+    if (!record)
+      break;
+    tag_t base_tag =
+        reinterpret_cast<uptr>(record_addr) >> kRecordAddrBaseTagShift;
+    uptr fp = (record >> kRecordFPShift) << kRecordFPLShift;
+    uptr pc_mask = (1ULL << kRecordFPShift) - 1;
+    uptr pc = record & pc_mask;
+    FrameInfo frame;
+    if (Symbolizer::GetOrInit()->SymbolizeFrame(pc, &frame)) {
+      for (LocalInfo &local : frame.locals) {
+        if (!local.has_frame_offset || !local.has_size || !local.has_tag_offset)
+          continue;
+        tag_t obj_tag = base_tag ^ local.tag_offset;
+        if (obj_tag != addr_tag)
+          continue;
+        // Calculate the offset from the object address to the faulting
+        // address. Because we only store bits 4-19 of FP (bits 0-3 are
+        // guaranteed to be zero), the calculation is performed mod 2^20 and may
+        // harmlessly underflow if the address mod 2^20 is below the object
+        // address.
+        uptr obj_offset =
+            (untagged_addr - fp - local.frame_offset) & (kRecordFPModulus - 1);
+        if (obj_offset >= local.size)
+          continue;
+        if (!found_local) {
+          Printf("Potentially referenced stack objects:\n");
+          found_local = true;
+        }
+        Printf("  %s in %s %s:%d\n", local.name, local.function_name,
+               local.decl_file, local.decl_line);
+      }
+      frame.Clear();
+    }
+  }
+
+  if (found_local)
+    return;
+
+  // We didn't find any locals. Most likely we don't have symbols, so dump
+  // the information that we have for offline analysis.
+  InternalScopedString frame_desc(GetPageSizeCached() * 2);
+  Printf("Previously allocated frames:\n");
+  for (uptr i = 0; i < frames; i++) {
+    const uptr *record_addr = &(*sa)[i];
+    uptr record = *record_addr;
+    if (!record)
+      break;
+    uptr pc_mask = (1ULL << 48) - 1;
+    uptr pc = record & pc_mask;
+    frame_desc.append("  record_addr:0x%zx record:0x%zx",
+                      reinterpret_cast<uptr>(record_addr), record);
+    if (SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc)) {
+      RenderFrame(&frame_desc, " %F %L\n", 0, frame->info,
+                  common_flags()->symbolize_vs_style,
+                  common_flags()->strip_path_prefix);
+      frame->ClearAll();
+    }
+    Printf("%s", frame_desc.data());
+    frame_desc.clear();
+  }
+}
+
 void PrintAddressDescription(
     uptr tagged_addr, uptr access_size,
     StackAllocationsRingBuffer *current_stack_allocations) {
@@ -238,33 +307,10 @@ void PrintAddressDescription(
       Printf("%s", d.Default());
       t->Announce();
 
-      // Temporary report section, needs to be improved.
-      Printf("Previously allocated frames:\n");
       auto *sa = (t == GetCurrentThread() && current_stack_allocations)
                      ? current_stack_allocations
                      : t->stack_allocations();
-      uptr frames = Min((uptr)flags()->stack_history_size, sa->size());
-      InternalScopedString frame_desc(GetPageSizeCached() * 2);
-      for (uptr i = 0; i < frames; i++) {
-        uptr record = (*sa)[i];
-        if (!record)
-          break;
-        uptr sp = (record >> 48) << 4;
-        uptr pc_mask = (1ULL << 48) - 1;
-        uptr pc = record & pc_mask;
-        if (SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc)) {
-          frame_desc.append(" sp: 0x%zx ", sp);
-          RenderFrame(&frame_desc, "#%n %p %F %L\n", 0, frame->info,
-                      common_flags()->symbolize_vs_style,
-                      common_flags()->strip_path_prefix);
-          frame->ClearAll();
-          if (auto Descr = GetStackFrameDescr(pc))
-            frame_desc.append("  %s\n", Descr);
-        }
-        Printf("%s", frame_desc.data());
-        frame_desc.clear();
-      }
-
+      PrintStackAllocations(sa, addr_tag, untagged_addr);
       num_descriptions_printed++;
     }
   });

compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h

@@ -139,7 +139,7 @@ class CompactRingBuffer {
     SetNext(next);
   }
 
-  T operator[](uptr Idx) const {
+  const T &operator[](uptr Idx) const {
     CHECK_LT(Idx, size());
     const T *Begin = (const T *)StartOfStorage();
     sptr StorageIdx = Next() - Begin;

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc

@@ -66,6 +66,16 @@ void DataInfo::Clear() {
   internal_memset(this, 0, sizeof(DataInfo));
 }
 
+void FrameInfo::Clear() {
+  InternalFree(module);
+  for (LocalInfo &local : locals) {
+    InternalFree(local.function_name);
+    InternalFree(local.name);
+    InternalFree(local.decl_file);
+  }
+  locals.clear();
+}
+
 Symbolizer *Symbolizer::symbolizer_;
 StaticSpinMutex Symbolizer::init_mu_;
 LowLevelAllocator Symbolizer::symbolizer_allocator_;

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h

@@ -20,6 +20,7 @@
 
 #include "sanitizer_common.h"
 #include "sanitizer_mutex.h"
+#include "sanitizer_vector.h"
 
 namespace __sanitizer {
 
@@ -77,6 +78,32 @@ struct DataInfo {
   void Clear();
 };
 
+struct LocalInfo {
+  char *function_name = nullptr;
+  char *name = nullptr;
+  char *decl_file = nullptr;
+  unsigned decl_line = 0;
+
+  bool has_frame_offset = false;
+  bool has_size = false;
+  bool has_tag_offset = false;
+
+  sptr frame_offset;
+  uptr size;
+  uptr tag_offset;
+
+  void Clear();
+};
+
+struct FrameInfo {
+  char *module;
+  uptr module_offset;
+  ModuleArch module_arch;
+
+  InternalMmapVector<LocalInfo> locals;
+  void Clear();
+};
+
 class SymbolizerTool;
 
 class Symbolizer final {
@@ -89,6 +116,7 @@ class Symbolizer final {
   // all inlined functions, if necessary).
   SymbolizedStack *SymbolizePC(uptr address);
   bool SymbolizeData(uptr address, DataInfo *info);
+  bool SymbolizeFrame(uptr address, FrameInfo *info);
 
   // The module names Symbolizer returns are stable and unique for every given
   // module.  It is safe to store and compare them as pointers.

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h

@@ -15,6 +15,7 @@
 
 #include "sanitizer_symbolizer.h"
 #include "sanitizer_file.h"
+#include "sanitizer_vector.h"
 
 namespace __sanitizer {
 
@@ -58,6 +59,10 @@ class SymbolizerTool {
     UNIMPLEMENTED();
   }
 
+  virtual bool SymbolizeFrame(uptr addr, FrameInfo *info) {
+    return false;
+  }
+
   virtual void Flush() {}
 
   // Return nullptr to fallback to the default platform-specific demangler.
@@ -120,12 +125,13 @@ class LLVMSymbolizer : public SymbolizerTool {
   explicit LLVMSymbolizer(const char *path, LowLevelAllocator *allocator);
 
   bool SymbolizePC(uptr addr, SymbolizedStack *stack) override;
-
   bool SymbolizeData(uptr addr, DataInfo *info) override;
+  bool SymbolizeFrame(uptr addr, FrameInfo *info) override;
 
  private:
-  const char *FormatAndSendCommand(bool is_data, const char *module_name,
-                                   uptr module_offset, ModuleArch arch);
+  const char *FormatAndSendCommand(const char *command_prefix,
+                                   const char *module_name, uptr module_offset,
+                                   ModuleArch arch);
 
   LLVMSymbolizerProcess *symbolizer_process_;
   static const uptr kBufferSize = 16 * 1024;

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

@@ -58,6 +58,16 @@ const char *ExtractUptr(const char *str, const char *delims, uptr *result) {
   return ret;
 }
 
+const char *ExtractSptr(const char *str, const char *delims, sptr *result) {
+  char *buff;
+  const char *ret = ExtractToken(str, delims, &buff);
+  if (buff != 0) {
+    *result = (sptr)internal_atoll(buff);
+  }
+  InternalFree(buff);
+  return ret;
+}
+
 const char *ExtractTokenUpToDelimiter(const char *str, const char *delimiter,
                                       char **result) {
   const char *found_delimiter = internal_strstr(str, delimiter);
@@ -112,6 +122,22 @@ bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
   return true;
 }
 
+bool Symbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) {
+  BlockingMutexLock l(&mu_);
+  const char *module_name;
+  if (!FindModuleNameAndOffsetForAddress(
+          addr, &module_name, &info->module_offset, &info->module_arch))
+    return false;
+  info->module = internal_strdup(module_name);
+  for (auto &tool : tools_) {
+    SymbolizerScope sym_scope(this);
+    if (tool.SymbolizeFrame(addr, info)) {
+      return true;
+    }
+  }
+  return true;
+}
+
 bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
                                              uptr *module_address) {
   BlockingMutexLock l(&mu_);
@@ -343,10 +369,38 @@ void ParseSymbolizeDataOutput(const char *str, DataInfo *info) {
   str = ExtractUptr(str, "\n", &info->size);
 }
 
+static void ParseSymbolizeFrameOutput(const char *str,
+                                      InternalMmapVector<LocalInfo> *locals) {
+  if (internal_strncmp(str, "??", 2) == 0)
+    return;
+
+  while (*str) {
+    LocalInfo local;
+    str = ExtractToken(str, "\n", &local.function_name);
+    str = ExtractToken(str, "\n", &local.name);
+
+    AddressInfo addr;
+    str = ParseFileLineInfo(&addr, str);
+    local.decl_file = addr.file;
+    local.decl_line = addr.line;
+
+    local.has_frame_offset = internal_strncmp(str, "??", 2) != 0;
+    str = ExtractSptr(str, " ", &local.frame_offset);
+
+    local.has_size = internal_strncmp(str, "??", 2) != 0;
+    str = ExtractUptr(str, " ", &local.size);
+
+    local.has_tag_offset = internal_strncmp(str, "??", 2) != 0;
+    str = ExtractUptr(str, "\n", &local.tag_offset);
+
+    locals->push_back(local);
+  }
+}
+
 bool LLVMSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
   AddressInfo *info = &stack->info;
   const char *buf = FormatAndSendCommand(
-      /*is_data*/ false, info->module, info->module_offset, info->module_arch);
+      "CODE", info->module, info->module_offset, info->module_arch);
   if (buf) {
     ParseSymbolizePCOutput(buf, stack);
     return true;
@@ -356,7 +410,7 @@ bool LLVMSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
 
 bool LLVMSymbolizer::SymbolizeData(uptr addr, DataInfo *info) {
   const char *buf = FormatAndSendCommand(
-      /*is_data*/ true, info->module, info->module_offset, info->module_arch);
+      "DATA", info->module, info->module_offset, info->module_arch);
   if (buf) {
     ParseSymbolizeDataOutput(buf, info);
     info->start += (addr - info->module_offset); // Add the base address.
@@ -365,22 +419,31 @@ bool LLVMSymbolizer::SymbolizeData(uptr addr, DataInfo *info) {
   return false;
 }
 
-const char *LLVMSymbolizer::FormatAndSendCommand(bool is_data,
+bool LLVMSymbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) {
+  const char *buf = FormatAndSendCommand(
+      "FRAME", info->module, info->module_offset, info->module_arch);
+  if (buf) {
+    ParseSymbolizeFrameOutput(buf, &info->locals);
+    return true;
+  }
+  return false;
+}
+
+const char *LLVMSymbolizer::FormatAndSendCommand(const char *command_prefix,
                                                  const char *module_name,
                                                  uptr module_offset,
                                                  ModuleArch arch) {
   CHECK(module_name);
-  const char *is_data_str = is_data ? "DATA " : "";
   if (arch == kModuleArchUnknown) {
-    if (internal_snprintf(buffer_, kBufferSize, "%s\"%s\" 0x%zx\n", is_data_str,
-                          module_name,
+    if (internal_snprintf(buffer_, kBufferSize, "%s \"%s\" 0x%zx\n",
+                          command_prefix, module_name,
                           module_offset) >= static_cast<int>(kBufferSize)) {
       Report("WARNING: Command buffer too small");
       return nullptr;
     }
   } else {
-    if (internal_snprintf(buffer_, kBufferSize, "%s\"%s:%s\" 0x%zx\n",
-                          is_data_str, module_name, ModuleArchToString(arch),
+    if (internal_snprintf(buffer_, kBufferSize, "%s \"%s:%s\" 0x%zx\n",
+                          command_prefix, module_name, ModuleArchToString(arch),
                           module_offset) >= static_cast<int>(kBufferSize)) {
       Report("WARNING: Command buffer too small");
       return nullptr;

compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c

@@ -0,0 +1,23 @@
+// RUN: %clang_hwasan -g %s -o %t && not %run %t 2>&1 | FileCheck %s
+
+// Dynamic allocation of stack objects does not affect FP, so the backend should
+// still be using FP-relative debug info locations that we can use to find stack
+// objects.
+
+__attribute((noinline))
+char *buggy(int b) {
+  char c[64];
+  char *volatile p = c;
+  if (b) {
+    p = __builtin_alloca(64);
+    p = c;
+  }
+  return p;
+}
+
+int main() {
+  char *p = buggy(1);
+  // CHECK: Potentially referenced stack objects:
+  // CHECK-NEXT: c in buggy
+  p[0] = 0;
+}

compiler-rt/test/hwasan/TestCases/stack-uar-realign.c

@@ -0,0 +1,20 @@
+// RUN: %clang_hwasan -g %s -o %t && not %run %t 2>&1 | FileCheck %s
+
+// Dynamic stack realignment causes debug info locations to use non-FP-relative
+// offsets because stack frames are realigned below FP, which means that we
+// can't associate addresses with stack objects in this case. Ideally we should
+// be able to handle this case somehow (e.g. by using a different register for
+// DW_AT_frame_base) but at least we shouldn't get confused by it.
+
+__attribute((noinline))
+char *buggy() {
+  _Alignas(64) char c[64];
+  char *volatile p = c;
+  return p;
+}
+
+int main() {
+  char *p = buggy();
+  // CHECK-NOT: Potentially referenced stack objects:
+  p[0] = 0;
+}

compiler-rt/test/hwasan/TestCases/stack-uar.c

@@ -1,6 +1,6 @@
 // Tests use-after-return detection and reporting.
-// RUN: %clang_hwasan -O0 -fno-discard-value-names %s -o %t && not %run %t 2>&1 | FileCheck %s
-// RUN: %clang_hwasan -O0 -fno-discard-value-names %s -o %t && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
+// RUN: %clang_hwasan -g %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang_hwasan -g %s -o %t && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
 
 // REQUIRES: stable-runtime
 
@@ -28,19 +28,16 @@ int main() {
   // CHECK: READ of size 1 at
   // CHECK: #0 {{.*}} in main{{.*}}stack-uar.c:[[@LINE-2]]
   // CHECK: is located in stack of thread
-  // CHECK: Previously allocated frames:
-  // CHECK: Unrelated3
-  // CHECK: 16 CCC
-  // CHECK: Unrelated2
-  // CHECK: 12 BB
-  // CHECK: Unrelated1
-  // CHECK: 8 A
-  // CHECK: buggy
-  // CHECK: 4096 zzz
+  // CHECK: Potentially referenced stack objects:
+  // CHECK-NEXT: zzz in buggy {{.*}}stack-uar.c:[[@LINE-19]]
+  // CHECK-NEXT: Memory tags around the buggy address
 
   // NOSYM: Previously allocated frames:
-  // NOSYM-NEXT: sp: 0x{{.*}} #0 0x{{.*}} ({{.*}}/stack-uar.c.tmp+0x{{.*}}){{$}}
-  // NOSYM-NEXT: 16 CCC;
+  // NOSYM-NEXT: record_addr:0x{{.*}} record:0x{{.*}} ({{.*}}/stack-uar.c.tmp+0x{{.*}}){{$}}
+  // NOSYM-NEXT: record_addr:0x{{.*}} record:0x{{.*}} ({{.*}}/stack-uar.c.tmp+0x{{.*}}){{$}}
+  // NOSYM-NEXT: record_addr:0x{{.*}} record:0x{{.*}} ({{.*}}/stack-uar.c.tmp+0x{{.*}}){{$}}
+  // NOSYM-NEXT: record_addr:0x{{.*}} record:0x{{.*}} ({{.*}}/stack-uar.c.tmp+0x{{.*}}){{$}}
+  // NOSYM-NEXT: Memory tags around the buggy address
 
   // CHECK: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
 }