[llvm] e26b25f - [HWASan] Add sizeof(global) in report even if symbols missing.

Mitch Phillips via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 9 13:03:12 PDT 2020


Author: Mitch Phillips
Date: 2020-06-09T13:02:13-07:00
New Revision: e26b25f8b1fdbc088abdfab933d909960989c64b

URL: https://github.com/llvm/llvm-project/commit/e26b25f8b1fdbc088abdfab933d909960989c64b
DIFF: https://github.com/llvm/llvm-project/commit/e26b25f8b1fdbc088abdfab933d909960989c64b.diff

LOG: [HWASan] Add sizeof(global) in report even if symbols missing.

Summary: Refactor the current global header iteration to be callback-based, and add a feature that reports the size of the global variable during reporting. This allows binaries without symbols to still report the size of the global variable, which is always available in the HWASan globals PT_NOTE metadata.

Reviewers: eugenis, pcc

Reviewed By: pcc

Subscribers: mgorny, llvm-commits, #sanitizers

Tags: #sanitizers, #llvm

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

Added: 
    compiler-rt/lib/hwasan/hwasan_globals.cpp
    compiler-rt/lib/hwasan/hwasan_globals.h

Modified: 
    compiler-rt/lib/hwasan/CMakeLists.txt
    compiler-rt/lib/hwasan/hwasan.cpp
    compiler-rt/lib/hwasan/hwasan_report.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_common.h
    compiler-rt/test/hwasan/TestCases/global.c
    llvm/utils/gn/build/libs/pthread/BUILD.gn
    llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/hwasan/CMakeLists.txt b/compiler-rt/lib/hwasan/CMakeLists.txt
index 03863e4be68d..d294579c970c 100644
--- a/compiler-rt/lib/hwasan/CMakeLists.txt
+++ b/compiler-rt/lib/hwasan/CMakeLists.txt
@@ -6,6 +6,7 @@ set(HWASAN_RTL_SOURCES
   hwasan_allocator.cpp
   hwasan_dynamic_shadow.cpp
   hwasan_exceptions.cpp
+  hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S
   hwasan_linux.cpp
@@ -29,6 +30,7 @@ set(HWASAN_RTL_HEADERS
   hwasan_dynamic_shadow.h
   hwasan_flags.h
   hwasan_flags.inc
+  hwasan_globals.h
   hwasan_interface_internal.h
   hwasan_malloc_bisect.h
   hwasan_mapping.h

diff  --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp
index 6f64829cc5ad..d67a88d455ef 100644
--- a/compiler-rt/lib/hwasan/hwasan.cpp
+++ b/compiler-rt/lib/hwasan/hwasan.cpp
@@ -12,8 +12,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "hwasan.h"
+
 #include "hwasan_checks.h"
 #include "hwasan_dynamic_shadow.h"
+#include "hwasan_globals.h"
 #include "hwasan_poisoning.h"
 #include "hwasan_report.h"
 #include "hwasan_thread.h"
@@ -194,93 +196,20 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
          request_fast);
 }
 
