michaelrj updated this revision to Diff 387713.
michaelrj added a comment.

add test for the new lint behavior:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113946/new/

https://reviews.llvm.org/D113946

Files:
  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

Index: libc/src/string/strndup.cpp
===================================================================
--- libc/src/string/strndup.cpp
+++ libc/src/string/strndup.cpp
@@ -23,7 +23,7 @@
   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;
   char *result =
Index: libc/src/string/strdup.cpp
===================================================================
--- libc/src/string/strdup.cpp
+++ libc/src/string/strdup.cpp
@@ -21,7 +21,7 @@
     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;
   }
Index: libc/src/math/generic/math_utils.h
===================================================================
--- libc/src/math/generic/math_utils.h
+++ libc/src/math/generic/math_utils.h
@@ -55,7 +55,7 @@
 
 template <typename T> static inline T with_errno(T x, int err) {
   if (math_errhandling & MATH_ERRNO)
-    errno = err; // NOLINT
+    errno = err;
   return x;
 }
 
Index: libc/src/__support/str_to_integer.h
===================================================================
--- libc/src/__support/str_to_integer.h
+++ libc/src/__support/str_to_integer.h
@@ -74,7 +74,7 @@
   const char *original_src = src;
 
   if (base < 0 || base == 1 || base > 36) {
-    errno = EINVAL; // NOLINT
+    errno = EINVAL;
     return 0;
   }
 
@@ -114,19 +114,19 @@
     // 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;
     }
Index: libc/src/__support/str_to_float.h
===================================================================
--- libc/src/__support/str_to_float.h
+++ libc/src/__support/str_to_float.h
@@ -220,7 +220,7 @@
           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.
@@ -230,7 +230,7 @@
                                fputil::FloatProperties<T>::mantissaWidth)) {
     *outputMantissa = 0;
     *outputExp2 = 0;
-    errno = ERANGE; // NOLINT
+    errno = ERANGE;
     return;
   }
 
@@ -273,7 +273,7 @@
   if (exp2 >= fputil::FPBits<T>::maxExponent) {
     *outputMantissa = 0;
     *outputExp2 = fputil::FPBits<T>::maxExponent;
-    errno = ERANGE; // NOLINT
+    errno = ERANGE;
     return;
   }
 
@@ -309,7 +309,7 @@
   }
 
   if (exp2 == 0) {
-    errno = ERANGE; // NOLINT
+    errno = ERANGE;
   }
 
   *outputMantissa = finalMantissa;
@@ -411,7 +411,7 @@
       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.
@@ -422,7 +422,7 @@
               2) {
     *outputMantissa = 0;
     *outputExp2 = 0;
-    errno = ERANGE; // NOLINT
+    errno = ERANGE;
     return;
   }
 
@@ -508,7 +508,7 @@
     // If we cut off any bits to fit this number into a subnormal, then it's
     // out of range for this size of float.
     if ((mantissa & ((1 << amountToShift) - 1)) > 0) {
-      errno = ERANGE; // NOLINT
+      errno = ERANGE;
     }
     mantissa = shiftRightAndRound(mantissa, amountToShift);
     if (mantissa == OVERFLOWED_MANTISSA) {
@@ -524,7 +524,7 @@
     // This indicates an overflow, so we make the result INF and set errno.
     biasedExponent = (1 << fputil::FloatProperties<T>::exponentWidth) - 1;
     mantissa = 0;
-    errno = ERANGE; // NOLINT
+    errno = ERANGE;
   }
   *outputMantissa = mantissa;
   *outputExp2 = biasedExponent;
Index: libc/src/__support/FPUtil/NearestIntegerOperations.h
===================================================================
--- libc/src/__support/FPUtil/NearestIntegerOperations.h
+++ libc/src/__support/FPUtil/NearestIntegerOperations.h
@@ -246,7 +246,7 @@
   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);
Index: libc/docs/clang_tidy_checks.rst
===================================================================
--- libc/docs/clang_tidy_checks.rst
+++ libc/docs/clang_tidy_checks.rst
@@ -67,6 +67,11 @@
 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``, and ``free`` since they are always external
+and can be intercepted.
+
 .. code-block:: c++
 
     namespace __llvm_libc {
@@ -83,4 +88,7 @@
     // Disallow calling into functions in the global namespace.
     ::strlen("!");
 
+    // Allow calling into specific global functions (explained above)
+    ::malloc(10);
+
     } // namespace __llvm_libc
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -10,6 +10,9 @@
 // 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 @@
   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
Index: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -10,6 +10,8 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
+#include <unordered_set>
+
 using namespace clang::ast_matchers;
 
 namespace clang {
@@ -30,6 +32,13 @@
       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 std::unordered_set<std::string> FUNCTIONS_TO_IGNORE_NAMESPACE = {
+    "__errno_location", "malloc", "calloc", "realloc", "free"};
+
 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,11 @@
   if (NS && NS->getName() == "__llvm_libc")
     return;
 
+  if (FUNCTIONS_TO_IGNORE_NAMESPACE.find(FuncDecl->getName().str()) !=
+      FUNCTIONS_TO_IGNORE_NAMESPACE.end()) {
+    return;
+  }
+
   diag(UsageSiteExpr->getBeginLoc(), "%0 must resolve to a function declared "
                                      "within the '__llvm_libc' namespace")
       << FuncDecl;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to