[PATCH] D52249: [hwasan] Record and display stack history in stack-based reports.

[Kostya Serebryany via Phabricator via llvm-commits]

kcc accepted this revision.
kcc added a comment.
This revision is now accepted and ready to land.

LGTM with some nits (feel free to ignore any of them, if they are wrong)



================
Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cc:229
     return AllocateFromLocalPool(size);
-  return hwasan_malloc(size, &stack);
+  void *res = hwasan_malloc(size, &stack);
+  return res;
----------------
do you need this change? 


================
Comment at: compiler-rt/lib/hwasan/hwasan_report.cc:152
+      auto *sa = t->stack_allocations();
+      uptr frames = Min((uptr)flags()->stack_history_size, sa->size());
+      for (uptr i = 0; i < frames; i++) {
----------------
why not just sa->size()? 


================
Comment at: compiler-rt/lib/hwasan/hwasan_thread_list.h:21
+// * All stack ring buffers are located within (2**kShadowBaseAlignment - 1)
+// sized region adjacent starting at (2**kShadowBaseAlignment) below the shadow.
+// * Each ring buffer has a size of (2**N)*4096 where N is in [0, 8), and is
----------------
Is that 
  starting at (2**kShadowBaseAlignment+1)
?


================
Comment at: compiler-rt/lib/hwasan/hwasan_thread_list.h:27
+// ring buffer,
+//     A_next = (A + sizeof(uptr)) & ((1 << (N + 13)) - 1)
+//   is the address of the next element of that ring buffer (with wrap-around).
----------------
is this 
   (a + sizeof(uptr))) & ~mask ? 
(~ missing)


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h:81
+#if SANITIZER_WORDSIZE == 64
+template <class T>
+class CompactRingBuffer {
----------------
short comment? 


https://reviews.llvm.org/D52249