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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/lib/hwasan/hwasan.cpp:211
+        for (const hwasan_global &global : HwasanGlobalsFor(
+                 info->dlpi_addr, info->dlpi_phdr, info->dlpi_phnum)) {
+          InitializeSingleGlobal(global);
----------------
Remove braces


================
Comment at: compiler-rt/lib/hwasan/hwasan.cpp:259
+               __ehdr_start.e_phoff),
+           __ehdr_start.e_phnum)) {
+    InitializeSingleGlobal(global);
----------------
Remove braces


================
Comment at: compiler-rt/lib/hwasan/hwasan.cpp:320
                              ElfW(Half) phnum) {
-  InitGlobalsFromPhdrs(base, phdr, phnum);
+  for (const hwasan_global &global : HwasanGlobalsFor(base, phdr, phnum)) {
+    InitializeSingleGlobal(global);
----------------
Remove braces


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.cpp:92
+}  // namespace __hwasan
\ No newline at end of file

----------------
Newline


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:27
+  // The size of this global variable. Note that the size in the descriptor is
+  // max 2 << 16. When this occurs, we split the global up to have multiple
+  // descriptors.
----------------
2^24, "larger globals have multiple descriptors"


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:37
+  // global (in the PT_NOTE), and the fully relocated address of the global.
+  s32 relptr;
+  u32 info;
----------------
Rename this back to gv_relptr.


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:51
+#endif  // HWASAN_GLOBALS_H
\ No newline at end of file

----------------
Newline


================
Comment at: compiler-rt/lib/hwasan/hwasan_report.cpp:266
+  for (const hwasan_global &global : HwasanGlobalsFor(
+           reinterpret_cast<ElfW(Addr)>(ehdr), phdr, ehdr->e_phnum)) {
+    if (global.addr() <= ptr && ptr < global.addr() + global.size()) {
----------------
Remove braces


================
Comment at: compiler-rt/lib/hwasan/hwasan_report.cpp:266
+  for (const hwasan_global &global : HwasanGlobalsFor(
+           reinterpret_cast<ElfW(Addr)>(ehdr), phdr, ehdr->e_phnum)) {
+    if (global.addr() <= ptr && ptr < global.addr() + global.size()) {
----------------
pcc wrote:
> Remove braces
I'm not sure that this first argument can be `ehdr` -- it needs to be the load bias, which is normally the same as `ehdr` in position-independent binaries, but it can be different in e.g. non-PIE executables, binaries created using lld's partitioning feature [1] and possibly also binaries linked using a linker script.

To compute the load bias from the address of the ELF header, you can look for a PT_LOAD with p_offset=0. The load bias is found by subtracting that program header's p_vaddr from the address of the ELF header.

[1] https://lld.llvm.org/Partitions.html


================
Comment at: compiler-rt/lib/hwasan/hwasan_report.cpp:352
+          uptr size = GetGlobalSizeFromDescriptor(mem);
+          if (size == 0)
+            Printf(
----------------
Is `size == 0` realistic? I don't think you can declare a zero-size global in C.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80599/new/

https://reviews.llvm.org/D80599