ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, dexonsmith, steven_wu, jkorous, hiraditya, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. Herald added a subscriber: ilya-biryukov.
This patch fixes crashes in the following cases: - `SIGTERM` is not a literal - `SIGTERM` is a literal but `Token::PtrData == nullptr` (happens in `clangd`, because `clang-tidy` in `clangd` has some limitations to ensure reasonable performance) - `SIGTERM` undefined after definition Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D85301 Files: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp @@ -0,0 +1,11 @@ +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + +#define SIGTERM 15 +#undef SIGTERM // no-crash +using pthread_t = int; +int pthread_kill(pthread_t thread, int sig); + +int func() { + pthread_t thread; + return pthread_kill(thread, 0); +} Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp @@ -0,0 +1,10 @@ +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + +#define SIGTERM ((unsigned)15) // no-crash +using pthread_t = int; +int pthread_kill(pthread_t thread, int sig); + +int func() { + pthread_t thread; + return pthread_kill(thread, 0); +} Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -438,6 +438,21 @@ EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); } +TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) { + Annotations Main(R"cpp( + #define SIGTERM 15 + using pthread_t = int; + int pthread_kill(pthread_t thread, int sig); + int func() { + pthread_t thread; + return pthread_kill(thread, 0); + } + )cpp"); + TestTU TU = TestTU::withCode(Main.code()); + TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread"; + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp @@ -30,7 +30,8 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { const auto IsSigterm = [](const auto &KeyValue) -> bool { - return KeyValue.first->getName() == "SIGTERM"; + return KeyValue.first->getName() == "SIGTERM" && + KeyValue.first->hasMacroDefinition(); }; const auto TryExpandAsInteger = [](Preprocessor::macro_iterator It) -> Optional<unsigned> { @@ -38,6 +39,8 @@ return llvm::None; const MacroInfo *MI = PP->getMacroInfo(It->first); const Token &T = MI->tokens().back(); + if (!T.isLiteral() || !T.getLiteralData()) + return llvm::None; StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); llvm::APInt IntValue;
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp @@ -0,0 +1,11 @@ +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + +#define SIGTERM 15 +#undef SIGTERM // no-crash +using pthread_t = int; +int pthread_kill(pthread_t thread, int sig); + +int func() { + pthread_t thread; + return pthread_kill(thread, 0); +} Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp @@ -0,0 +1,10 @@ +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + +#define SIGTERM ((unsigned)15) // no-crash +using pthread_t = int; +int pthread_kill(pthread_t thread, int sig); + +int func() { + pthread_t thread; + return pthread_kill(thread, 0); +} Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -438,6 +438,21 @@ EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); } +TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) { + Annotations Main(R"cpp( + #define SIGTERM 15 + using pthread_t = int; + int pthread_kill(pthread_t thread, int sig); + int func() { + pthread_t thread; + return pthread_kill(thread, 0); + } + )cpp"); + TestTU TU = TestTU::withCode(Main.code()); + TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread"; + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp @@ -30,7 +30,8 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { const auto IsSigterm = [](const auto &KeyValue) -> bool { - return KeyValue.first->getName() == "SIGTERM"; + return KeyValue.first->getName() == "SIGTERM" && + KeyValue.first->hasMacroDefinition(); }; const auto TryExpandAsInteger = [](Preprocessor::macro_iterator It) -> Optional<unsigned> { @@ -38,6 +39,8 @@ return llvm::None; const MacroInfo *MI = PP->getMacroInfo(It->first); const Token &T = MI->tokens().back(); + if (!T.isLiteral() || !T.getLiteralData()) + return llvm::None; StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); llvm::APInt IntValue;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits