[PATCH] [compiler-rt] bug 23600 - sanitizer stack trace pc off by 1

[Alexey Samsonov]

Sorry for delayed response, I lost track of this patch for some reason =/


================
Comment at: lib/sanitizer_common/sanitizer_stacktrace.h:47
@@ +46,3 @@
+  // Nonzero if the stack trace was generated in response to a signal.
+  bool signaled;
+
----------------
I'm still concerned about us failing to propagate this field. StackTrace objects, for instance, are stored in / fetched from StackDepot, that would do nothing to save this "signaled" bit. If you use it only in `Print` method, perhaps you would now at call site where did you obtain this stack trace from, and whether you need to "adjust" the top frame.

That is, `StackTrace::Unwind` is not the right place to determine where the stack comes from - if `context` is missing, it means nothing - the context only serves as a hint for slow unwinder, and caller is not obliged to pass it down, if it has copied pc/bp from the context himself.

================
Comment at: lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc:44
@@ -35,1 +43,3 @@
     for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
+      // Restore the PC to match the stack trace.
+      cur->info.address += adjust;
----------------
Why do you need to adjust PCs in the error report? This would break offline symbolization - scripts that process ASan reports and provide file/line information would *not* calculate GetPreviousInstructionPc() on their own - they rely on ASan to give them to PCs of call instructions, not return addresses we see in the stack trace.

http://reviews.llvm.org/D10065

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/