From 96591cd1f17b998c652ee05860e03134161f3de8 Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Wed, 30 Jul 2014 01:49:19 +0000 Subject: [PATCH] [UBSan] Introduce ScopedReport object. This object is used to encapsulate all actions that need to be done before/after printing UBSan diagnostics. Currently these actions are: * locking a mutex to ensure that UBSan diagnostics from several threads won't mix with each other and with other sanitizers' reports * killing a program once the report is printed (if necessary). Use this object in all UBSan handlers. Unify the way we implement fatal and non-fatal handlers by making all the handlers simple one-liners that redirect __ubsan_handle_foo(_abort)? to handleFooImpl(). llvm-svn: 214279 --- compiler-rt/lib/ubsan/ubsan_diag.cc | 16 ++- compiler-rt/lib/ubsan/ubsan_diag.h | 12 ++ compiler-rt/lib/ubsan/ubsan_handlers.cc | 201 ++++++++++++++++------------ compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc | 4 +- 4 files changed, 143 insertions(+), 90 deletions(-) diff --git a/compiler-rt/lib/ubsan/ubsan_diag.cc b/compiler-rt/lib/ubsan/ubsan_diag.cc index a8f2a49..2b2ca29 100644 --- a/compiler-rt/lib/ubsan/ubsan_diag.cc +++ b/compiler-rt/lib/ubsan/ubsan_diag.cc @@ -274,9 +274,9 @@ static void renderMemorySnippet(const Decorator &Decor, MemoryLocation Loc, } Diag::~Diag() { - InitIfNecessary(); + // All diagnostics should be printed under report mutex. + CommonSanitizerReportMutex.CheckLocked(); Decorator Decor; - SpinMutexLock l(&CommonSanitizerReportMutex); Printf(Decor.Bold()); renderLocation(Loc); @@ -300,3 +300,15 @@ Diag::~Diag() { renderMemorySnippet(Decor, Loc.getMemoryLocation(), Ranges, NumRanges, Args); } + +ScopedReport::ScopedReport(bool DieAfterReport) + : DieAfterReport(DieAfterReport) { + InitIfNecessary(); + CommonSanitizerReportMutex.Lock(); +} + +ScopedReport::~ScopedReport() { + CommonSanitizerReportMutex.Unlock(); + if (DieAfterReport) + Die(); +} diff --git a/compiler-rt/lib/ubsan/ubsan_diag.h b/compiler-rt/lib/ubsan/ubsan_diag.h index 71c1344..4c8a3ea 100644 --- a/compiler-rt/lib/ubsan/ubsan_diag.h +++ b/compiler-rt/lib/ubsan/ubsan_diag.h @@ -206,6 +206,18 @@ public: void MaybePrintStackTrace(uptr pc, uptr bp); +/// \brief Instantiate this class before printing diagnostics in the error +/// report. This class ensures that reports from different threads and from +/// different sanitizers won't be mixed. If DieAfterReport is specified, it +/// will terminate the program in the destructor. +class ScopedReport { + bool DieAfterReport; + +public: + ScopedReport(bool DieAfterReport); + ~ScopedReport(); +}; + } // namespace __ubsan #endif // UBSAN_DIAG_H diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cc b/compiler-rt/lib/ubsan/ubsan_handlers.cc index d556431..afda393 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cc +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cc @@ -27,12 +27,14 @@ namespace __ubsan { } static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, - Location FallbackLoc) { + Location FallbackLoc, bool Abort) { Location Loc = Data->Loc.acquire(); - // Use the SourceLocation from Data to track deduplication, even if 'invalid' if (Loc.getSourceLocation().isDisabled()) return; + + ScopedReport R(Abort); + if (Data->Loc.isInvalid()) Loc = FallbackLoc; @@ -51,70 +53,53 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, if (Pointer) Diag(Pointer, DL_Note, "pointer points here"); } + void __ubsan::__ubsan_handle_type_mismatch(TypeMismatchData *Data, ValueHandle Pointer) { - handleTypeMismatchImpl(Data, Pointer, getCallerLocation()); + handleTypeMismatchImpl(Data, Pointer, getCallerLocation(), false); } void __ubsan::__ubsan_handle_type_mismatch_abort(TypeMismatchData *Data, ValueHandle Pointer) { - handleTypeMismatchImpl(Data, Pointer, getCallerLocation()); - Die(); + handleTypeMismatchImpl(Data, Pointer, getCallerLocation(), true); } /// \brief Common diagnostic emission for various forms of integer overflow. -template static void HandleIntegerOverflow(OverflowData *Data, - ValueHandle LHS, - const char *Operator, - T RHS) { +template +static void handleIntegerOverflowImpl(OverflowData *Data, ValueHandle LHS, + const char *Operator, T RHS, bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Diag(Loc, DL_Error, "%0 integer overflow: " "%1 %2 %3 cannot be represented in type %4") << (Data->Type.isSignedIntegerTy() ? "signed" : "unsigned") << Value(Data->Type, LHS) << Operator << RHS << Data->Type; } -void __ubsan::__ubsan_handle_add_overflow(OverflowData *Data, - ValueHandle LHS, ValueHandle RHS) { - HandleIntegerOverflow(Data, LHS, "+", Value(Data->Type, RHS)); -} -void __ubsan::__ubsan_handle_add_overflow_abort(OverflowData *Data, - ValueHandle LHS, - ValueHandle RHS) { - __ubsan_handle_add_overflow(Data, LHS, RHS); - Die(); -} - -void __ubsan::__ubsan_handle_sub_overflow(OverflowData *Data, - ValueHandle LHS, ValueHandle RHS) { - HandleIntegerOverflow(Data, LHS, "-", Value(Data->Type, RHS)); -} -void __ubsan::__ubsan_handle_sub_overflow_abort(OverflowData *Data, - ValueHandle LHS, - ValueHandle RHS) { - __ubsan_handle_sub_overflow(Data, LHS, RHS); - Die(); -} - -void __ubsan::__ubsan_handle_mul_overflow(OverflowData *Data, - ValueHandle LHS, ValueHandle RHS) { - HandleIntegerOverflow(Data, LHS, "*", Value(Data->Type, RHS)); -} -void __ubsan::__ubsan_handle_mul_overflow_abort(OverflowData *Data, - ValueHandle LHS, - ValueHandle RHS) { - __ubsan_handle_mul_overflow(Data, LHS, RHS); - Die(); -} - -void __ubsan::__ubsan_handle_negate_overflow(OverflowData *Data, - ValueHandle OldVal) { +#define UBSAN_OVERFLOW_HANDLER(handler_name, op, abort) \ + void __ubsan::handler_name(OverflowData *Data, ValueHandle LHS, \ + ValueHandle RHS) { \ + handleIntegerOverflowImpl(Data, LHS, op, Value(Data->Type, RHS), abort); \ + } + +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_add_overflow, "+", false) +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_add_overflow_abort, "+", true) +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_sub_overflow, "-", false) +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_sub_overflow_abort, "-", true) +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_mul_overflow, "*", false) +UBSAN_OVERFLOW_HANDLER(__ubsan_handle_mul_overflow_abort, "*", true) + +static void handleNegateOverflowImpl(OverflowData *Data, ValueHandle OldVal, + bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + if (Data->Type.isSignedIntegerTy()) Diag(Loc, DL_Error, "negation of %0 cannot be represented in type %1; " @@ -125,18 +110,24 @@ void __ubsan::__ubsan_handle_negate_overflow(OverflowData *Data, "negation of %0 cannot be represented in type %1") << Value(Data->Type, OldVal) << Data->Type; } + +void __ubsan::__ubsan_handle_negate_overflow(OverflowData *Data, + ValueHandle OldVal) { + handleNegateOverflowImpl(Data, OldVal, false); +} void __ubsan::__ubsan_handle_negate_overflow_abort(OverflowData *Data, ValueHandle OldVal) { - __ubsan_handle_negate_overflow(Data, OldVal); - Die(); + handleNegateOverflowImpl(Data, OldVal, true); } -void __ubsan::__ubsan_handle_divrem_overflow(OverflowData *Data, - ValueHandle LHS, ValueHandle RHS) { +static void handleDivremOverflowImpl(OverflowData *Data, ValueHandle LHS, + ValueHandle RHS, bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Value LHSVal(Data->Type, LHS); Value RHSVal(Data->Type, RHS); if (RHSVal.isMinusOne()) @@ -146,20 +137,26 @@ void __ubsan::__ubsan_handle_divrem_overflow(OverflowData *Data, else Diag(Loc, DL_Error, "division by zero"); } + +void __ubsan::__ubsan_handle_divrem_overflow(OverflowData *Data, + ValueHandle LHS, ValueHandle RHS) { + handleDivremOverflowImpl(Data, LHS, RHS, false); +} void __ubsan::__ubsan_handle_divrem_overflow_abort(OverflowData *Data, ValueHandle LHS, ValueHandle RHS) { - __ubsan_handle_divrem_overflow(Data, LHS, RHS); - Die(); + handleDivremOverflowImpl(Data, LHS, RHS, true); } -void __ubsan::__ubsan_handle_shift_out_of_bounds(ShiftOutOfBoundsData *Data, - ValueHandle LHS, - ValueHandle RHS) { +static void handleShiftOutOfBoundsImpl(ShiftOutOfBoundsData *Data, + ValueHandle LHS, ValueHandle RHS, + bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Value LHSVal(Data->LHSType, LHS); Value RHSVal(Data->RHSType, RHS); if (RHSVal.isNegative()) @@ -175,107 +172,139 @@ void __ubsan::__ubsan_handle_shift_out_of_bounds(ShiftOutOfBoundsData *Data, "left shift of %0 by %1 places cannot be represented in type %2") << LHSVal << RHSVal << Data->LHSType; } + +void __ubsan::__ubsan_handle_shift_out_of_bounds(ShiftOutOfBoundsData *Data, + ValueHandle LHS, + ValueHandle RHS) { + handleShiftOutOfBoundsImpl(Data, LHS, RHS, false); +} void __ubsan::__ubsan_handle_shift_out_of_bounds_abort( ShiftOutOfBoundsData *Data, ValueHandle LHS, ValueHandle RHS) { - __ubsan_handle_shift_out_of_bounds(Data, LHS, RHS); - Die(); + handleShiftOutOfBoundsImpl(Data, LHS, RHS, true); } -void __ubsan::__ubsan_handle_out_of_bounds(OutOfBoundsData *Data, - ValueHandle Index) { +static void handleOutOfBoundsImpl(OutOfBoundsData *Data, ValueHandle Index, + bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Value IndexVal(Data->IndexType, Index); Diag(Loc, DL_Error, "index %0 out of bounds for type %1") << IndexVal << Data->ArrayType; } + +void __ubsan::__ubsan_handle_out_of_bounds(OutOfBoundsData *Data, + ValueHandle Index) { + handleOutOfBoundsImpl(Data, Index, false); +} void __ubsan::__ubsan_handle_out_of_bounds_abort(OutOfBoundsData *Data, ValueHandle Index) { - __ubsan_handle_out_of_bounds(Data, Index); - Die(); + handleOutOfBoundsImpl(Data, Index, true); } void __ubsan::__ubsan_handle_builtin_unreachable(UnreachableData *Data) { + ScopedReport R(true); Diag(Data->Loc, DL_Error, "execution reached a __builtin_unreachable() call"); - Die(); } void __ubsan::__ubsan_handle_missing_return(UnreachableData *Data) { + ScopedReport R(true); Diag(Data->Loc, DL_Error, "execution reached the end of a value-returning function " "without returning a value"); - Die(); } -void __ubsan::__ubsan_handle_vla_bound_not_positive(VLABoundData *Data, - ValueHandle Bound) { +static void handleVLABoundNotPositive(VLABoundData *Data, ValueHandle Bound, + bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Diag(Loc, DL_Error, "variable length array bound evaluates to " "non-positive value %0") << Value(Data->Type, Bound); } + +void __ubsan::__ubsan_handle_vla_bound_not_positive(VLABoundData *Data, + ValueHandle Bound) { + handleVLABoundNotPositive(Data, Bound, false); +} void __ubsan::__ubsan_handle_vla_bound_not_positive_abort(VLABoundData *Data, - ValueHandle Bound) { - __ubsan_handle_vla_bound_not_positive(Data, Bound); - Die(); + ValueHandle Bound) { + handleVLABoundNotPositive(Data, Bound, true); } -void __ubsan::__ubsan_handle_float_cast_overflow(FloatCastOverflowData *Data, - ValueHandle From) { +static void handleFloatCastOverflow(FloatCastOverflowData *Data, ValueHandle From, + bool Abort) { // TODO: Add deduplication once a SourceLocation is generated for this check. + ScopedReport R(Abort); + Diag(getCallerLocation(), DL_Error, "value %0 is outside the range of representable values of type %2") << Value(Data->FromType, From) << Data->FromType << Data->ToType; } -void __ubsan::__ubsan_handle_float_cast_overflow_abort( - FloatCastOverflowData *Data, - ValueHandle From) { - Diag(getCallerLocation(), DL_Error, - "value %0 is outside the range of representable values of type %2") - << Value(Data->FromType, From) << Data->FromType << Data->ToType; - Die(); + +void __ubsan::__ubsan_handle_float_cast_overflow(FloatCastOverflowData *Data, + ValueHandle From) { + handleFloatCastOverflow(Data, From, false); +} +void +__ubsan::__ubsan_handle_float_cast_overflow_abort(FloatCastOverflowData *Data, + ValueHandle From) { + handleFloatCastOverflow(Data, From, true); } -void __ubsan::__ubsan_handle_load_invalid_value(InvalidValueData *Data, - ValueHandle Val) { +static void handleLoadInvalidValue(InvalidValueData *Data, ValueHandle Val, + bool Abort) { SourceLocation Loc = Data->Loc.acquire(); if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Diag(Loc, DL_Error, "load of value %0, which is not a valid value for type %1") << Value(Data->Type, Val) << Data->Type; } + +void __ubsan::__ubsan_handle_load_invalid_value(InvalidValueData *Data, + ValueHandle Val) { + handleLoadInvalidValue(Data, Val, false); +} void __ubsan::__ubsan_handle_load_invalid_value_abort(InvalidValueData *Data, ValueHandle Val) { - __ubsan_handle_load_invalid_value(Data, Val); - Die(); + handleLoadInvalidValue(Data, Val, true); } -void __ubsan::__ubsan_handle_function_type_mismatch( - FunctionTypeMismatchData *Data, - ValueHandle Function) { +static void handleFunctionTypeMismatch(FunctionTypeMismatchData *Data, + ValueHandle Function, bool Abort) { const char *FName = "(unknown)"; Location Loc = getFunctionLocation(Function, &FName); + ScopedReport R(Abort); + Diag(Data->Loc, DL_Error, "call to function %0 through pointer to incorrect function type %1") << FName << Data->Type; Diag(Loc, DL_Note, "%0 defined here") << FName; } +void +__ubsan::__ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data, + ValueHandle Function) { + handleFunctionTypeMismatch(Data, Function, false); +} + void __ubsan::__ubsan_handle_function_type_mismatch_abort( - FunctionTypeMismatchData *Data, - ValueHandle Function) { - __ubsan_handle_function_type_mismatch(Data, Function); - Die(); + FunctionTypeMismatchData *Data, ValueHandle Function) { + handleFunctionTypeMismatch(Data, Function, true); } diff --git a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc index 7014c68..205362e 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc +++ b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc @@ -37,6 +37,8 @@ static void HandleDynamicTypeCacheMiss( if (Loc.isDisabled()) return; + ScopedReport R(Abort); + Diag(Loc, DL_Error, "%0 address %1 which does not point to an object of type %2") << TypeCheckKinds[Data->TypeCheckKind] << (void*)Pointer << Data->Type; @@ -61,8 +63,6 @@ static void HandleDynamicTypeCacheMiss( << Range(Pointer, Pointer + sizeof(uptr), "vptr for %2 base class of %1"); MaybePrintStackTrace(pc, bp); - if (Abort) - Die(); } void __ubsan::__ubsan_handle_dynamic_type_cache_miss( -- 2.7.4