[PATCH] D58696: [NFC][Sanitizer] Cleanup GetStackTrace implementations

Julian Lettner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 26 17:54:01 PST 2019


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


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h:108
+    if (context)
+      UnwindSlow(pc, max_depth);
+    else
----------------
this bug is fixed


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h:166
 
+#if SANITIZER_CAN_FAST_UNWIND
+  #define UNWIND_FAST(stack, pc, bp, top, bottom, max_depth) \
----------------
vitalybuka wrote:
> yln wrote:
> > vitalybuka wrote:
> > > Can you please undo this macro? On first look I don't see value in them.
> > > If you want to try convince me please try new patch with macro change only.
> > The only reason for these macros is that we can't directly do a call like `stack->UnwindFast(..)` from other code since it might not be available on a platform (not even a dummy symbol).
> > 
> > Let me know if you think that having dummy implementation that errors with "not supported" is preferred over having these macros.
> dummy implementation as we do for the rest of sanitizers
Note that the macro approach is what's currently there. These macros just make it more explicit. I am happy to change this over to defining dummy symbols. Can I do this as a follow-up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58696





More information about the llvm-commits mailing list