[UBsan] Dumping call stacks when reporting bad-cast (-fsanitize=vptr)

[byoungyoung]

Summary

When UBsan reports a bad-casting, this patch additional dumps call stacks in the report. To avoid the online symbolization, ubsan_init() is also added to initialize an empty symbolizer.

[samsonov]

Thanks for working on this. I will take a look at it shortly.

[byoungyoung]

Updated the patch including a test case. The environment variable, UBSAN_NO_SYMBOLIZE, is used to selectively turn off the online symbolization.

[samsonov]

Once again, sorry for the delay.

lib/ubsan/ubsan_diag.cc

40

I'm opposed to this. Instead, you should use the value of "symbolize" flag in sanitizer_common. Probably it's time to introduce UBSAN_OPTIONS similar to another sanitizers. I will take a look at existing code and return with more comments.

lib/ubsan/ubsan_diag.h

29

And again, it's better to make use of fast_unwind_on_fatal flag from sanitizer_common.

lib/ubsan/ubsan_handlers_cxx.cc

71

It makes sense to hide this feature under a runtime flag.

[samsonov]

lib/ubsan/ubsan_handlers_cxx.cc

38

Required early initialization of UBSan is a separate problem. Let's deal with it later, in subsequent patches.

[samsonov]

FYI I'm working on a slightly modified version of this patch. Will update this thread tomorrow.

lib/ubsan/ubsan_diag.h

 #define UBSAN_DIAG_H
 
 #include "ubsan_value.h"
