[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.

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@255081 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 38 insertions, 24 deletions

diff --git a/lib/ubsan/ubsan_diag.cc b/lib/ubsan/ubsan_diag.cc
index f4c1a417d..5cafd2c4b 100644
--- a/lib/ubsan/ubsan_diag.cc
+++ b/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();
 }
 
diff --git a/lib/ubsan/ubsan_diag.h b/lib/ubsan/ubsan_diag.h
index c7f361e37..eae2556a0 100644
--- a/lib/ubsan/ubsan_diag.h
+++ b/lib/ubsan/ubsan_diag.h
@@ -211,9 +211,9 @@ public:
 };
 
 struct ReportOptions {
-  /// If DieAfterReport is specified, UBSan will terminate the program after the
-  /// report is printed.
-  bool DieAfterReport;
+  // If FromUnrecoverableHandler is specified, UBSan runtime handler is not
+  // expected to return.
+  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
diff --git a/lib/ubsan/ubsan_handlers.cc b/lib/ubsan/ubsan_handlers.cc
index 69bde0dca..b85a5aba8 100644
--- a/lib/ubsan/ubsan_handlers.cc
+++ b/lib/ubsan/ubsan_handlers.cc
@@ -21,17 +21,20 @@
 using namespace __sanitizer;
 using namespace __ubsan;
 
-static bool ignoreReport(SourceLocation SLoc, ReportOptions Opts) {
-  // If source location is already acquired, we don't need to print an error
-  // report for the second time. However, if we're in an unrecoverable handler,
-  // it's possible that location was required by concurrently running thread.
-  // In this case, we should continue the execution to ensure that any of
-  // threads will grab the report mutex and print the report before
-  // crashing the program.
-  return SLoc.isDisabled() && !Opts.DieAfterReport;
+namespace __ubsan {
+bool ignoreReport(SourceLocation SLoc, ReportOptions Opts) {
+  // We are not allowed to skip error report: if we are in unrecoverable
+  // handler, we have to terminate the program right now, and therefore
+  // have to print some diagnostic.
+  //
+  // Even if source location is disabled, it doesn't mean that we have
+  // 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)
diff --git a/lib/ubsan/ubsan_handlers_cxx.cc b/lib/ubsan/ubsan_handlers_cxx.cc
index 674e406c5..7e090827d 100644
--- a/lib/ubsan/ubsan_handlers_cxx.cc
+++ b/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);
-  HandleDynamicTypeCacheMiss(Data, Pointer, Hash, Opts);
+  // Note: -fsanitize=vptr is always recoverable.
+  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