diff --git a/libc/utils/CMakeLists.txt b/libc/utils/CMakeLists.txt --- a/libc/utils/CMakeLists.txt +++ b/libc/utils/CMakeLists.txt @@ -1,5 +1,6 @@ add_subdirectory(CPP) add_subdirectory(HdrGen) +add_subdirectory(LibcTidy) add_subdirectory(testutils) add_subdirectory(UnitTest) add_subdirectory(benchmarks) diff --git a/libc/utils/HdrGen/PublicAPICommand.h b/libc/utils/HdrGen/PublicAPICommand.h --- a/libc/utils/HdrGen/PublicAPICommand.h +++ b/libc/utils/HdrGen/PublicAPICommand.h @@ -12,6 +12,7 @@ #include "Command.h" #include "llvm/ADT/StringRef.h" +#include "llvm/TableGen/Record.h" #include #include @@ -38,4 +39,15 @@ } // namespace llvm_libc +inline bool isa(llvm::Record *Def, llvm::Record *TypeClass) { + llvm::RecordRecTy *RecordType = Def->getType(); + llvm::ArrayRef Classes = RecordType->getClasses(); + // We want exact types. That is, we don't want the classes listed in + // spec.td to be subclassed. Hence, we do not want the record |Def| + // to be of more than one class type. + if (Classes.size() != 1) + return false; + return Classes[0] == TypeClass; +} + #endif // LLVM_LIBC_UTILS_HDRGEN_PUBLICAPICOMMAND_H diff --git a/libc/utils/HdrGen/PublicAPICommand.cpp b/libc/utils/HdrGen/PublicAPICommand.cpp --- a/libc/utils/HdrGen/PublicAPICommand.cpp +++ b/libc/utils/HdrGen/PublicAPICommand.cpp @@ -12,7 +12,6 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/SourceMgr.h" #include "llvm/TableGen/Error.h" -#include "llvm/TableGen/Record.h" static const char NamedTypeClassName[] = "NamedType"; static const char PtrTypeClassName[] = "PtrType"; @@ -23,17 +22,6 @@ static const char StandardSpecClassName[] = "StandardSpec"; static const char PublicAPIClassName[] = "PublicAPI"; -static bool isa(llvm::Record *Def, llvm::Record *TypeClass) { - llvm::RecordRecTy *RecordType = Def->getType(); - llvm::ArrayRef Classes = RecordType->getClasses(); - // We want exact types. That is, we don't want the classes listed in - // spec.td to be subclassed. Hence, we do not want the record |Def| - // to be of more than one class type.. - if (Classes.size() != 1) - return false; - return Classes[0] == TypeClass; -} - // Text blocks for macro definitions and type decls can be indented to // suit the surrounding tablegen listing. We need to dedent such blocks // before writing them out. diff --git a/libc/utils/LibcTidy/CMakeLists.txt b/libc/utils/LibcTidy/CMakeLists.txt new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/CMakeLists.txt @@ -0,0 +1,17 @@ +set(LLVM_LINK_COMPONENTS support TableGen) +include(${PROJECT_SOURCE_DIR}/../clang/cmake/modules/AddClang.cmake) + +include_directories(${PROJECT_BINARY_DIR}/tools/clang/include/) +include_directories(${PROJECT_SOURCE_DIR}/../clang/include/) + +add_clang_executable(libc-tidy + LibcTidyTool.cpp + TableGenUtil.cpp) + +target_link_libraries(libc-tidy + PRIVATE + clangTooling + clangBasic + clangASTMatchers) + +add_subdirectory(tests) diff --git a/libc/utils/LibcTidy/LibcTidyTool.cpp b/libc/utils/LibcTidy/LibcTidyTool.cpp new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/LibcTidyTool.cpp @@ -0,0 +1,134 @@ +//===-- LibcTidyTool.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 "TableGenUtil.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Tooling/CommonOptionsParser.h" +#include "clang/Tooling/Tooling.h" + +#include "llvm/Support/CommandLine.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/raw_ostream.h" + +#include "llvm/TableGen/Main.h" +#include "llvm/TableGen/Record.h" + +using namespace clang; +using namespace clang::ast_matchers; + +// Apply a custom category to all command-line options so that they are the +// only ones displayed. +static llvm::cl::OptionCategory LibCTidyCategory("libc-tidy options"); + +llvm::cl::opt TableGenFilePathOption( + "tablegen-filepath", + llvm::cl::desc( + "The filepath to the '.td' file containing libc public api functions"), + llvm::cl::Required, llvm::cl::cat(LibCTidyCategory)); + +llvm::cl::opt TableGenIncludeDirOption( + "tablegen-includedir", + llvm::cl::desc("The include dir to pass to tablegen."), llvm::cl::Required, + llvm::cl::cat(LibCTidyCategory)); + +bool isNOLINTPresent(const SourceManager *SM, SourceLocation loc) { + FileID file; + unsigned offset; + std::tie(file, offset) = SM->getDecomposedSpellingLoc(loc); + llvm::Optional buffer = SM->getBufferData(file); + if (!buffer) + return false; + + // Check if there's a NOLINT on this line. + StringRef restOfLine = buffer->substr(offset).split('\n').first; + return restOfLine.contains("LIBC-NOLINT"); +} + +//-- Check Callbacks --// +class QualifiedCallCheck : public MatchFinder::MatchCallback { +public: + QualifiedCallCheck(NameSet &functions) : functions(functions) {} + + void registerMatchers(MatchFinder &finder) { + StatementMatcher qualifiedCallMatcher = + callExpr(callee(functionDecl().bind("func"))).bind("call-site"); + finder.addMatcher(qualifiedCallMatcher, this); + } + + void run(const MatchFinder::MatchResult &result) override { + const auto *funcDecl = result.Nodes.getNodeAs("func"); + const auto *callsiteExpr = result.Nodes.getNodeAs("call-site"); + SourceLocation loc = callsiteExpr->getBeginLoc(); + + // Only look for libc public API function calls. + if (functions.find(funcDecl->getNameInfo().getAsString()) == + functions.end()) + return; + // If the call is prefixed with the qualified __llvm_libc:: it's good to go. + if (llvm::StringRef(funcDecl->getQualifiedNameAsString()) + .startswith("__llvm_libc::")) + return; + // Ignore calls where there's a NOLINT set. + if (isNOLINTPresent(result.SourceManager, loc)) + return; + + DiagnosticsEngine &DE = result.Context->getDiagnostics(); + const unsigned ID = DE.getCustomDiagID( + DiagnosticsEngine::Error, + "%0 must be called with qualified name '__llvm_libc'."); + DE.Report(loc, ID).AddString(funcDecl->getName()); + } + +private: + NameSet &functions; +}; + +NameSet publicFunctions; + +bool TidyTableGenMain(llvm::raw_ostream &OS, llvm::RecordKeeper &records) { + publicFunctions = getAllLibcFunctions(records); + return false; +} + +//-- END Check Callbacks --// + +int main(int argc, const char **argv) { + // Both clang tooling and TableGen expect positional arguments. So instead we + // remove and manually provide the arguments below for TableGenMain. + llvm::cl::Option *tblGenFileOpt = + llvm::cl::TopLevelSubCommand->PositionalOpts.pop_back_val(); + llvm::cl::Option *tblGenIncludeOpt = + llvm::cl::TopLevelSubCommand->OptionsMap["I"]; + + tooling::CommonOptionsParser OptionsParser(argc, argv, LibCTidyCategory); + tooling::ClangTool Tool(OptionsParser.getCompilations(), + OptionsParser.getSourcePathList()); + + // Manually pass our option values to TableGenMain's options. + llvm::cl::ProvidePositionalOption(tblGenFileOpt, TableGenFilePathOption, 0); + llvm::cl::ProvidePositionalOption(tblGenIncludeOpt, TableGenIncludeDirOption, + 0); + + // TODO: argv needs to be const for tooling's OptionParser above but + // TableGenMain expects a regular char*. There might be a cleaner way + // to grab argv[0] as a plain char* from the const version above... + std::string programName(argv[0]); + TableGenMain(&programName[0], &TidyTableGenMain); + + MatchFinder finder; + + // Call Checks + QualifiedCallCheck qualifiedCallCheck(publicFunctions); + qualifiedCallCheck.registerMatchers(finder); + + return Tool.run(tooling::newFrontendActionFactory(&finder).get()); +} diff --git a/libc/utils/LibcTidy/TableGenUtil.h b/libc/utils/LibcTidy/TableGenUtil.h new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/TableGenUtil.h @@ -0,0 +1,21 @@ +//===-- TableGenUtil.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_LIBC_UTILS_LIBCTIDY_TABLEGENUTIL_H +#define LLVM_LIBC_UTILS_LIBCTIDY_TABLEGENUTIL_H + +#include "llvm/Support/Error.h" +#include "llvm/TableGen/Record.h" +#include +#include + +using NameSet = std::unordered_set; + +NameSet getAllLibcFunctions(llvm::RecordKeeper &records); + +#endif // LLVM_LIBC_UTILS_LIBCTIDY_TABLEGENUTIL_H diff --git a/libc/utils/LibcTidy/TableGenUtil.cpp b/libc/utils/LibcTidy/TableGenUtil.cpp new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/TableGenUtil.cpp @@ -0,0 +1,28 @@ +//===-- TableGenUtil.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 "TableGenUtil.h" +#include "../HdrGen/PublicAPICommand.h" + +NameSet getAllLibcFunctions(llvm::RecordKeeper &records) { + NameSet funcNames; + + llvm::Record *publicAPIClass = records.getClass("PublicAPI"); + const auto &defsMap = records.getDefs(); + for (const auto &pair : defsMap) { + llvm::Record *def = pair.second.get(); + if (!isa(def, publicAPIClass)) + continue; + + auto functionNameList = def->getValueAsListOfStrings("Functions"); + for (llvm::StringRef functionName : functionNameList) { + funcNames.insert(std::string(functionName)); + } + } + return funcNames; +} diff --git a/libc/utils/LibcTidy/tests/.clang-format b/libc/utils/LibcTidy/tests/.clang-format new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/tests/.clang-format @@ -0,0 +1,2 @@ +BasedOnStyle: LLVM +ColumnLimit: 0 diff --git a/libc/utils/LibcTidy/tests/CMakeLists.txt b/libc/utils/LibcTidy/tests/CMakeLists.txt new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/tests/CMakeLists.txt @@ -0,0 +1,8 @@ +add_lit_testsuite(check-libc-tidy "Libc-Tidy Regression Tests." + ${CMAKE_CURRENT_BINARY_DIR} + DEPENDS libc-tidy FileCheck not + EXCLUDE_FROM_CHECK_ALL) + +configure_lit_site_cfg( + ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in + ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg) diff --git a/libc/utils/LibcTidy/tests/Test.cpp b/libc/utils/LibcTidy/tests/Test.cpp new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/tests/Test.cpp @@ -0,0 +1,26 @@ +// RUN: not libc-tidy %s --tablegen-filepath "%S/mock_api.td" \ +// RUN: --tablegen-includedir "%S/../../../" -- 2>&1 | FileCheck %s + +namespace __llvm_libc { +void libc_api_func() {} +} // namespace __llvm_libc +void libc_api_func() {} +void regular_func() {} + +void Test() { + // Allow calls with the fully qualified name. + __llvm_libc::libc_api_func(); + // CHECK-NOT: error: libc_api_func must be called with qualified name '__llvm_libc'. + + // Should not trigger on non libc functions. + regular_func(); + // CHECK-NOT: error: regular_func must be called with qualified name '__llvm_libc'. + + // Disallow just plain calls. + libc_api_func(); + // CHECK: :[[@LINE-1]]:3: error: libc_api_func must be called with qualified name '__llvm_libc'. + + // Allow when NOLINT is explicitly requested. + libc_api_func(); // LIBC-NOLINT + // CHECK-NOT: error: libc_api_func must be called with qualified name '__llvm_libc'. +} diff --git a/libc/utils/LibcTidy/tests/lit.site.cfg.in b/libc/utils/LibcTidy/tests/lit.site.cfg.in new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/tests/lit.site.cfg.in @@ -0,0 +1,8 @@ +import os +import lit.formats + +config.name = 'libc-tidy' +config.test_format = lit.formats.ShTest() +config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@" +config.suffixes = ['.cpp'] +config.environment['PATH'] = "@LLVM_TOOLS_DIR@" diff --git a/libc/utils/LibcTidy/tests/mock_api.td b/libc/utils/LibcTidy/tests/mock_api.td new file mode 100644 --- /dev/null +++ b/libc/utils/LibcTidy/tests/mock_api.td @@ -0,0 +1,13 @@ +include "config/public_api.td" + +def FakeAPI : PublicAPI<"fakeapi.h"> { + let Functions = [ + "libc_api_func", + ]; + + let TypeDeclarations = [ + ]; + + let Macros = [ + ]; +}