[dfsan] Add stack-trace printing functions to dfsan interface

[vitalybuka]

why __sanitizer_print_stack_trace is not enough?

[stephan.yichao.zhao]

In D104165#2814790, @vitalybuka wrote:

why __sanitizer_print_stack_trace is not enough?

We wanted to record stack traces to some databases for post-process. This is similar to the request of printing dfsan origin traces to a buffer.

[stephan.yichao.zhao]

compiler-rt/lib/dfsan/dfsan.cpp

845

Please replace __sanitizer_print_stack_trace by dfsan_print_stack_trace.

The only difference between them is that __sanitizer_print_stack_trace uses common_flags()->fast_unwind_on_fatal.
I think that controls fast unwind. Since DFSan always runs on linux where fast unwind can also be supported, so that flag is true.

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h

142 ↗

(On Diff #351611)

@vitalybuka: is fine to make this public?

If not, it is still fine for dfsan_sprint_stack_trace, because we can always post-process stack frames to fill out unrelated frames.
I think we need that kind of post-filtering anyway.

[vitalybuka]

In D104165#2814817, @stephan.yichao.zhao wrote:

In D104165#2814790, @vitalybuka wrote:

why __sanitizer_print_stack_trace is not enough?

We wanted to record stack traces to some databases for post-process. This is similar to the request of printing dfsan origin traces to a buffer.

To clarify, I understand why you need dfsan_sprint_stack_trace.
But dfsan_print_stack_trace looks very similar to sanitizer_print_stack_trace.
Please consider to adjust your use-case and just use sanitizer_print_stack_trace.

But please don't wait for my review and proceed with @stephan.yichao.zhao if you disagree.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h

142 ↗

(On Diff #351611)

fine with me

[gbalats]

compiler-rt/lib/dfsan/dfsan.cpp

845

Not sure what the suggestion is. You mean I should remove __sanitizer_print_stack_trace altogether? Or replace its body with a call to dfsan_print_stack_trace? Or something else?

Not sure if removing is okay, since I see a declaration provided in sanitizer/common_interface_defs.h and every *san.cpp provides its own definition. Having one call the other will insert another frame and I really don't want to use/introduce a macro like GET_FATAL_STACK_TRACE_PC_BP that hides the declaration of stack (which makes the code below harder to understand since we call many methods of stack but its type is not evident). Unless we replace it with an inlined function.

[stephan.yichao.zhao]

compiler-rt/lib/dfsan/dfsan.cpp

845

I suggest we keep __sanitizer_print_stack_trace, and could define it like

void __sanitizer_print_stack_trace() {
   GET_CALLER_PC_BP;
   GET_STORE_STACK_TRACE_PC_BP(pc, bp);
   stack.Print();
}

It is fine to remove GET_FATAL_STACK_TRACE_PC_BP because in our case the 4th argument is always true.
GET_STORE_STACK_TRACE_PC_BP has been shared with other code too.
That GET_CALLER_PC_BP equals to stack.PopStackFrames. So we do not need to change sanitizer_stacktrace.h.

[stephan.yichao.zhao]

compiler-rt/test/dfsan/stack_trace.c

52

[gbalats]

Remove dfsan_print_stack_trace

[gbalats]

compiler-rt/lib/dfsan/dfsan.cpp

845

Done. Removed dfsan_print_stack_trace.

compiler-rt/test/dfsan/stack_trace.c

52

Acknowledged. By removing the leading whitespace, it's not evident why tinybuf, of size 8, can hold just these contents.

compiler-rt/include/sanitizer/dfsan_interface.h

 /// \endcode
 ///
 /// Typical usage that handles truncation:
 /// \code
 ///   char buf[1024];
 ///   int len = dfsan_sprint_origin_trace(&var, nullptr, buf, sizeof(buf));
 ///
 ///   if (len < sizeof(buf)) {
-///     ProcessOriginTrace(tmpbuf);
-///   else {
+///     ProcessOriginTrace(buf);
+///   } else {
 ///     char *tmpbuf = new char[len + 1];
 ///     dfsan_sprint_origin_trace(&var, nullptr, tmpbuf, len + 1);
 ///     ProcessOriginTrace(tmpbuf);
 ///     delete[] tmpbuf;
 ///   }
 /// \endcode
 ///
 /// \param addr The tainted memory address whose origin we are printing.
 /// \param description A description printed at the beginning of the trace.
 /// \param [out] out_buf The output buffer to write the results to.
 /// \param out_buf_size The size of \p out_buf.
 ///
 /// \returns The number of symbols that should have been written to \p out_buf
 /// (not including trailing null byte '\0'). Thus, the string is truncated iff
 /// return value is not less than \p out_buf_size.
 size_t dfsan_sprint_origin_trace(const void *addr, const char *description,
                                  char *out_buf, size_t out_buf_size);
 
+/// Prints the stack trace leading to this call to a pre-allocated output
+/// buffer.
+///
+/// For usage examples, see dfsan_sprint_origin_trace.
+///
+/// \param [out] out_buf The output buffer to write the results to.
+/// \param out_buf_size The size of \p out_buf.
+///
+/// \returns The number of symbols that should have been written to \p out_buf
+/// (not including trailing null byte '\0'). Thus, the string is truncated iff
+/// return value is not less than \p out_buf_size.
+size_t dfsan_sprint_stack_trace(char *out_buf, size_t out_buf_size);
+
 /// Retrieves the very first origin associated with the data at the given
 /// address.
 dfsan_origin dfsan_get_init_origin(const void *addr);
 #ifdef __cplusplus
 }  // extern "C"
 
 template <typename T>
 void dfsan_set_label(dfsan_label label, T &data) { // NOLINT
   dfsan_set_label(label, (void *)&data, sizeof(T));
 }
 
 #endif
 
 #endif  // DFSAN_INTERFACE_H

compiler-rt/lib/dfsan/dfsan.cpp

dfsan_get_init_origin(const void *addr) {
   while (o.isChainedOrigin()) {
     StackTrace stack;
     origin_id = o.raw_id();
     o = o.getNextChainedOrigin(&stack);
   }
   return origin_id;
 }
 
-#define GET_FATAL_STACK_TRACE_PC_BP(pc, bp) \
-  BufferedStackTrace stack;                 \
-  stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_fatal);
-
 void __sanitizer::BufferedStackTrace::UnwindImpl(uptr pc, uptr bp,
                                                  void *context,
                                                  bool request_fast,
                                                  u32 max_depth) {
   using namespace __dfsan;
   DFsanThread *t = GetCurrentThread();
   if (!t || !StackTrace::WillUseFastUnwind(request_fast)) {
     return Unwind(max_depth, pc, bp, context, 0, 0, false);
   }
   Unwind(max_depth, pc, bp, nullptr, t->stack_top(), t->stack_bottom(), true);
 }
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_print_stack_trace() {
-  GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME());
+  GET_CALLER_PC_BP;
+  GET_STORE_STACK_TRACE_PC_BP(pc, bp);
   stack.Print();
 }
 
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE size_t
+dfsan_sprint_stack_trace(char *out_buf, size_t out_buf_size) {
+  CHECK(out_buf);
+  GET_CALLER_PC_BP;
+  GET_STORE_STACK_TRACE_PC_BP(pc, bp);
+  return stack.PrintTo(out_buf, out_buf_size);
+}
+
 void Flags::SetDefaults() {
 #define DFSAN_FLAG(Type, Name, DefaultValue, Description) Name = DefaultValue;
 #include "dfsan_flags.inc"
 #undef DFSAN_FLAG
 }
 
 static void RegisterDfsanFlags(FlagParser *parser, Flags *f) {
 #define DFSAN_FLAG(Type, Name, DefaultValue, Description) \

compiler-rt/lib/dfsan/done_abilist.txt

 fun:dfsan_set_write_callback=uninstrumented
 fun:dfsan_set_write_callback=custom
 fun:dfsan_flush=uninstrumented
 fun:dfsan_flush=discard
 fun:dfsan_print_origin_trace=uninstrumented
 fun:dfsan_print_origin_trace=discard
 fun:dfsan_sprint_origin_trace=uninstrumented
 fun:dfsan_sprint_origin_trace=discard
+fun:dfsan_sprint_stack_trace=uninstrumented
+fun:dfsan_sprint_stack_trace=discard
 fun:dfsan_get_origin=uninstrumented
 fun:dfsan_get_origin=custom
 fun:dfsan_get_init_origin=uninstrumented
 fun:dfsan_get_init_origin=discard
 
 
 ###############################################################################
 # glibc

compiler-rt/test/dfsan/stack_trace.c

+// RUN: %clang_dfsan -gmlt %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+//
+// REQUIRES: x86_64-target-arch
+
+#include <assert.h>
+#include <sanitizer/dfsan_interface.h>
+#include <stdio.h>
+#include <string.h>
+
+#define NOINLINE __attribute__((noinline))
+
+NOINLINE size_t bar(int depth, char *buf, size_t len) {
+  if (!depth) {
+    return dfsan_sprint_stack_trace(buf, len);
+  }
+
+  return bar(depth - 1, buf, len);
+}
+
+NOINLINE size_t baz(int depth, char *buf, size_t len) {
+  return bar(depth, buf, len);
+}
+
+int main(int argc, char *argv[]) {
+  char buf[3000];
+  size_t length = dfsan_sprint_stack_trace(buf, sizeof(buf));
+  assert(length < sizeof(buf));
+  printf("==OUTPUT==\n%s==EOS==\n", buf);
+
+  // CHECK: ==OUTPUT==
+  // CHECK: #0 {{.*}} in main [[FILEPATH:.*]]/stack_trace.c:[[# @LINE - 5 ]]
+  // CHECK: ==EOS==
+
+  length = baz(8, buf, sizeof(buf));
+  printf("==OUTPUT==\n%s==EOS==\n", buf);
+
+  // CHECK: ==OUTPUT==
+  // CHECK: #0 {{.*}} in dfs$bar [[FILEPATH]]/stack_trace.c:15
+  // CHECK-COUNT-8: #{{[1-9]+}} {{.*}} in dfs$bar [[FILEPATH]]/stack_trace.c:18
+  // CHECK: #9 {{.*}} in dfs$baz [[FILEPATH]]/stack_trace.c:22
+  // CHECK: #10 {{.*}} in main [[FILEPATH]]/stack_trace.c:[[# @LINE - 7 ]]
+  // CHECK: ==EOS==
+
+  char tinybuf[8];
+  size_t same_length = baz(8, tinybuf, sizeof(tinybuf));
+
+  printf("==TRUNCATED OUTPUT==\n%s==EOS==\n", tinybuf);
+  // CHECK: ==TRUNCATED OUTPUT==
+  // CHECK:     #0 ==EOS==
+
+  printf("Returned length: %zu\n", length);
+  printf("Actual length: %zu\n", strlen(buf));
+  printf("Returned length with truncation: %zu\n", same_length);
+
+  // CHECK: Returned length: [[#LEN:]]
+  // CHECK: Actual length: [[#LEN]]
+  // CHECK: Returned length with truncation: [[#LEN]]
+
+  buf[0] = '\0';
+  length = baz(8, buf, 0);
+  printf("Output=\"%s\"\n", buf);
+  printf("Returned length: %zu\n", length);
+  // CHECK: Output=""
+  // CHECK: Returned length: [[#LEN]]
+}