[PATCH] D58741: [NFC][Sanitizer] Add new BufferedStackTrace::Unwind API

[Julian Lettner via Phabricator via llvm-commits]

yln marked 9 inline comments as done.
yln added inline comments.


================
Comment at: compiler-rt/lib/msan/msan.h:342
+    if (!SANITIZER_CAN_FAST_UNWIND || !fast)                              \
+      size = Min(size, 1);                                                \
+    stack.Unwind(pc, bp, nullptr, fast, size);                            \
----------------
vitalybuka wrote:
> do you need fast = false; here?
Good catch, again! The way it is now is not strictly NFC.
I will remove the second part of the or and inline `fast`.


================
Comment at: compiler-rt/lib/msan/msan.h:352
   if (msan_inited)                                       \
-  GetStackTrace(&stack, kStackTraceMax, pc, bp, nullptr, \
-                common_flags()->fast_unwind_on_fatal)
+    stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_fatal)
 
----------------
vitalybuka wrote:
> Can you add BufferedStackTrace::UnwindOnFatal and read fast_unwind_on_fatal there?
> if possible in followup patch
Ideally we would have Unwind** methods so that we didn't even need an overload with the `request_fast` parameter (which is confusing and easy to misuse). That's another big refactoring however...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58741