[UBSan] Clarify the way we disable de-duplication of reports from unrecoverable handlers.

Let unrecoverable handlers be responsbile for killing the
program with Die(), and let functions which print the error
report know if it's going to happen. Re-write the comments to
describe the situation.

llvm-svn: 255081

compiler-rt/lib/ubsan/ubsan_diag.cc

@@ -365,7 +365,7 @@ ScopedReport::~ScopedReport() {
   MaybePrintStackTrace(Opts.pc, Opts.bp);
   MaybeReportErrorSummary(SummaryLoc, Type);
   CommonSanitizerReportMutex.Unlock();
-  if (Opts.DieAfterReport || flags()->halt_on_error)
+  if (flags()->halt_on_error)
     Die();
 }
 

compiler-rt/lib/ubsan/ubsan_diag.h

@@ -211,9 +211,9 @@ public:
 };
 
 struct ReportOptions {
-  /// If DieAfterReport is specified, UBSan will terminate the program after the
+  // If FromUnrecoverableHandler is specified, UBSan runtime handler is not
-  /// report is printed.
+  // expected to return.
-  bool DieAfterReport;
+  bool FromUnrecoverableHandler;
   /// pc/bp are used to unwind the stack trace.
   uptr pc;
   uptr bp;
@@ -225,9 +225,11 @@ enum class ErrorType {
 #undef UBSAN_CHECK
 };
 
-#define GET_REPORT_OPTIONS(die_after_report) \
+bool ignoreReport(SourceLocation SLoc, ReportOptions Opts);
+
+#define GET_REPORT_OPTIONS(unrecoverable_handler) \
     GET_CALLER_PC_BP; \
-    ReportOptions Opts = {die_after_report, pc, bp}
+    ReportOptions Opts = {unrecoverable_handler, pc, bp}
 
 /// \brief Instantiate this class before printing diagnostics in the error
 /// report. This class ensures that reports from different threads and from

compiler-rt/lib/ubsan/ubsan_handlers.cc

@@ -21,17 +21,20 @@
 using namespace __sanitizer;
 using namespace __ubsan;
 
-static bool ignoreReport(SourceLocation SLoc, ReportOptions Opts) {
+namespace __ubsan {
-  // If source location is already acquired, we don't need to print an error
+bool ignoreReport(SourceLocation SLoc, ReportOptions Opts) {
-  // report for the second time. However, if we're in an unrecoverable handler,
+  // We are not allowed to skip error report: if we are in unrecoverable
-  // it's possible that location was required by concurrently running thread.
+  // handler, we have to terminate the program right now, and therefore
-  // In this case, we should continue the execution to ensure that any of
+  // have to print some diagnostic.
-  // threads will grab the report mutex and print the report before
+  //
-  // crashing the program.
+  // Even if source location is disabled, it doesn't mean that we have
-  return SLoc.isDisabled() && !Opts.DieAfterReport;
+  // already report an error to the user: some concurrently running
+  // thread could have acquired it, but not yet printed the report.
+  if (Opts.FromUnrecoverableHandler)
+    return false;
+  return SLoc.isDisabled();
 }
 
-namespace __ubsan {
 const char *TypeCheckKinds[] = {
     "load of", "store to", "reference binding to", "member access within",
     "member call on", "constructor call on", "downcast of", "downcast of",
@@ -116,12 +119,13 @@ static void handleIntegerOverflowImpl(OverflowData *Data, ValueHandle LHS,
     << Value(Data->Type, LHS) << Operator << RHS << Data->Type;
 }
 
-#define UBSAN_OVERFLOW_HANDLER(handler_name, op, abort)                        \
+#define UBSAN_OVERFLOW_HANDLER(handler_name, op, unrecoverable)                \
   void __ubsan::handler_name(OverflowData *Data, ValueHandle LHS,              \
                              ValueHandle RHS) {                                \
-    GET_REPORT_OPTIONS(abort);                                                 \
+    GET_REPORT_OPTIONS(unrecoverable);                                         \
     handleIntegerOverflowImpl(Data, LHS, op, Value(Data->Type, RHS), Opts);    \
-    if (abort) Die();                                                          \
+    if (unrecoverable)                                                         \
+      Die();                                                                   \
   }
 
 UBSAN_OVERFLOW_HANDLER(__ubsan_handle_add_overflow, "+", false)

compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc

@@ -29,21 +29,22 @@ namespace __ubsan {
   extern const char *TypeCheckKinds[];
 }
 
-static void HandleDynamicTypeCacheMiss(
+// Returns true if UBSan has printed an error report.
+static bool HandleDynamicTypeCacheMiss(
     DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash,
     ReportOptions Opts) {
   if (checkDynamicType((void*)Pointer, Data->TypeInfo, Hash))
     // Just a cache miss. The type matches after all.
-    return;
+    return false;
 
   // Check if error report should be suppressed.
   DynamicTypeInfo DTI = getDynamicTypeInfoFromObject((void*)Pointer);
   if (DTI.isValid() && IsVptrCheckSuppressed(DTI.getMostDerivedTypeName()))
-    return;
+    return false;
 
   SourceLocation Loc = Data->Loc.acquire();
   if (Loc.isDisabled())
-    return;
+    return false;
 
   ScopedReport R(Opts, Loc, ErrorType::DynamicTypeMismatch);
 
@@ -69,6 +70,7 @@ static void HandleDynamicTypeCacheMiss(
         << TypeName(DTI.getSubobjectTypeName())
         << Range(Pointer, Pointer + sizeof(uptr),
                  "vptr for %2 base class of %1");
+  return true;
 }
 
 void __ubsan::__ubsan_handle_dynamic_type_cache_miss(
@@ -78,13 +80,18 @@ void __ubsan::__ubsan_handle_dynamic_type_cache_miss(
 }
 void __ubsan::__ubsan_handle_dynamic_type_cache_miss_abort(
     DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash) {
-  GET_REPORT_OPTIONS(true);
+  // Note: -fsanitize=vptr is always recoverable.
-  HandleDynamicTypeCacheMiss(Data, Pointer, Hash, Opts);
+  GET_REPORT_OPTIONS(false);
+  if (HandleDynamicTypeCacheMiss(Data, Pointer, Hash, Opts))
+    Die();
 }
 
 static void HandleCFIBadType(CFIBadTypeData *Data, ValueHandle Vtable,
                              ReportOptions Opts) {
   SourceLocation Loc = Data->Loc.acquire();
+
+  if (ignoreReport(Loc, Opts))
+    return;
   ScopedReport R(Opts, Loc, ErrorType::CFIBadType);
   DynamicTypeInfo DTI = getDynamicTypeInfoFromVtable((void*)Vtable);
 
@@ -117,6 +124,7 @@ void __ubsan::__ubsan_handle_cfi_bad_type_abort(CFIBadTypeData *Data,
                                                 ValueHandle Vtable) {
   GET_REPORT_OPTIONS(true);
   HandleCFIBadType(Data, Vtable, Opts);
+  Die();
 }
 
 #endif  // CAN_SANITIZE_UB