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

Alexey Samsonov via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 8 16:12:58 PST 2015


Author: samsonov
Date: Tue Dec  8 18:12:57 2015
New Revision: 255081

URL: http://llvm.org/viewvc/llvm-project?rev=255081&view=rev
Log:
[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.

Modified:
    compiler-rt/trunk/lib/ubsan/ubsan_diag.cc
    compiler-rt/trunk/lib/ubsan/ubsan_diag.h
    compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
    compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc

Modified: compiler-rt/trunk/lib/ubsan/ubsan_diag.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_diag.cc?rev=255081&r1=255080&r2=255081&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_diag.cc (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_diag.cc Tue Dec  8 18:12:57 2015
@@ -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();
 }
 

Modified: compiler-rt/trunk/lib/ubsan/ubsan_diag.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_diag.h?rev=255081&r1=255080&r2=255081&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_diag.h (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_diag.h Tue Dec  8 18:12:57 2015
@@ -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

Modified: compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc?rev=255081&r1=255080&r2=255081&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc Tue Dec  8 18:12:57 2015
@@ -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(Ov
     << 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)

Modified: compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc?rev=255081&r1=255080&r2=255081&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc Tue Dec  8 18:12:57 2015
@@ -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_typ
 }
 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_typ
                                                 ValueHandle Vtable) {
   GET_REPORT_OPTIONS(true);
   HandleCFIBadType(Data, Vtable, Opts);
+  Die();
 }
 
 #endif  // CAN_SANITIZE_UB




More information about the llvm-commits mailing list