+#include "sanitizer_common/sanitizer_stacktrace.h"
 
 namespace __ubsan {
 
+#define MAX_PRINT_STACK_DEPTH 50
+
+void InitializeSanitizerCommon();
+
+// \brief Print the current stack traces while skipping the caller's frame.
+ALWAYS_INLINE void PrintCurrentStack() {
+  StackTrace stack;
+  stack.Unwind(MAX_PRINT_STACK_DEPTH, GET_CALLER_PC(), GET_CURRENT_FRAME(),
+               0, 0, 0, false);
+  stack.Print();
+}
+
 /// \brief A location within a loaded module in the program. These are used when
 /// the location can't be resolved to a SourceLocation.
 class ModuleLocation {

lib/ubsan/ubsan_diag.cc

 
 using namespace __ubsan;
 
-static void InitializeSanitizerCommon() {
+void __ubsan::InitializeSanitizerCommon() {
   static StaticSpinMutex init_mu;
   SpinMutexLock l(&init_mu);
   static bool initialized;

     CommonFlags *cf = common_flags();
     SetCommonFlagsDefaults(cf);
     cf->print_summary = false;
+
+    // Initialize symbolizer with an empty string to avoid aggressive online
+    // symbolizations.
+    if (GetEnv("UBSAN_NO_SYMBOLIZE"))
+      Symbolizer::Init("");
   }
   initialized = true;
 }

lib/ubsan/ubsan_handlers_cxx.cc

 static void HandleDynamicTypeCacheMiss(
     DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash,
     bool Abort) {
+  // The initialization routine has to be invoked in the early process runinng
+  // phase (before PrepareForSandboxing() get invoked), but UBsan does not
+  // interpose any process startup functions (e.g., .preinit_array for Linux).
+  //
+  // FIXME: This paranoid try may significantly slowdown the execution due to
+  //        the syncronization in InitializeSanitizerCommon().
+  InitializeSanitizerCommon();
+
   if (checkDynamicType((void*)Pointer, Data->TypeInfo, Hash))
     // Just a cache miss. The type matches after all.
     return;

       << MangledName(DTI.getSubobjectTypeName())
       << Range(Pointer, Pointer + sizeof(uptr), "vptr for %2 base class of %1");
 
+  PrintCurrentStack();
+
   if (Abort)
     Die();
 }

test/ubsan/TestCases/TypeCheck/vptr.cpp

     break;
 
   case 'm':
-    // CHECK-MEMBER: vptr.cpp:[[@LINE+5]]:15: runtime error: member access within address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-MEMBER: vptr.cpp:[[@LINE+7]]:15: runtime error: member access within address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
     // CHECK-MEMBER-NEXT: [[PTR]]: note: object is of type [[DYN_TYPE:'S'|'U']]
     // CHECK-MEMBER-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
     // CHECK-MEMBER-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
     // CHECK-MEMBER-NEXT: {{^              vptr for}} [[DYN_TYPE]]
+    // CHECK-MEMBER-NEXT: #0 [[PTR:0x[0-9a-f]*]] in
+    // CHECK-MEMBER-NEXT: #1 [[PTR:0x[0-9a-f]*]] in
     return p->b;
 
     // CHECK-NULL-MEMBER: vptr.cpp:[[@LINE-2]]:15: runtime error: member access within address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'

     // CHECK-NULL-MEMBER-NEXT: {{^  ?.. .. .. ..  ?00 00 00 00  ?00 00 00 00  ?}}
     // CHECK-NULL-MEMBER-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
     // CHECK-NULL-MEMBER-NEXT: {{^              invalid vptr}}
+    // CHECK-NULL-MEMBER-NEXT: #0 [[PTR:0x[0-9a-f]*]] in
+    // CHECK-NULL-MEMBER-NEXT: #1 [[PTR:0x[0-9a-f]*]] in
 
   case 'f':
-    // CHECK-MEMFUN: vptr.cpp:[[@LINE+5]]:12: runtime error: member call on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-MEMFUN: vptr.cpp:[[@LINE+7]]:12: runtime error: member call on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
     // CHECK-MEMFUN-NEXT: [[PTR]]: note: object is of type [[DYN_TYPE:'S'|'U']]
     // CHECK-MEMFUN-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
     // CHECK-MEMFUN-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
     // CHECK-MEMFUN-NEXT: {{^              vptr for}} [[DYN_TYPE]]
+    // CHECK-MEMFUN-NEXT: #0 [[PTR:0x[0-9a-f]*]] in
+    // CHECK-MEMFUN-NEXT: #1 [[PTR:0x[0-9a-f]*]] in
     return p->g();
 
   case 'o':
-    // CHECK-OFFSET: vptr.cpp:[[@LINE+5]]:12: runtime error: member call on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'U'
+    // CHECK-OFFSET: vptr.cpp:[[@LINE+7]]:12: runtime error: member call on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'U'
     // CHECK-OFFSET-NEXT: 0x{{[0-9a-f]*}}: note: object is base class subobject at offset {{8|16}} within object of type [[DYN_TYPE:'U']]
     // CHECK-OFFSET-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  .. .. .. .. .. .. .. ..  .. .. .. .. .. .. .. ..  }}
     // CHECK-OFFSET-NEXT: {{^              \^                        (                         ~~~~~~~~~~~~)?~~~~~~~~~~~ *$}}
     // CHECK-OFFSET-NEXT: {{^                                       (                         )?vptr for}} 'T' base class of [[DYN_TYPE]]
+    // CHECK-OFFSET-NEXT: #0 [[PTR:0x[0-9a-f]*]] in
+    // CHECK-OFFSET-NEXT: #1 [[PTR:0x[0-9a-f]*]] in
     return reinterpret_cast<U*>(p)->v() - 2;
 
   case 'c':
-    // CHECK-DOWNCAST: vptr.cpp:[[@LINE+5]]:5: runtime error: downcast of address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-DOWNCAST: vptr.cpp:[[@LINE+7]]:5: runtime error: downcast of address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
     // CHECK-DOWNCAST-NEXT: [[PTR]]: note: object is of type [[DYN_TYPE:'S'|'U']]
     // CHECK-DOWNCAST-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
     // CHECK-DOWNCAST-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
     // CHECK-DOWNCAST-NEXT: {{^              vptr for}} [[DYN_TYPE]]
+    // CHECK-DOWNCAST-NEXT: #0 [[PTR:0x[0-9a-f]*]] in
+    // CHECK-DOWNCAST-NEXT: #1 [[PTR:0x[0-9a-f]*]] in
     static_cast<T*>(reinterpret_cast<S*>(p));
     return 0;
   }