[PATCH] D58536: [Sanitizer] Rework BufferedStackTrace::Unwind (part 1)

[Julian Lettner via Phabricator via llvm-commits]

yln created this revision.
Herald added subscribers: llvm-commits, Sanitizers, jdoerfert, jrtc27, fedor.sergeev, kubamracek, jyknight.
Herald added projects: Sanitizers, LLVM.

In a previous revision (https://reviews.llvm.org/D58156) I fixed a few
failing tests on Darwin involving stack printing.

The underlying issue has to do with stack unwinding. We have the notion
of a slow and a fast unwinder. Linux has both, on Darwin only the fast
unwinder is available.

Considering this, the current API is hard to use correctly.

  BufferedStackTrace::Unwind(u32 max_depth, uptr pc, uptr bp, void *context,
                             uptr stack_top, uptr stack_bottom,
                             bool request_fast_unwind)

The only thing the slow unwinder needs to work is `pc`. It can also take
an optional `context`. The fast unwinder additionally requires
`bp`,`stack_top`, `stack_bottom`. The `context` is not applicable to the
fast unwinder.

The semantics of `request_fast_unwind` is really only a "request" to use
the fast unwinder. On platforms that only support one kind of unwinder
it is essentially ignored.

This means that code like below doesn't work on Darwin.

  bool fast = common_flags()->fast_unwind_on_fatal;
  if (fast)
    GetThreadStackTopAndBottom(..., &top, &bottom);
  
  stack.Unwind(..., top, bottom, fast);
  // May use fast unwinder, but top and bottom haven't been provided.
  
  // Even the following is broken
  stack.Unwind(..., 0, 0, false);

The function `StackTrace::WillUseFastUnwind` was introduced to remedy
this. In my last patch, I applied it some places that caused test
failures. This patch adds a few more calls.

However, ultimately `BufferedStackTrace::Unwind` is not a good API. (I
guess it just grew more complex over time.) It tries to hide which
unwinder is used, but to use it correctly the caller needs to be aware
which unwinder is going to be used. Its set of parameters is the union
of required arguments of both unwinders.

This patch removes `Unwind` and directly calls the fast or slow unwind
functions. This change (part 1) is purely mechanical. In a follow-up I
want to try to cleanup the call sites. Maybe it is possible to hide most
of the details behind yet another macro.

rdar://problem/48177534


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58536

Files:
  compiler-rt/lib/asan/asan_stack.h
  compiler-rt/lib/hwasan/hwasan.cc
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/msan/msan.cc
  compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc
  compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h
  compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc
  compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cc
  compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cc
  compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cc
  compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cc
  compiler-rt/lib/sanitizer_common/sanitizer_unwind_win.cc
  compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cc
  compiler-rt/lib/tsan/dd/dd_rtl.cc
  compiler-rt/lib/tsan/rtl/tsan_rtl.cc
  compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
  compiler-rt/lib/ubsan/ubsan_diag.cc
  compiler-rt/lib/ubsan/ubsan_diag_standalone.cc
  compiler-rt/test/sanitizer_common/TestCases/Posix/dedup_token_length_test.cc

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58536.187895.patch
Type: text/x-patch
Size: 18148 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190222/965e3be8/attachment.bin>