[clang-tidy] Add check callee-namespace.

[PaulkaToast]

Summary

This check will ensure that all calls to functions resolve to one inside the __llvm_libc namespace.

This is done to ensure that if we include a public header then we don't accidentally call into the a function within the global namespace.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

119

Please enclose __llvm_libc in double back-ticks.

clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst

6

Please enclose __llvm_libc in double back-ticks.

[aaron.ballman]

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

23–24

Under what circumstances could the translation unit be passed in as the declaration? I think this code can just be removed.

31–32

Do you also want to catch binding of function pointers? e.g.,

float (*fp)(float) = sinf;

44–47

const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
if (NS && NS->getName() == "__llvm_libc")
  return;

49–50

How about: %0 must resolve to a function declared within the '__llvm_libc' namespace

53–54

This diagnostic seems a bit strange -- I would expect some text after the colon.

[PaulkaToast]

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

23–24

Actually you are totally right. This case would never occur, thanks (:

31–32

I hadn't considered function pointers, does seem like a good thing to catch though. Modified the matches to catch this case as well. Thank you!

53–54

I was trying mimic the clang's previous definition diagnostic. e.g. : https://godbolt.org/z/V4tKr-
Although the colon does seem to confusing so I removed it.

[aaron.ballman]

LGTM aside from some minor nits.

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

23–25

Maybe const DeclContext *Parent = Decl->getParent(); and then use Parent in this function? Not critical, but it would simplify things a bit.

53–54

Ah, thank you for the explanation. I think I would word it resolves to this declaration (or something along those lines) to be a bit less grammatically ambiguous. When the diagnostic ends in "to", I assume there's a part of the diagnostic missing and I wonder "to what?"

[PaulkaToast]

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

53–54

I agree with this, thanks for the insight (:

clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyLLVMLibcModule
+  CalleeNamespaceCheck.cpp
   ImplementationInNamespaceCheck.cpp
   LLVMLibcTidyModule.cpp
   RestrictSystemLibcHeadersCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyPortabilityModule
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h

+//===-- CalleeNamespaceCheck.h ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_CALLEENAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_CALLEENAMESPACECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm_libc {
+
+/// Checks all llvm-libc implementation is within the correct namespace.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc-callee-namespace.html
+class CalleeNamespaceCheck : public ClangTidyCheck {
+public:
+  CalleeNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace llvm_libc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_CALLEENAMESPACECHECK_H

clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

+//===-- CalleeNamespaceCheck.cpp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CalleeNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_libc {
+
+// Gets the outermost namespace of a DeclContext, or the TranslationUnit if
+// there isn't any namespace.
+const DeclContext *getOutermostNamespace(const DeclContext *Decl) {
+  if (Decl->isTranslationUnit())
+    return Decl;
+  if (Decl->getParent() && Decl->getParent()->isTranslationUnit())
+    return Decl;
+  return getOutermostNamespace(Decl->getParent());
+}
+
+void CalleeNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(callee(functionDecl().bind("func"))).bind("call-site"), this);
+}
+
+void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const auto *CallsiteExpr = Result.Nodes.getNodeAs<CallExpr>("call-site");
+
+  // Ignore compiler builtin functions.
+  if (FuncDecl->getBuiltinID() != 0)
+    return;
+
+  // If the outermost namespace of the function is __llvm_libc, we're good.
+  const DeclContext *NS = getOutermostNamespace(cast<DeclContext>(FuncDecl));
+  if (isa<NamespaceDecl>(NS) &&
+      cast<NamespaceDecl>(NS)->getName() == "__llvm_libc")
+    return;
+
+  diag(CallsiteExpr->getBeginLoc(), "function %0 must call to internal "
+                                    "implementation in `__llvm_libc` namespace")
+      << FuncDecl;
+
+  diag(FuncDecl->getLocation(),
+       "currently resolves to: ", clang::DiagnosticIDs::Note);
+}
+
+} // namespace llvm_libc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp

 //===--- LLVMLibcTidyModule.cpp - clang-tidy ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace llvm_libc {
 
 class LLVMLibcModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<CalleeNamespaceCheck>(
+        "llvmlibc-callee-namespace");
     CheckFactories.registerCheck<ImplementationInNamespaceCheck>(
         "llvmlibc-implementation-in-namespace");
     CheckFactories.registerCheck<RestrictSystemLibcHeadersCheck>(
         "llvmlibc-restrict-system-libc-headers");
   }
 };
 
 // Register the LLVMLibcTidyModule using this statically initialized variable.

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-suspicious-include
   them.
 
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
 
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
+- New :doc:`llvmlibc-callee-namespace
+  <clang-tidy/checks/llvmlibc-callee-namespace>` check.
+
+  Checks all calls resolve to functions within __llvm_libc namespace.
+
 - New :doc:`llvmlibc-implementation-in-namespace
   <clang-tidy/checks/llvmlibc-implementation-in-namespace>` check.
 
   Checks all llvm-libc implementation is within the correct namespace.
 
 - New :doc:`llvmlibc-restrict-system-libc-headers
   <clang-tidy/checks/llvmlibc-restrict-system-libc-headers>` check.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `hicpp-signed-bitwise <hicpp-signed-bitwise.html>`_,
    `linuxkernel-must-use-errs <linuxkernel-must-use-errs.html>`_,
    `llvm-header-guard <llvm-header-guard.html>`_,
    `llvm-include-order <llvm-include-order.html>`_, "Yes"
    `llvm-namespace-comment <llvm-namespace-comment.html>`_,
    `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
    `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes"
    `llvm-twine-local <llvm-twine-local.html>`_, "Yes"
+   `llvmlibc-callee-namespace <llvmlibc-calle-namespace.html>`_,
    `llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
    `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-no-recursion <misc-no-recursion.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,

clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst

+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+====================================
+
+Checks all calls resolve to functions within __llvm_libc namespace.
+
+.. code-block:: c++
+    
+    namespace __llvm_libc {
+
+    // Allow calls with the fully qualified name.
+    __llvm_libc::strlen("hello");
+
+    // Allow calls to compiler provided functions.
+    (void)__builtin_abs(-1);
+
+    // Bare calls are allowed as long as they resolve to the correct namespace.
+    strlen("world");
+
+    // Disallow calling into functions in the global namespace.
+    ::strlen("!");
+
+    } // namespace __llvm_libc

clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp

+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+  // Allow calls with the fully qualified name.
+  __llvm_libc::libc_api_func();
+  __llvm_libc::nested::nested_func();
+
+  // Should not trigger on compiler provided function calls.
+  (void)__builtin_abs(-1);
+
+  // Bare calls are allowed as long as they resolve to the correct namespace.
+  libc_api_func();
+  nested::nested_func();
+
+  // Disallow calling into global namespace for implemented entrypoints.
+  ::libc_api_func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'libc_api_func' must call to internal implementation in `__llvm_libc` namespace
+  // CHECK-MESSAGES: :11:6: note: currently resolves to:
+}
+
+} // namespace __llvm_libc