[PATCH] D58555: [NFC][Sanitizer] Add argument checks to BufferedStackTrace::Unwind* functions

Julian Lettner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 25 11:02:58 PST 2019


yln marked an inline comment as done.
yln added inline comments.


================
Comment at: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.h:59-60
   static bool WillUseFastUnwind(bool request_fast_unwind) {
+    static_assert(SANITIZER_CAN_FAST_UNWIND || SANITIZER_CAN_SLOW_UNWIND,
+                  "Neither fast nor slow unwinder is supported");
     if (!SANITIZER_CAN_FAST_UNWIND)
----------------
rtrieu wrote:
> rtrieu wrote:
> > Is this suppose to be a run time check or a compile time check?  The location of this assert makes it look like a run time check, only checking when the function is called, but that would require a regular assert instead of a static assert.  If this is meant to be a compile time check as it is now, it would be clearer that it appears right after the two macros are defined rather than inside this function.
> > 
> > I think a run-time check is better to prevent compile errors from merely including this file without calling this function.
> I've removed this assert for the time being so it doesn't block anyone over the weekend.  r354720
This was meant to be a sanity check at compile time. I was completely unaware that there are platform/architectures that don't support any unwinder. How do we handle such cases?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58555





More information about the llvm-commits mailing list