[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_globals.cpp:25
+size_t hwasan_global::size() const { return info & 0xffffff; }
+int32_t hwasan_global::relptr() const { return gv_relptr; }
+uintptr_t hwasan_global::addr() const {
----------------
No trivial accessors please.


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.cpp:95
+
+  return {nullptr, nullptr};
+}
----------------
With the ArrayRef ctor changed this becomes `return {}`.


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:24-32
+  // Disallow all constructors here, this object should only ever be casted
+  // over the global in the ELF PT_NOTE (in order for `addr()` to work
+  // correctly).
+  hwasan_global() = delete;
+  ~hwasan_global() = delete;
+  explicit hwasan_global(const hwasan_global &) = delete;
+  explicit hwasan_global(const hwasan_global &&) = delete;
----------------
I wouldn't add this stuff, this type of error is pretty unlikely and would be caught in code review.


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:37
+  // descriptors.
+  uptr size() const;
+  // The relative address between the start of the descriptor for the HWASan
----------------
I would just put the definitions inline so that readers don't need to go hunting for them, they're pretty small.


================
Comment at: compiler-rt/lib/hwasan/hwasan_globals.h:46
+
+  // Members.
+  s32 gv_relptr;
----------------
Remove obvious comment.


================
Comment at: compiler-rt/lib/hwasan/hwasan_report.cpp:252
+// 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 phdr.
+// Returns zero if the lookup failed.
----------------
s/phdr/descriptor/


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common.h:985
+  ArrayRef(T *begin, T *end) : begin_(begin), end_(end) {}
+  ArrayRef() = delete;
+
----------------
Shouldn't it create an empty ArrayRef?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80599