diff --git a/compiler-rt/lib/hwasan/hwasan.h b/compiler-rt/lib/hwasan/hwasan.h --- a/compiler-rt/lib/hwasan/hwasan.h +++ b/compiler-rt/lib/hwasan/hwasan.h @@ -44,6 +44,11 @@ // 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 InstallTrapHandler(); void InstallAtExitHandler(); -const char *GetStackOriginDescr(u32 id, uptr *pc); -const char *GetStackFrameDescr(uptr pc); - void EnterSymbolizer(); void ExitSymbolizer(); bool IsInSymbolizer(); diff --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp --- a/compiler-rt/lib/hwasan/hwasan.cpp +++ b/compiler-rt/lib/hwasan/hwasan.cpp @@ -193,36 +193,6 @@ void UpdateMemoryUsage() {} #endif -struct FrameDescription { - uptr PC; - const char *Descr; -}; - -struct FrameDescriptionArray { - FrameDescription *beg, *end; -}; - -static InternalMmapVectorNoCtor AllFrames; - -void InitFrameDescriptors(uptr b, uptr e) { - FrameDescription *beg = reinterpret_cast(b); - FrameDescription *end = reinterpret_cast(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 @@ 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(); diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp --- a/compiler-rt/lib/hwasan/hwasan_report.cpp +++ b/compiler-rt/lib/hwasan/hwasan_report.cpp @@ -139,6 +139,74 @@ 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(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 the lower 20 bits of FP, 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(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 +306,10 @@ 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++; } }); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h b/compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h @@ -139,7 +139,7 @@ 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; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h +++ b/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 @@ 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 locals; + void Clear(); +}; + class SymbolizerTool; class Symbolizer final { @@ -89,6 +116,7 @@ // 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. diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc @@ -66,6 +66,16 @@ 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_; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h +++ b/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 @@ 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 @@ 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; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc @@ -58,6 +58,16 @@ 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 @@ 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 @@ str = ExtractUptr(str, "\n", &info->size); } +static void ParseSymbolizeFrameOutput(const char *str, + InternalMmapVector *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::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 @@ 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(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(kBufferSize)) { Report("WARNING: Command buffer too small"); return nullptr; diff --git a/compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c b/compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/compiler-rt/test/hwasan/TestCases/stack-uar-realign.c b/compiler-rt/test/hwasan/TestCases/stack-uar-realign.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/compiler-rt/test/hwasan/TestCases/stack-uar.c b/compiler-rt/test/hwasan/TestCases/stack-uar.c --- a/compiler-rt/test/hwasan/TestCases/stack-uar.c +++ b/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 @@ // 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 }