[PATCH] D63841: [GWP-ASan] Add generic unwinders and structure backtrace output.

[Mitch Phillips via Phabricator via llvm-commits]

hctim marked 4 inline comments as done.
hctim added a comment.

In D63841#1561393 <https://reviews.llvm.org/D63841#1561393>, @eugenis wrote:

> We will need something different for out-of-process unwinding, which is AFAIK a requirement both in android and chrome.


It looks like `malloc_debug` in Android uses in-process unwinding, based on `libunwindstack`. My assumption was that we would happily do it in-process there. Chromium also unwinds in-process AFAICT.

Out-of-process crash handling will be platform dependent, but we will likely have to provide some hooks at a later date to support this.



================
Comment at: compiler-rt/lib/gwp_asan/CMakeLists.txt:140
+      PARENT_TARGET gwp_asan)
+  endforeach()
 endif()
----------------
eugenis wrote:
> That's a lot of libraries. We need at most two - one standalone implementation for testing, and one "plugin" for the actual users.
The only problem here is that I'd expect people to mix-and-match these.

For example, Scudo is currently using the options parser, and the libc unwinder. Soon enough they'll be using the libc unwinder and providing their own options parser. I think we need to keep them decoupled to allow people to pick what they need (hence the `optional` dir).


================
Comment at: compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp:32
+  Trace.Reset();
+  assert(Size <= __sanitizer::kStackTraceMax &&
+         "GWP-ASan asking for higher stack depth than supported by "
----------------
vlad.tsyrklevich wrote:
> Why not std::min() instead of assert(), otherwise all backtrace implementations are beholden to __sanitizer::kStackTraceMax
Done, with the caveat that we don't have c++ headers so have to do this ourselves ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63841