-struct hwasan_global {
-  s32 gv_relptr;
-  u32 info;
-};
-
-static void InitGlobals(const hwasan_global *begin, const hwasan_global *end) {
-  for (auto *desc = begin; desc != end; ++desc) {
-    uptr gv = reinterpret_cast<uptr>(desc) + desc->gv_relptr;
-    uptr size = desc->info & 0xffffff;
-    uptr full_granule_size = RoundDownTo(size, 16);
-    u8 tag = desc->info >> 24;
-    TagMemoryAligned(gv, full_granule_size, tag);
-    if (size % 16)
-      TagMemoryAligned(gv + full_granule_size, 16, size % 16);
-  }
-}
-
-enum { NT_LLVM_HWASAN_GLOBALS = 3 };
-
-struct hwasan_global_note {
-  s32 begin_relptr;
-  s32 end_relptr;
-};
-
-// Check that the given library meets the code model requirements for tagged
-// globals. These properties are not checked at link time so they need to be
-// checked at runtime.
-static void CheckCodeModel(ElfW(Addr) base, const ElfW(Phdr) * phdr,
-                           ElfW(Half) phnum) {
-  ElfW(Addr) min_addr = -1ull, max_addr = 0;
-  for (unsigned i = 0; i != phnum; ++i) {
-    if (phdr[i].p_type != PT_LOAD)
-      continue;
-    ElfW(Addr) lo = base + phdr[i].p_vaddr, hi = lo + phdr[i].p_memsz;
-    if (min_addr > lo)
-      min_addr = lo;
-    if (max_addr < hi)
-      max_addr = hi;
-  }
-
-  if (max_addr - min_addr > 1ull << 32) {
-    Report("FATAL: HWAddressSanitizer: library size exceeds 2^32\n");
-    Die();
-  }
-  if (max_addr > 1ull << 48) {
-    Report("FATAL: HWAddressSanitizer: library loaded above address 2^48\n");
-    Die();
-  }
-}
-
-static void InitGlobalsFromPhdrs(ElfW(Addr) base, const ElfW(Phdr) * phdr,
-                                 ElfW(Half) phnum) {
-  for (unsigned i = 0; i != phnum; ++i) {
-    if (phdr[i].p_type != PT_NOTE)
-      continue;
-    const char *note = reinterpret_cast<const char *>(base + phdr[i].p_vaddr);
-    const char *nend = note + phdr[i].p_memsz;
-    while (note < nend) {
-      auto *nhdr = reinterpret_cast<const ElfW(Nhdr) *>(note);
-      const char *name = note + sizeof(ElfW(Nhdr));
-      const char *desc = name + RoundUpTo(nhdr->n_namesz, 4);
-      if (nhdr->n_type != NT_LLVM_HWASAN_GLOBALS ||
-          internal_strcmp(name, "LLVM") != 0) {
-        note = desc + RoundUpTo(nhdr->n_descsz, 4);
-        continue;
-      }
-
-      // Only libraries with instrumented globals need to be checked against the
-      // code model since they use relocations that aren't checked at link time.
-      CheckCodeModel(base, phdr, phnum);
-
-      auto *global_note = reinterpret_cast<const hwasan_global_note *>(desc);
-      auto *global_begin = reinterpret_cast<const hwasan_global *>(
-          note + global_note->begin_relptr);
-      auto *global_end = reinterpret_cast<const hwasan_global *>(
-          note + global_note->end_relptr);
-      InitGlobals(global_begin, global_end);
-      return;
-    }
-  }
+static bool InitializeSingleGlobal(const hwasan_global &global) {
+  uptr full_granule_size = RoundDownTo(global.size(), 16);
+  TagMemoryAligned(global.addr(), full_granule_size, global.tag());
+  if (global.size() % 16)
+    TagMemoryAligned(global.addr() + full_granule_size, 16, global.size() % 16);
+  return false;
 }
 
 static void InitLoadedGlobals() {
   dl_iterate_phdr(
-      [](dl_phdr_info *info, size_t size, void *data) {
-        InitGlobalsFromPhdrs(info->dlpi_addr, info->dlpi_phdr,
-                             info->dlpi_phnum);
+      [](dl_phdr_info *info, size_t /* size */, void * /* data */) -> int {
+        for (const hwasan_global &global : HwasanGlobalsFor(
+                 info->dlpi_addr, info->dlpi_phdr, info->dlpi_phnum))
+          InitializeSingleGlobal(global);
         return 0;
       },
       nullptr);
@@ -321,11 +250,13 @@ void __hwasan_init_static() {
   // Fortunately, since this is a statically linked executable we can use the
   // linker-defined symbol __ehdr_start to find the only relevant set of phdrs.
   extern ElfW(Ehdr) __ehdr_start;
-  InitGlobalsFromPhdrs(
-      0,
-      reinterpret_cast<const ElfW(Phdr) *>(
-          reinterpret_cast<const char *>(&__ehdr_start) + __ehdr_start.e_phoff),
-      __ehdr_start.e_phnum);
+  for (const hwasan_global &global : HwasanGlobalsFor(
+           /* base */ 0,
+           reinterpret_cast<const ElfW(Phdr) *>(
+               reinterpret_cast<const char *>(&__ehdr_start) +
+               __ehdr_start.e_phoff),
+           __ehdr_start.e_phnum))
+    InitializeSingleGlobal(global);
 }
 
 void __hwasan_init() {
@@ -384,7 +315,8 @@ void __hwasan_init() {
 
 void __hwasan_library_loaded(ElfW(Addr) base, const ElfW(Phdr) * phdr,
                              ElfW(Half) phnum) {
-  InitGlobalsFromPhdrs(base, phdr, phnum);
+  for (const hwasan_global &global : HwasanGlobalsFor(base, phdr, phnum))
+    InitializeSingleGlobal(global);
 }
 
 void __hwasan_library_unloaded(ElfW(Addr) base, const ElfW(Phdr) * phdr,

diff  --git a/compiler-rt/lib/hwasan/hwasan_globals.cpp b/compiler-rt/lib/hwasan/hwasan_globals.cpp
new file mode 100644
index 000000000000..d71bcd792e1f
--- /dev/null
+++ b/compiler-rt/lib/hwasan/hwasan_globals.cpp
@@ -0,0 +1,91 @@
+//===-- hwasan_globals.cpp ------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of HWAddressSanitizer.
+//
+// HWAddressSanitizer globals-specific runtime.
+//===----------------------------------------------------------------------===//
+
+#include "hwasan_globals.h"
+
+namespace __hwasan {
+
+enum { NT_LLVM_HWASAN_GLOBALS = 3 };
+struct hwasan_global_note {
+  s32 begin_relptr;
+  s32 end_relptr;
+};
+
+// Check that the given library meets the code model requirements for tagged
+// globals. These properties are not checked at link time so they need to be
+// checked at runtime.
+static void CheckCodeModel(ElfW(Addr) base, const ElfW(Phdr) * phdr,
+                           ElfW(Half) phnum) {
+  ElfW(Addr) min_addr = -1ull, max_addr = 0;
+  for (unsigned i = 0; i != phnum; ++i) {
+    if (phdr[i].p_type != PT_LOAD)
+      continue;
+    ElfW(Addr) lo = base + phdr[i].p_vaddr, hi = lo + phdr[i].p_memsz;
+    if (min_addr > lo)
+      min_addr = lo;
+    if (max_addr < hi)
+      max_addr = hi;
+  }
+
+  if (max_addr - min_addr > 1ull << 32) {
+    Report("FATAL: HWAddressSanitizer: library size exceeds 2^32\n");
+    Die();
+  }
+  if (max_addr > 1ull << 48) {
+    Report("FATAL: HWAddressSanitizer: library loaded above address 2^48\n");
+    Die();
+  }
+}
+
+ArrayRef<const hwasan_global> HwasanGlobalsFor(ElfW(Addr) base,
+                                               const ElfW(Phdr) * phdr,
+                                               ElfW(Half) phnum) {
+  // Read the phdrs from this DSO.
+  for (unsigned i = 0; i != phnum; ++i) {
+    if (phdr[i].p_type != PT_NOTE)
+      continue;
+
+    const char *note = reinterpret_cast<const char *>(base + phdr[i].p_vaddr);
+    const char *nend = note + phdr[i].p_memsz;
+
+    // Traverse all the notes until we find a HWASan note.
+    while (note < nend) {
+      auto *nhdr = reinterpret_cast<const ElfW(Nhdr) *>(note);
+      const char *name = note + sizeof(ElfW(Nhdr));
+      const char *desc = name + RoundUpTo(nhdr->n_namesz, 4);
+
+      // Discard non-HWASan-Globals notes.
+      if (nhdr->n_type != NT_LLVM_HWASAN_GLOBALS ||
+          internal_strcmp(name, "LLVM") != 0) {
+        note = desc + RoundUpTo(nhdr->n_descsz, 4);
+        continue;
+      }
+
+      // Only libraries with instrumented globals need to be checked against the
+      // code model since they use relocations that aren't checked at link time.
+      CheckCodeModel(base, phdr, phnum);
+
+      auto *global_note = reinterpret_cast<const hwasan_global_note *>(desc);
+      auto *globals_begin = reinterpret_cast<const hwasan_global *>(
+          note + global_note->begin_relptr);
+      auto *globals_end = reinterpret_cast<const hwasan_global *>(
+          note + global_note->end_relptr);
+
+      return {globals_begin, globals_end};
+    }
+  }
+
+  return {};
+}
+
+}  // namespace __hwasan

diff  --git a/compiler-rt/lib/hwasan/hwasan_globals.h b/compiler-rt/lib/hwasan/hwasan_globals.h
new file mode 100644
index 000000000000..fd7adf7a0588
--- /dev/null
+++ b/compiler-rt/lib/hwasan/hwasan_globals.h
@@ -0,0 +1,49 @@
+//===-- hwasan_globals.h ----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of HWAddressSanitizer.
+//
+// Private Hwasan header.
+//===----------------------------------------------------------------------===//
+
+#ifndef HWASAN_GLOBALS_H
+#define HWASAN_GLOBALS_H
+
+#include <link.h>
+
+#include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
+
+namespace __hwasan {
+// This object should only ever be casted over the global (i.e. not constructed)
+// in the ELF PT_NOTE in order for `addr()` to work correctly.
+struct hwasan_global {
+  // The size of this global variable. Note that the size in the descriptor is
+  // max 1 << 24. Larger globals have multiple descriptors.
+  uptr size() const { return info & 0xffffff; }
+  // The fully-relocated address of this global.
+  uptr addr() const { return reinterpret_cast<uintptr_t>(this) + gv_relptr; }
+  // The static tag of this global.
+  u8 tag() const { return info >> 24; };
+
+  // The relative address between the start of the descriptor for the HWASan
+  // global (in the PT_NOTE), and the fully relocated address of the global.
+  s32 gv_relptr;
+  u32 info;
+};
+
+// Walk through the specific DSO (as specified by the base, phdr, and phnum),
+// and return the range of the [beginning, end) of the HWASan globals descriptor
+// array.
+ArrayRef<const hwasan_global> HwasanGlobalsFor(ElfW(Addr) base,
+                                               const ElfW(Phdr) * phdr,
+                                               ElfW(Half) phnum);
+
+}  // namespace __hwasan
+
+#endif  // HWASAN_GLOBALS_H

diff  --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 21a24c449afd..4608adfe546c 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -11,10 +11,14 @@
 // Error reporting.
 //===----------------------------------------------------------------------===//
 
+#include "hwasan_report.h"
+
+#include <dlfcn.h>
+
 #include "hwasan.h"
 #include "hwasan_allocator.h"
+#include "hwasan_globals.h"
 #include "hwasan_mapping.h"
-#include "hwasan_report.h"
 #include "hwasan_thread.h"
 #include "hwasan_thread_list.h"
 #include "sanitizer_common/sanitizer_allocator_internal.h"
@@ -243,6 +247,42 @@ static bool TagsEqual(tag_t tag, tag_t *tag_ptr) {
   return tag == inline_tag;
 }
 
+// HWASan globals store the size of the global in the descriptor. In cases where
+// we don't have a binary with symbols, we can't grab the size of the global
+// from the debug info - but we might be able to retrieve it from the
+// descriptor. Returns zero if the lookup failed.
+static uptr GetGlobalSizeFromDescriptor(uptr ptr) {
+  // Find the ELF object that this global resides in.
+  Dl_info info;
+  dladdr(reinterpret_cast<void *>(ptr), &info);
+  auto *ehdr = reinterpret_cast<const ElfW(Ehdr) *>(info.dli_fbase);
+  auto *phdr = reinterpret_cast<const ElfW(Phdr) *>(
+      reinterpret_cast<const u8 *>(ehdr) + ehdr->e_phoff);
+
+  // Get the load bias. This is normally the same as the dli_fbase address on
+  // position-independent code, but can be 
diff erent on non-PIE executables,
+  // binaries using LLD's partitioning feature, or binaries compiled with a
+  // linker script.
+  ElfW(Addr) load_bias = 0;
+  for (const auto &phdr :
+       ArrayRef<const ElfW(Phdr)>(phdr, phdr + ehdr->e_phnum)) {
+    if (phdr.p_type != PT_LOAD || phdr.p_offset != 0)
+      continue;
+    load_bias = reinterpret_cast<ElfW(Addr)>(ehdr) - phdr.p_vaddr;
+    break;
+  }
+
+  // Walk all globals in this ELF object, looking for the one we're interested
+  // in. Once we find it, we can stop iterating and return the size of the
+  // global we're interested in.
+  for (const hwasan_global &global :
+       HwasanGlobalsFor(load_bias, phdr, ehdr->e_phnum))
+    if (global.addr() <= ptr && ptr < global.addr() + global.size())
+      return global.size();
+
+  return 0;
+}
+
 void PrintAddressDescription(
     uptr tagged_addr, uptr access_size,
     StackAllocationsRingBuffer *current_stack_allocations) {
@@ -319,9 +359,19 @@ void PrintAddressDescription(
               candidate == left ? "right" : "left", info.size, info.name,
               info.start, info.start + info.size, module_name);
         } else {
-          Printf("%p is located to the %s of a global variable in (%s+0x%x)\n",
-                 untagged_addr, candidate == left ? "right" : "left",
-                 module_name, module_address);
+          uptr size = GetGlobalSizeFromDescriptor(mem);
+          if (size == 0)
+            // We couldn't find the size of the global from the descriptors.
+            Printf(
+                "%p is located to the %s of a global variable in (%s+0x%x)\n",
+                untagged_addr, candidate == left ? "right" : "left",
+                module_name, module_address);
+          else
+            Printf(
+                "%p is located to the %s of a %zd-byte global variable in "
+                "(%s+0x%x)\n",
+                untagged_addr, candidate == left ? "right" : "left", size,
+                module_name, module_address);
         }
         num_descriptions_printed++;
       }

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index ac16e0e47efc..07b307a602c9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -978,6 +978,20 @@ INLINE u32 GetNumberOfCPUsCached() {
   return NumberOfCPUsCached;
 }
 
+template <typename T>
+class ArrayRef {
+ public:
+  ArrayRef() {}
+  ArrayRef(T *begin, T *end) : begin_(begin), end_(end) {}
+
+  T *begin() { return begin_; }
+  T *end() { return end_; }
+
+ private:
+  T *begin_ = nullptr;
+  T *end_ = nullptr;
+};
+
 }  // namespace __sanitizer
 
 inline void *operator new(__sanitizer::operator_new_size_type size,

diff  --git a/compiler-rt/test/hwasan/TestCases/global.c b/compiler-rt/test/hwasan/TestCases/global.c
index f1aef2290aaa..7b6bbad3b191 100644
--- a/compiler-rt/test/hwasan/TestCases/global.c
+++ b/compiler-rt/test/hwasan/TestCases/global.c
@@ -9,9 +9,9 @@ int x = 1;
 
 int main(int argc, char **argv) {
   // RSYM: is located 0 bytes to the right of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp
-  // RNOSYM: is located to the right of a global variable in ({{.*}}global.c.tmp+{{.*}})
+  // RNOSYM: is located to the right of a 4-byte global variable in ({{.*}}global.c.tmp+{{.*}})
   // LSYM: is located 4 bytes to the left of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp
-  // LNOSYM: is located to the left of a global variable in ({{.*}}global.c.tmp+{{.*}})
+  // LNOSYM: is located to the left of a 4-byte global variable in ({{.*}}global.c.tmp+{{.*}})
   // CHECK-NOT: can not describe
   (&x)[atoi(argv[1])] = 1;
 }

diff  --git a/llvm/utils/gn/build/libs/pthread/BUILD.gn b/llvm/utils/gn/build/libs/pthread/BUILD.gn
index 69290ba6b5f8..7708d3146033 100644
--- a/llvm/utils/gn/build/libs/pthread/BUILD.gn
+++ b/llvm/utils/gn/build/libs/pthread/BUILD.gn
@@ -2,11 +2,12 @@ import("//llvm/utils/gn/build/libs/pthread/enable.gni")
 
 config("pthread_config") {
   visibility = [ ":pthread" ]
-  ldflags = [ "-pthread" ]
+  libs = [ "pthread" ]
 }
 
 group("pthread") {
-  if (llvm_enable_threads && current_os != "win") {
-    all_dependent_configs = [ ":pthread_config" ]
+  # On Android, bionic has built-in support for pthreads.
+  if (llvm_enable_threads && current_os != "win" && current_os != "android") {
+    public_configs = [ ":pthread_config" ]
   }
 }

diff  --git a/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn b/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn
index b3b1fdc6a087..5b425a47afdc 100644
--- a/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn
+++ b/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn
@@ -43,6 +43,8 @@ source_set("sources") {
     "hwasan_dynamic_shadow.h",
     "hwasan_exceptions.cpp",
     "hwasan_flags.h",
+    "hwasan_globals.cpp",
+    "hwasan_globals.h",
     "hwasan_interceptors.cpp",
     "hwasan_interceptors_vfork.S",
     "hwasan_interface_internal.h",


        


More information about the llvm-commits mailing list