[libc-commits] [libc] 155f5a6 - [libc][clang-tidy] fix namespace check for externals
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Tue Nov 30 11:45:07 PST 2021
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);
More information about the libc-commits
mailing list