Author: Michael Jones Date: 2021-11-30T11:44:24-08:00 New Revision: 155f5a6dac62a902a30f60e2717c4ba8fb828139
URL: https://github.com/llvm/llvm-project/commit/155f5a6dac62a902a30f60e2717c4ba8fb828139 DIFF: https://github.com/llvm/llvm-project/commit/155f5a6dac62a902a30f60e2717c4ba8fb828139.diff LOG: [libc][clang-tidy] fix namespace check for externals Up until now, all references to `errno` were marked with `NOLINT`, since it was technically calling an external function. This fixes the lint rules so that `errno`, as well as `malloc`, `calloc`, `realloc`, and `free` are all allowed to be called as external functions. All of the relevant `NOLINT` comments have been removed, and the documentation has been updated. Reviewed By: sivachandra, lntue, aaron.ballman Differential Revision: https://reviews.llvm.org/D113946 Added: Modified: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp index fbc6762a44e78..0636883c48193 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp @@ -10,6 +10,8 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/StringSet.h" + using namespace clang::ast_matchers; namespace clang { @@ -30,6 +32,13 @@ void CalleeNamespaceCheck::registerMatchers(MatchFinder *Finder) { declRefExpr(to(functionDecl().bind("func"))).bind("use-site"), this); } +// A list of functions that are exempted from this check. The __errno_location +// function is for setting errno, which is allowed in libc, and the other +// functions are specifically allowed to be external so that they can be +// intercepted. +static const llvm::StringSet<> IgnoredFunctions = { + "__errno_location", "malloc", "calloc", "realloc", "free", "aligned_alloc"}; + void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) { const auto *UsageSiteExpr = Result.Nodes.getNodeAs<DeclRefExpr>("use-site"); const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func"); @@ -43,6 +52,9 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) { if (NS && NS->getName() == "__llvm_libc") return; + if (IgnoredFunctions.contains(FuncDecl->getName())) + return; + diag(UsageSiteExpr->getBeginLoc(), "%0 must resolve to a function declared " "within the '__llvm_libc' namespace") << FuncDecl; diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp index b7fee5dd7e0b3..8ee968433f61c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp @@ -10,6 +10,9 @@ void libc_api_func() {} // Emulate a function from the public headers like string.h void libc_api_func() {} +// Emulate a function specifically allowed by the exception list. +void malloc() {} + namespace __llvm_libc { void Test() { // Allow calls with the fully qualified name. @@ -37,6 +40,9 @@ void Test() { badPtr(); // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace // CHECK-MESSAGES: :11:6: note: resolves to this declaration + + // Allow calling into global namespace for specific functions. + ::malloc(); } } // namespace __llvm_libc diff --git a/libc/docs/clang_tidy_checks.rst b/libc/docs/clang_tidy_checks.rst index ffdc45bdd977c..01480672c465b 100644 --- a/libc/docs/clang_tidy_checks.rst +++ b/libc/docs/clang_tidy_checks.rst @@ -67,6 +67,11 @@ a public header with non-namespaced functions like ``string.h`` is included. This check ensures any function call resolves to a function within the __llvm_libc namespace. +There are exceptions for the following functions: +``__errno_location`` so that ``errno`` can be set; +``malloc``, ``calloc``, ``realloc``, ``aligned_alloc``, and ``free`` since they +are always external and can be intercepted. + .. code-block:: c++ namespace __llvm_libc { @@ -83,4 +88,7 @@ __llvm_libc namespace. // Disallow calling into functions in the global namespace. ::strlen("!"); + // Allow calling into specific global functions (explained above) + ::malloc(10); + } // namespace __llvm_libc diff --git a/libc/src/__support/FPUtil/NearestIntegerOperations.h b/libc/src/__support/FPUtil/NearestIntegerOperations.h index 99e2ba7150377..6862488edac8a 100644 --- a/libc/src/__support/FPUtil/NearestIntegerOperations.h +++ b/libc/src/__support/FPUtil/NearestIntegerOperations.h @@ -246,7 +246,7 @@ static inline I roundedFloatToSignedInteger(F x) { FPBits<F> bits(x); auto setDomainErrorAndRaiseInvalid = []() { #if math_errhandling & MATH_ERRNO - errno = EDOM; // NOLINT + errno = EDOM; #endif #if math_errhandling & MATH_ERREXCEPT raiseExcept(FE_INVALID); diff --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h index 7c2672017c685..7673334cefabf 100644 --- a/libc/src/__support/str_to_float.h +++ b/libc/src/__support/str_to_float.h @@ -200,7 +200,7 @@ simpleDecimalConversion(const char *__restrict numStart, static_cast<int64_t>(fputil::FloatProperties<T>::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits<T>::maxExponent; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -210,7 +210,7 @@ simpleDecimalConversion(const char *__restrict numStart, fputil::FloatProperties<T>::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } @@ -253,7 +253,7 @@ simpleDecimalConversion(const char *__restrict numStart, if (exp2 >= fputil::FPBits<T>::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits<T>::maxExponent; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } @@ -289,7 +289,7 @@ simpleDecimalConversion(const char *__restrict numStart, } if (exp2 == 0) { - errno = ERANGE; // NOLINT + errno = ERANGE; } *outputMantissa = finalMantissa; @@ -391,7 +391,7 @@ decimalExpToFloat(typename fputil::FPBits<T>::UIntType mantissa, int32_t exp10, static_cast<int64_t>(fputil::FloatProperties<T>::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits<T>::maxExponent; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -402,7 +402,7 @@ decimalExpToFloat(typename fputil::FPBits<T>::UIntType mantissa, int32_t exp10, 2) { *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } @@ -467,7 +467,7 @@ binaryExpToFloat(typename fputil::FPBits<T>::UIntType mantissa, int32_t exp2, // This indicates an overflow, so we make the result INF and set errno. *outputExp2 = (1 << fputil::FloatProperties<T>::exponentWidth) - 1; *outputMantissa = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } @@ -483,7 +483,7 @@ binaryExpToFloat(typename fputil::FPBits<T>::UIntType mantissa, int32_t exp2, // Return 0 if the exponent is too small. *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } } @@ -511,12 +511,12 @@ binaryExpToFloat(typename fputil::FPBits<T>::UIntType mantissa, int32_t exp2, ++biasedExponent; if (biasedExponent == INF_EXP) { - errno = ERANGE; // NOLINT + errno = ERANGE; } } if (biasedExponent == 0) { - errno = ERANGE; // NOLINT + errno = ERANGE; } *outputMantissa = mantissa & fputil::FloatProperties<T>::mantissaMask; diff --git a/libc/src/__support/str_to_integer.h b/libc/src/__support/str_to_integer.h index ec7f6f54a88f1..3ecf098e1c204 100644 --- a/libc/src/__support/str_to_integer.h +++ b/libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ static inline T strtointeger(const char *__restrict src, const char *original_src = src; if (base < 0 || base == 1 || base > 36) { - errno = EINVAL; // NOLINT + errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ static inline T strtointeger(const char *__restrict src, // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } diff --git a/libc/src/math/generic/math_utils.h b/libc/src/math/generic/math_utils.h index ef86e88a361a0..11c2bd1f0a4ae 100644 --- a/libc/src/math/generic/math_utils.h +++ b/libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template <> struct XFlowValues<double> { template <typename T> static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) - errno = err; // NOLINT + errno = err; return x; } diff --git a/libc/src/string/strdup.cpp b/libc/src/string/strdup.cpp index ddda3a63b1cb6..ee324b0736634 100644 --- a/libc/src/string/strdup.cpp +++ b/libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ LLVM_LIBC_FUNCTION(char *, strdup, (const char *src)) { return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast<char *>(::malloc(len)); // NOLINT + char *dest = reinterpret_cast<char *>(::malloc(len)); if (dest == nullptr) { return nullptr; } diff --git a/libc/src/string/strndup.cpp b/libc/src/string/strndup.cpp index d8e0818284722..4856a484b851b 100644 --- a/libc/src/string/strndup.cpp +++ b/libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ LLVM_LIBC_FUNCTION(char *, strndup, (const char *src, size_t size)) { size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast<char *>(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast<char *>(::malloc(len + 1)); if (dest == nullptr) return nullptr; inline_memcpy(dest, src, len + 1); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
