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

[pcc]

Summary

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.

Depends on D63300

Depends on D63468

[eugenis]

compiler-rt/lib/hwasan/hwasan_report.cpp

255 ↗

(On Diff #205238)

Let's give names to the magic numbers - they are ABI details after all.

266 ↗

(On Diff #205238)

This is a bit tricky. Leave a comment that underflow modulo 2^20 is expected when the address is expected when the address is below the local variable?

281 ↗

(On Diff #205238)

I think it might be useful to iterate until the end of the buffer to find more candidates; if anything, that would avoid losing the information that's already there.

This is good enough for now. Some time in the future I think we should refactor this to collect all the candidates (including ex. potential buffer overflows with larger offsets, across multiple small allocation) and sort them by likelihood.

282 ↗

(On Diff #205238)

This function is way too big. Do you mind splitting it?

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h

87 ↗

(On Diff #205238)

group the bools together to reduce the gaps

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h

18 ↗

(On Diff #205238)

is this include necessary here?

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

424 ↗

(On Diff #205238)

remove the space from "FRAME ", it can be added in the callee

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

36 ↗

(On Diff #205238)

The advantage of printing decoded records, as before, is that they can be parsed with all the old symbolization scripts. Explicitly listing the last digits of FP is also sometimes useful to match against register values in the report.

Could we change the format to

[tag], fp:0x12345 #0 PC (module+offset)

#0 is also for compatibility with symbolization scripts.

[eugenis]

Please check what happens in "weird" cases with dynamic stack realignment or dynamic sp adjustment. If one or both of these cases can not be handled with this debug info format, can we at least avoid getting confused by them? Are we simply missing fp-offset in the debug info if the offset is not statically known? Add a test case.

[pcc]

• Address review comments

compiler-rt/lib/hwasan/hwasan_report.cpp

281 ↗

(On Diff #205238)

By the time we get here, we've already visited every frame. We don't stop when we find the first match or anything like that.

Or are you saying that we should print the Previously allocated frames section in all cases (e.g. for manual identification of OOB accesses)?

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h

18 ↗

(On Diff #205238)

No, removed.

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

36 ↗

(On Diff #205238)

Do you think it's important to be compatible with old symbolization scripts here? I think we will probably need a new symbolization script anyway that does all the same things that the runtime is doing to identify stack objects. That's also why I decided to print raw values here, so that we'd be less likely to need to change the line format in the future if the entry format changes.

[pcc]

In D63469#1554412, @eugenis wrote:

Please check what happens in "weird" cases with dynamic stack realignment or dynamic sp adjustment. If one or both of these cases can not be handled with this debug info format, can we at least avoid getting confused by them? Are we simply missing fp-offset in the debug info if the offset is not statically known? Add a test case.

Forgot to address this part. I'll upload a new patch in a bit.

[pcc]

• Add tests covering UAR with dynamic alloca and stack realignment

[pcc]

In D63469#1554412, @eugenis wrote:

Please check what happens in "weird" cases with dynamic stack realignment or dynamic sp adjustment. If one or both of these cases can not be handled with this debug info format, can we at least avoid getting confused by them? Are we simply missing fp-offset in the debug info if the offset is not statically known? Add a test case.

In the case of dynamic sp adjustment, things seem to work just fine (for non-dynamic-alloca variables, which is the only kind that we instrument). I see a DW_OP_fbreg associated with each of the variables.

In the case of dynamic stack realignment, the debug info is expressed relative to another register with the post-realignment frame address (usually x19), which means we end up using e.g. DW_OP_breg19 instead of DW_OP_fbreg in the variable's DW_AT_location. We don't get confused by this sort of debug info because llvm-symbolizer only emits an fp offset if it see a DW_OP_fbreg in the location. I think it should be possible in principle to get this case to work by teaching the backend to use the post-realignment frame address register as DW_AT_frame_base, making the backend prefer DW_OP_fbreg for frames with stack realignment and creating a new intrinsic that returns the value of the DW_AT_frame_base designated register (whose value we would store in the ring buffer).

[eugenis]

LGTM
We will need some kind of offline symbolization solution to go with this.

compiler-rt/lib/hwasan/hwasan_report.cpp

281 ↗

(On Diff #205238)

I think I was confused by the amount of indentation in this function. Looks good now.

165 ↗

(On Diff #206349)

the lower 20 bits of FP

Nit: they are not the lowest 20 bits. Add "significant" or something like that?

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

36 ↗

(On Diff #205238)

sgtm

[pcc]

compiler-rt/lib/hwasan/hwasan_report.cpp

165 ↗

(On Diff #206349)

Reworded to bits 4-19 of FP (bits 0-3 are guaranteed to be zero); should be a little clearer now.

compiler-rt/trunk/lib/hwasan/hwasan.h

 // translation and can be used to store a tag.
 const unsigned kAddressTagShift = 56;
 const uptr kAddressTagMask = 0xFFUL << kAddressTagShift;
 
 // Minimal alignment of the shadow base address. Determines the space available
 // 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;
 }
 
 static inline uptr UntagAddr(uptr tagged_addr) {
   return tagged_addr & ~kAddressTagMask;
 }
 

 void *hwasan_memalign(uptr alignment, uptr size, StackTrace *stack);
 int hwasan_posix_memalign(void **memptr, uptr alignment, uptr size,
                         StackTrace *stack);
 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();
 
 struct SymbolizerScope {
   SymbolizerScope() { EnterSymbolizer(); }
   ~SymbolizerScope() { ExitSymbolizer(); }
 };

compiler-rt/trunk/lib/hwasan/hwasan.cpp

void UpdateMemoryUsage() {
   HwasanFormatMemoryUsage(s);
   internal_strncpy(memory_usage_buffer, s.data(), kMemoryUsageBufferSize - 1);
   memory_usage_buffer[kMemoryUsageBufferSize - 1] = '\0';
 }
 #else
 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;
 
   if (!InitShadow()) {
     Printf("FATAL: HWAddressSanitizer cannot mmap the shadow memory.\n");
     DumpProcessMap();
     Die();

 }
 
 // Interface.
 
 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();
   InitInstrumentation();
 }
 
 void __hwasan_init() {
   CHECK(!hwasan_init_is_running);

compiler-rt/trunk/lib/hwasan/hwasan_report.cpp

if (h.tagged_addr <= tagged_addr &&
         h.tagged_addr + h.requested_size > tagged_addr) {
       *har = h;
       return i + 1;
     }
   }
   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) {
   Decorator d;
   int num_descriptions_printed = 0;
   uptr untagged_addr = UntagAddr(tagged_addr);
 
   // Print some very basic information about the address, if it's a heap.
hwasanThreadList().VisitAllLiveThreads([&](Thread *t) {
     // Very basic check for stack memory.
     if (t->AddrIsInStack(untagged_addr)) {
       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();
 
-      // 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++;
     }
   });
 
   // Print the remaining threads, as an extra information, 1 line per thread.
   hwasanThreadList().VisitAllLiveThreads([&](Thread *t) { t->Announce(); });
 
   if (!num_descriptions_printed)

compiler-rt/trunk/lib/sanitizer_common/sanitizer_ring_buffer.h

public:
   void push(T t) {
     T *next = Next();
     *next = t;
     next++;
     next = (T *)((uptr)next & ~GetStorageSize());
     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;
     StorageIdx -= (sptr)(Idx + 1);
     if (StorageIdx < 0)
       StorageIdx += size();
     return Begin[StorageIdx];
   }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h

 // Generally we should try to avoid calling system library functions during
 // symbolization (and use their replacements from sanitizer_libc.h instead).
 //===----------------------------------------------------------------------===//
 #ifndef SANITIZER_SYMBOLIZER_H
 #define SANITIZER_SYMBOLIZER_H
 
 #include "sanitizer_common.h"
 #include "sanitizer_mutex.h"
+#include "sanitizer_vector.h"
 
 namespace __sanitizer {
 
 struct AddressInfo {
   // Owns all the string members. Storage for them is
   // (de)allocated using sanitizer internal allocator.
   uptr address;
 
struct DataInfo {
   char *name;
   uptr start;
   uptr size;
 
   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 {
  public:
   /// Initialize and return platform-specific implementation of symbolizer
   /// (if it wasn't already initialized).
   static Symbolizer *GetOrInit();
   static void LateInitialize();
   // Returns a list of symbolized frames for a given address (containing
   // 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.
   bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
                                    uptr *module_address);
   const char *GetModuleNameForPc(uptr pc) {
     const char *module_name = nullptr;
     uptr unused;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc

 
 void DataInfo::Clear() {
   InternalFree(module);
   InternalFree(file);
   InternalFree(name);
   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_;
 
 void Symbolizer::InvalidateModuleList() {
   modules_fresh_ = false;
 }
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h

 // symbolizers.
 //
 //===----------------------------------------------------------------------===//
 #ifndef SANITIZER_SYMBOLIZER_INTERNAL_H
 #define SANITIZER_SYMBOLIZER_INTERNAL_H
 
 #include "sanitizer_symbolizer.h"
 #include "sanitizer_file.h"
+#include "sanitizer_vector.h"
 
 namespace __sanitizer {
 
 // Parsing helpers, 'str' is searched for delimiter(s) and a string or uptr
 // is extracted. When extracting a string, a newly allocated (using
 // InternalAlloc) and null-terminataed buffer is returned. They return a pointer
 // to the next characted after the found delimiter.
 const char *ExtractToken(const char *str, const char *delims, char **result);
public:
   }
 
   // The |info| parameter is inout. It is pre-filled with the module base
   // and module offset values.
   virtual bool SymbolizeData(uptr addr, DataInfo *info) {
     UNIMPLEMENTED();
   }
 
+  virtual bool SymbolizeFrame(uptr addr, FrameInfo *info) {
+    return false;
+  }
+
   virtual void Flush() {}
 
   // Return nullptr to fallback to the default platform-specific demangler.
   virtual const char *Demangle(const char *name) {
     return nullptr;
   }
 };
 

 
 // This tool invokes llvm-symbolizer in a subprocess. It should be as portable
 // as the llvm-symbolizer tool is.
 class LLVMSymbolizer : public SymbolizerTool {
  public:
   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;
   char buffer_[kBufferSize];
 };
 
 // Parses one or more two-line strings in the following format:
 //   <function_name>

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

const char *ExtractUptr(const char *str, const char *delims, uptr *result) {
   const char *ret = ExtractToken(str, delims, &buff);
   if (buff != 0) {
     *result = (uptr)internal_atoll(buff);
   }
   InternalFree(buff);
   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);
   uptr prefix_len =
       found_delimiter ? found_delimiter - str : internal_strlen(str);
   *result = (char *)InternalAlloc(prefix_len + 1);
   internal_memcpy(*result, str, prefix_len);
   (*result)[prefix_len] = '\0';
for (auto &tool : tools_) {
     SymbolizerScope sym_scope(this);
     if (tool.SymbolizeData(addr, info)) {
       return true;
     }
   }
   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_);
   const char *internal_module_name = nullptr;
   ModuleArch arch;
   if (!FindModuleNameAndOffsetForAddress(pc, &internal_module_name,
                                          module_address, &arch))
     return false;

 //   <start_address> <size>
 // Used by LLVMSymbolizer and InternalSymbolizer.
 void ParseSymbolizeDataOutput(const char *str, DataInfo *info) {
   str = ExtractToken(str, "\n", &info->name);
   str = ExtractUptr(str, " ", &info->start);
   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;
   }
   return false;
 }
 
 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.
     return true;
   }
   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;
     }
   }
   return symbolizer_process_->SendCommand(buffer_);
 }
 

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

+// 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/trunk/test/hwasan/TestCases/stack-uar-realign.c

+// 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/trunk/test/hwasan/TestCases/stack-uar.c

 // 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
 
 void USE(void *x) { // pretend_to_do_something(void *x)
   __asm__ __volatile__("" : : "r" (x) : "memory");
 }
 
 __attribute__((noinline))
int main() {
   char *p = buggy();
   Unrelated1();
   Unrelated2();
   Unrelated3();
   return *p;
   // 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
 }