llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-compiler-rt-sanitizer Author: Fangrui Song (MaskRay) <details> <summary>Changes</summary> The initial check-in of compiler-rt/lib/nsan #<!-- -->94322 has a lot of style issues. Fix them before the history becomes more useful. --- Patch is 54.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96142.diff 8 Files Affected: - (modified) compiler-rt/lib/nsan/CMakeLists.txt (+5-5) - (renamed) compiler-rt/lib/nsan/nsan.cpp (+133-141) - (modified) compiler-rt/lib/nsan/nsan.h (+3-3) - (renamed) compiler-rt/lib/nsan/nsan_flags.cpp (+6-7) - (removed) compiler-rt/lib/nsan/nsan_interceptors.cc (-364) - (added) compiler-rt/lib/nsan/nsan_interceptors.cpp (+362) - (renamed) compiler-rt/lib/nsan/nsan_stats.cpp (+1-1) - (renamed) compiler-rt/lib/nsan/nsan_suppressions.cpp (+26-26) ``````````diff diff --git a/compiler-rt/lib/nsan/CMakeLists.txt b/compiler-rt/lib/nsan/CMakeLists.txt index ae94c96d60727..36c7b2b9dfada 100644 --- a/compiler-rt/lib/nsan/CMakeLists.txt +++ b/compiler-rt/lib/nsan/CMakeLists.txt @@ -3,11 +3,11 @@ add_compiler_rt_component(nsan) include_directories(..) set(NSAN_SOURCES - nsan.cc - nsan_flags.cc - nsan_interceptors.cc - nsan_stats.cc - nsan_suppressions.cc + nsan.cpp + nsan_flags.cpp + nsan_interceptors.cpp + nsan_stats.cpp + nsan_suppressions.cpp ) set(NSAN_HEADERS diff --git a/compiler-rt/lib/nsan/nsan.cc b/compiler-rt/lib/nsan/nsan.cpp similarity index 78% rename from compiler-rt/lib/nsan/nsan.cc rename to compiler-rt/lib/nsan/nsan.cpp index d0d29ddfba0e3..ece1130f73d14 100644 --- a/compiler-rt/lib/nsan/nsan.cc +++ b/compiler-rt/lib/nsan/nsan.cpp @@ -82,22 +82,22 @@ const char FTInfo<float>::kTypePattern[sizeof(float)]; const char FTInfo<double>::kTypePattern[sizeof(double)]; const char FTInfo<long double>::kTypePattern[sizeof(long double)]; -// Helper for __nsan_dump_shadow_mem: Reads the value at address `Ptr`, +// Helper for __nsan_dump_shadow_mem: Reads the value at address `ptr`, // identified by its type id. -template <typename ShadowFT> __float128 readShadowInternal(const u8 *Ptr) { +template <typename ShadowFT> __float128 readShadowInternal(const u8 *ptr) { ShadowFT Shadow; - __builtin_memcpy(&Shadow, Ptr, sizeof(Shadow)); + __builtin_memcpy(&Shadow, ptr, sizeof(Shadow)); return Shadow; } -__float128 readShadow(const u8 *Ptr, const char ShadowTypeId) { +__float128 readShadow(const u8 *ptr, const char ShadowTypeId) { switch (ShadowTypeId) { case 'd': - return readShadowInternal<double>(Ptr); + return readShadowInternal<double>(ptr); case 'l': - return readShadowInternal<long double>(Ptr); + return readShadowInternal<long double>(ptr); case 'q': - return readShadowInternal<__float128>(Ptr); + return readShadowInternal<__float128>(ptr); default: return 0.0; } @@ -120,30 +120,30 @@ struct PrintBuffer { template <typename FT> struct FTPrinter {}; template <> struct FTPrinter<double> { - static PrintBuffer dec(double Value) { - PrintBuffer Result; - snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20f", Value); - return Result; + static PrintBuffer dec(double value) { + PrintBuffer result; + snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20f", value); + return result; } - static PrintBuffer hex(double Value) { - PrintBuffer Result; - snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20a", Value); - return Result; + static PrintBuffer hex(double value) { + PrintBuffer result; + snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20a", value); + return result; } }; template <> struct FTPrinter<float> : FTPrinter<double> {}; template <> struct FTPrinter<long double> { - static PrintBuffer dec(long double Value) { - PrintBuffer Result; - snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20Lf", Value); - return Result; + static PrintBuffer dec(long double value) { + PrintBuffer result; + snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20Lf", value); + return result; } - static PrintBuffer hex(long double Value) { - PrintBuffer Result; - snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20La", Value); - return Result; + static PrintBuffer hex(long double value) { + PrintBuffer result; + snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20La", value); + return result; } }; @@ -151,15 +151,15 @@ template <> struct FTPrinter<long double> { template <> struct FTPrinter<__float128> : FTPrinter<long double> {}; // This is a template so that there are no implicit conversions. -template <typename FT> inline FT ftAbs(FT V); +template <typename FT> inline FT ftAbs(FT v); -template <> inline long double ftAbs(long double V) { return fabsl(V); } -template <> inline double ftAbs(double V) { return fabs(V); } +template <> inline long double ftAbs(long double v) { return fabsl(v); } +template <> inline double ftAbs(double v) { return fabs(v); } // We don't care about nans. // std::abs(__float128) code is suboptimal and generates a function call to // __getf2(). -template <typename FT> inline FT ftAbs(FT V) { return V >= FT{0} ? V : -V; } +template <typename FT> inline FT ftAbs(FT v) { return v >= FT{0} ? v : -v; } template <typename FT1, typename FT2, bool Enable> struct LargestFTImpl { using type = FT2; @@ -192,7 +192,7 @@ extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __nsan_print_accumulated_stats() { nsan_stats->print(); } -static void nsanAtexit() { +static void NsanAtexit() { Printf("Numerical Sanitizer exit stats:\n"); __nsan_print_accumulated_stats(); nsan_stats = nullptr; @@ -204,13 +204,13 @@ static void nsanAtexit() { // around long double being the same for nsan and the target application. // We have to have 3 versions because we need to know which type we are storing // since we are setting the type shadow memory. -template <typename FT> static u8 *getShadowPtrForStore(u8 *StoreAddr, uptr N) { - unsigned char *ShadowType = getShadowTypeAddrFor(StoreAddr); - for (uptr I = 0; I < N; ++I) { - __builtin_memcpy(ShadowType + I * sizeof(FT), FTInfo<FT>::kTypePattern, +template <typename FT> static u8 *getShadowPtrForStore(u8 *store_addr, uptr n) { + unsigned char *shadow_type = getShadowTypeAddrFor(store_addr); + for (uptr i = 0; i < n; ++i) { + __builtin_memcpy(shadow_type + i * sizeof(FT), FTInfo<FT>::kTypePattern, sizeof(FTInfo<FT>::kTypePattern)); } - return getShadowAddrFor(StoreAddr); + return getShadowAddrFor(store_addr); } extern "C" SANITIZER_INTERFACE_ATTRIBUTE u8 * @@ -228,36 +228,36 @@ __nsan_get_shadow_ptr_for_longdouble_store(u8 *store_addr, uptr n) { return getShadowPtrForStore<long double>(store_addr, n); } -template <typename FT> static bool isValidShadowType(const u8 *ShadowType) { - return __builtin_memcmp(ShadowType, FTInfo<FT>::kTypePattern, sizeof(FT)) == +template <typename FT> static bool isValidShadowType(const u8 *shadow_type) { + return __builtin_memcmp(shadow_type, FTInfo<FT>::kTypePattern, sizeof(FT)) == 0; } -template <int kSize, typename T> static bool isZero(const T *Ptr) { +template <int kSize, typename T> static bool isZero(const T *ptr) { constexpr const char kZeros[kSize] = {}; // Zero initialized. - return __builtin_memcmp(Ptr, kZeros, kSize) == 0; + return __builtin_memcmp(ptr, kZeros, kSize) == 0; } -template <typename FT> static bool isUnknownShadowType(const u8 *ShadowType) { - return isZero<sizeof(FTInfo<FT>::kTypePattern)>(ShadowType); +template <typename FT> static bool isUnknownShadowType(const u8 *shadow_type) { + return isZero<sizeof(FTInfo<FT>::kTypePattern)>(shadow_type); } // The three folowing functions check that the address stores a complete // shadow value of the given type and return a pointer for loading. // They return nullptr if the type of the value is unknown or incomplete. template <typename FT> -static const u8 *getShadowPtrForLoad(const u8 *LoadAddr, uptr N) { - const u8 *const ShadowType = getShadowTypeAddrFor(LoadAddr); - for (uptr I = 0; I < N; ++I) { - if (!isValidShadowType<FT>(ShadowType + I * sizeof(FT))) { +static const u8 *getShadowPtrForLoad(const u8 *load_addr, uptr n) { + const u8 *const shadow_type = getShadowTypeAddrFor(load_addr); + for (uptr i = 0; i < n; ++i) { + if (!isValidShadowType<FT>(shadow_type + i * sizeof(FT))) { // If loadtracking stats are enabled, log loads with invalid types // (tampered with through type punning). if (flags().enable_loadtracking_stats) { - if (isUnknownShadowType<FT>(ShadowType + I * sizeof(FT))) { + if (isUnknownShadowType<FT>(shadow_type + i * sizeof(FT))) { // Warn only if the value is non-zero. Zero is special because // applications typically initialize large buffers to zero in an // untyped way. - if (!isZero<sizeof(FT)>(LoadAddr)) { + if (!isZero<sizeof(FT)>(load_addr)) { GET_CALLER_PC_BP; nsan_stats->addUnknownLoadTrackingEvent(pc, bp); } @@ -269,7 +269,7 @@ static const u8 *getShadowPtrForLoad(const u8 *LoadAddr, uptr N) { return nullptr; } } - return getShadowAddrFor(LoadAddr); + return getShadowAddrFor(load_addr); } extern "C" SANITIZER_INTERFACE_ATTRIBUTE const u8 * @@ -308,14 +308,14 @@ static int getValuePos(u8 c) { return c >> kValueSizeSizeBits; } // Checks the consistency of the value types at the given type pointer. // If the value is inconsistent, returns ValueType::kUnknown. Else, return the // consistent type. -template <typename FT> static bool checkValueConsistency(const u8 *ShadowType) { - const int Pos = getValuePos(*ShadowType); +template <typename FT> +static bool checkValueConsistency(const u8 *shadow_type) { + const int pos = getValuePos(*shadow_type); // Check that all bytes from the start of the value are ordered. - for (uptr I = 0; I < sizeof(FT); ++I) { - const u8 T = *(ShadowType - Pos + I); - if (!(getValueType(T) == FTInfo<FT>::kValueType && getValuePos(T) == I)) { + for (uptr i = 0; i < sizeof(FT); ++i) { + const u8 T = *(shadow_type - pos + i); + if (!(getValueType(T) == FTInfo<FT>::kValueType && getValuePos(T) == i)) return false; - } } return true; } @@ -325,15 +325,14 @@ template <typename FT> static bool checkValueConsistency(const u8 *ShadowType) { extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __nsan_dump_shadow_mem(const u8 *addr, size_t size_bytes, size_t bytes_per_line, size_t shadow_value_type_ids) { - const u8 *const ShadowType = getShadowTypeAddrFor(addr); - const u8 *const Shadow = getShadowAddrFor(addr); + const u8 *const shadow_type = getShadowTypeAddrFor(addr); + const u8 *const shadow = getShadowAddrFor(addr); constexpr int kMaxNumDecodedValues = 16; - __float128 DecodedValues[kMaxNumDecodedValues]; - int NumDecodedValues = 0; - if (bytes_per_line > 4 * kMaxNumDecodedValues) { + __float128 decoded_values[kMaxNumDecodedValues]; + int num_decoded_values = 0; + if (bytes_per_line > 4 * kMaxNumDecodedValues) bytes_per_line = 4 * kMaxNumDecodedValues; - } // We keep track of the current type and position as we go. ValueType LastValueTy = kUnknownValueType; @@ -343,8 +342,8 @@ __nsan_dump_shadow_mem(const u8 *addr, size_t size_bytes, size_t bytes_per_line, ++R) { printf("%p: ", (void *)(addr + R * bytes_per_line)); for (size_t C = 0; C < bytes_per_line && Offset < size_bytes; ++C) { - const ValueType ValueTy = getValueType(ShadowType[Offset]); - const int pos = getValuePos(ShadowType[Offset]); + const ValueType ValueTy = getValueType(shadow_type[Offset]); + const int pos = getValuePos(shadow_type[Offset]); if (ValueTy == LastValueTy && pos == LastPos + 1) { ++LastPos; } else { @@ -359,64 +358,59 @@ __nsan_dump_shadow_mem(const u8 *addr, size_t size_bytes, size_t bytes_per_line, case kFloatValueType: printf("f%x ", pos); if (LastPos == sizeof(float) - 1) { - DecodedValues[NumDecodedValues] = - readShadow(Shadow + kShadowScale * (Offset + 1 - sizeof(float)), + decoded_values[num_decoded_values] = + readShadow(shadow + kShadowScale * (Offset + 1 - sizeof(float)), static_cast<char>(shadow_value_type_ids & 0xff)); - ++NumDecodedValues; + ++num_decoded_values; } break; case kDoubleValueType: printf("d%x ", pos); if (LastPos == sizeof(double) - 1) { - DecodedValues[NumDecodedValues] = readShadow( - Shadow + kShadowScale * (Offset + 1 - sizeof(double)), + decoded_values[num_decoded_values] = readShadow( + shadow + kShadowScale * (Offset + 1 - sizeof(double)), static_cast<char>((shadow_value_type_ids >> 8) & 0xff)); - ++NumDecodedValues; + ++num_decoded_values; } break; case kFp80ValueType: printf("l%x ", pos); if (LastPos == sizeof(long double) - 1) { - DecodedValues[NumDecodedValues] = readShadow( - Shadow + kShadowScale * (Offset + 1 - sizeof(long double)), + decoded_values[num_decoded_values] = readShadow( + shadow + kShadowScale * (Offset + 1 - sizeof(long double)), static_cast<char>((shadow_value_type_ids >> 16) & 0xff)); - ++NumDecodedValues; + ++num_decoded_values; } break; } ++Offset; } - for (int I = 0; I < NumDecodedValues; ++I) { - printf(" (%s)", FTPrinter<__float128>::dec(DecodedValues[I]).Buffer); + for (int i = 0; i < num_decoded_values; ++i) { + printf(" (%s)", FTPrinter<__float128>::dec(decoded_values[i]).Buffer); } - NumDecodedValues = 0; + num_decoded_values = 0; printf("\n"); } } SANITIZER_INTERFACE_ATTRIBUTE -ALIGNED(16) -THREADLOCAL -uptr __nsan_shadow_ret_tag = 0; +alignas(16) thread_local uptr __nsan_shadow_ret_tag = 0; SANITIZER_INTERFACE_ATTRIBUTE -ALIGNED(16) -THREADLOCAL -char __nsan_shadow_ret_ptr[kMaxVectorWidth * sizeof(__float128)]; +alignas(16) thread_local char __nsan_shadow_ret_ptr[kMaxVectorWidth * + sizeof(__float128)]; SANITIZER_INTERFACE_ATTRIBUTE -ALIGNED(16) -THREADLOCAL -uptr __nsan_shadow_args_tag = 0; +alignas(16) thread_local uptr __nsan_shadow_args_tag = 0; // Maximum number of args. This should be enough for anyone (tm). An alternate // scheme is to have the generated code create an alloca and make // __nsan_shadow_args_ptr point ot the alloca. constexpr const int kMaxNumArgs = 128; SANITIZER_INTERFACE_ATTRIBUTE -ALIGNED(16) -THREADLOCAL -char __nsan_shadow_args_ptr[kMaxVectorWidth * kMaxNumArgs * sizeof(__float128)]; +alignas( + 16) thread_local char __nsan_shadow_args_ptr[kMaxVectorWidth * kMaxNumArgs * + sizeof(__float128)]; enum ContinuationType { // Keep in sync with instrumentation pass. kContinueWithShadow = 0, @@ -428,36 +422,36 @@ enum ContinuationType { // Keep in sync with instrumentation pass. // rather than the shadow value. This prevents one error to propagate to all // subsequent operations. This behaviour is tunable with flags. template <typename FT, typename ShadowFT> -int32_t checkFT(const FT Value, ShadowFT Shadow, CheckTypeT CheckType, +int32_t checkFT(const FT value, ShadowFT Shadow, CheckTypeT CheckType, uptr CheckArg) { // We do all comparisons in the InternalFT domain, which is the largest FT // type. using InternalFT = LargestFT<FT, ShadowFT>; - const InternalFT CheckValue = Value; - const InternalFT CheckShadow = Shadow; + const InternalFT check_value = value; + const InternalFT check_shadow = Shadow; // See this article for an interesting discussion of how to compare floats: // https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ static constexpr const FT Eps = FTInfo<FT>::kEpsilon; - const InternalFT AbsErr = ftAbs(CheckValue - CheckShadow); + const InternalFT abs_err = ftAbs(check_value - check_shadow); if (flags().enable_check_stats) { GET_CALLER_PC_BP; - // We are re-computing `Largest` here because this is a cold branch, and we - // want to avoid having to move the computation of `Largest` before the + // We are re-computing `largest` here because this is a cold branch, and we + // want to avoid having to move the computation of `largest` before the // absolute value check when this branch is not taken. - const InternalFT Largest = max(ftAbs(CheckValue), ftAbs(CheckShadow)); - nsan_stats->addCheck(CheckType, pc, bp, AbsErr / Largest); + const InternalFT largest = max(ftAbs(check_value), ftAbs(check_shadow)); + nsan_stats->addCheck(CheckType, pc, bp, abs_err / largest); } - // Note: writing the comparison that way ensures that when `AbsErr` is Nan + // Note: writing the comparison that way ensures that when `abs_err` is Nan // (value and shadow are inf or -inf), we pass the test. - if (!(AbsErr >= flags().cached_absolute_error_threshold)) + if (!(abs_err >= flags().cached_absolute_error_threshold)) return kContinueWithShadow; - const InternalFT Largest = max(ftAbs(CheckValue), ftAbs(CheckShadow)); - if (AbsErr * (1ull << flags().log2_max_relative_error) <= Largest) + const InternalFT largest = max(ftAbs(check_value), ftAbs(check_shadow)); + if (abs_err * (1ull << flags().log2_max_relative_error) <= largest) return kContinueWithShadow; // No problem here. if (!flags().disable_warnings) { @@ -474,13 +468,13 @@ int32_t checkFT(const FT Value, ShadowFT Shadow, CheckTypeT CheckType, Printf("%s", D.Warning()); // Printf does not support float formatting. char RelErrBuf[64] = "inf"; - if (Largest > Eps) { + if (largest > Eps) { snprintf(RelErrBuf, sizeof(RelErrBuf) - 1, "%.20Lf%% (2^%.0Lf epsilons)", - static_cast<long double>(100.0 * AbsErr / Largest), - log2l(static_cast<long double>(AbsErr / Largest / Eps))); + static_cast<long double>(100.0 * abs_err / largest), + log2l(static_cast<long double>(abs_err / largest / Eps))); } char UlpErrBuf[128] = ""; - const double ShadowUlpDiff = getULPDiff(CheckValue, CheckShadow); + const double ShadowUlpDiff = getULPDiff(check_value, check_shadow); if (ShadowUlpDiff != kMaxULPDiff) { // This is the ULP diff in the internal domain. The user actually cares // about that in the original domain. @@ -529,18 +523,18 @@ int32_t checkFT(const FT Value, ShadowFT Shadow, CheckTypeT CheckType, "Relative error: %s\n" "Absolute error: %s\n" "%s\n", - FTInfo<FT>::kCppTypeName, ValuePrinter::dec(Value).Buffer, - ValuePrinter::hex(Value).Buffer, FTInfo<ShadowFT>::kCppTypeName, + FTInfo<FT>::kCppTypeName, ValuePrinter::dec(value).Buffer, + ValuePrinter::hex(value).Buffer, FTInfo<ShadowFT>::kCppTypeName, ShadowPrinter::dec(Shadow).Buffer, ShadowPrinter::hex(Shadow).Buffer, FTInfo<FT>::kCppTypeName, ValuePrinter::dec(Shadow).Buffer, ValuePrinter::hex(Shadow).Buffer, RelErrBuf, - ValuePrinter::hex(AbsErr).Buffer, UlpErrBuf); + ValuePrinter::hex(abs_err).Buffer, UlpErrBuf); stack.Print(); } if (flags().enable_warning_stats) { GET_CALLER_PC_BP; - nsan_stats->addWarning(CheckType, pc, bp, AbsErr / Largest); + nsan_stats->addWarning(CheckType, pc, bp, abs_err / largest); } if (flags().halt_on_error) { @@ -614,9 +608,9 @@ static const char *getPredicateName(int v) { template <typename FT, typename ShadowFT> void fCmpFailFT(const FT Lhs, const FT Rhs, ShadowFT LhsShadow, - ShadowFT RhsShadow, int Predicate, bool Result, + ShadowFT RhsShadow, int Predicate, bool result, bool ShadowResult) { - if (Result == ShadowResult) { + if (result == ShadowResult) { // When a vector comparison fails, we fail each element of the comparison // to simplify instrumented code. Skip elements where the shadow comparison // gave the same result as the original one. @@ -657,14 +651,14 @@ void fCmpFailFT(const FT Lhs, const FT Rhs, ShadowFT LhsShadow, "%s", // Native, decimal. FTInfo<FT>::kCppTypeName, ValuePrinter::dec(Lhs).Buffer, PredicateName, - ValuePrinter::dec(Rhs).Buffer, getTruthValueName(Result), + ValuePrinter::dec(Rhs).Buffer, getTruthValueName(result), // Shadow, decimal FTInfo<ShadowFT>::kCppTypeName, ShadowPrinter::dec(LhsShadow).Buffer, PredicateName, ShadowPrinter::dec(RhsShadow).Buffer, getTruthValueName(ShadowResult), // Native, hex. FTInfo<FT>::kCppTypeName, ValuePrinter::hex(Lhs).Buffer, PredicateName, - ValuePrinter::hex(Rhs).Buffer, getTruthValueName(Result), + ValuePrinter::hex(Rhs).Buffer, getTruthValueName(result), // Shadow... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/96142 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits