[PATCH] D58861: [NFC][Sanitizer] Cleanup ASan's GetStackTrace implementation

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka requested changes to this revision.
vitalybuka added a comment.
This revision now requires changes to proceed.

> ways. My goal is to parameterize this algorithm (via templating or callbacks) so we can share the implementation and get rid of the inversed dependency (sanitizer_common depends on

concrete implementations in asan, ubsan, etc.). This should also help us to avoid those pesky linker errors caused by undefined, duplicate, and weak symbols on Windows.

I like what you have already done, but I don't think we are looking to remove inverse deps. Historically it's how compiler-rt was made, and fixing just unwinder will not make a difference.
Spreading this to other parts seems is a lot of work.



================
Comment at: compiler-rt/lib/asan/asan_stack.cc:52
+    uptr bottom = t->stack_bottom();
+    if (IsValidFrame(bp, top, bottom)) {
+      UnwindFast(pc, bp, top, bottom, max_depth);
----------------
Originally the patch was reverted when IsValidFrame was added like this
https://reviews.llvm.org/D18690?vs=on&id=52346&whitespace=ignore-most#toc

Also given that you've commented out top/bottom checks r354718 I don't want to enabled IsValidFrame everywhere.
Could you please keep it for MIPS, and return to this later, if you still want this, after resolving r354718


================
Comment at: compiler-rt/lib/asan/asan_stack.cc:56
+#if SANITIZER_CAN_SLOW_UNWIND
+    UnwindSlowWithOptionalContext(pc, context, max_depth);
+#endif
----------------
misaligned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58861