https://github.com/ccotter updated https://github.com/llvm/llvm-project/pull/114244
>From fd914cc82688b122654d2d7ada72007541b197c0 Mon Sep 17 00:00:00 2001 From: Chris Cotter <ccotte...@bloomberg.net> Date: Wed, 30 Oct 2024 10:54:49 -0400 Subject: [PATCH] Add bugprone-sprintf-overlap --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/UndefinedSprintfOverlapCheck.cpp | 93 +++++++++++++++++++ .../bugprone/UndefinedSprintfOverlapCheck.h | 36 +++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../bugprone/undefined-sprintf-overlap.rst | 23 +++++ .../docs/clang-tidy/checks/list.rst | 3 +- .../bugprone/undefined-sprintf-overlap.cpp | 59 ++++++++++++ 8 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 33ac65e715ce81..ac3a08c80d7ae6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -86,6 +86,7 @@ #include "TooSmallLoopVariableCheck.h" #include "UncheckedOptionalAccessCheck.h" #include "UndefinedMemoryManipulationCheck.h" +#include "UndefinedSprintfOverlapCheck.h" #include "UndelegatedConstructorCheck.h" #include "UnhandledExceptionAtNewCheck.h" #include "UnhandledSelfAssignmentCheck.h" @@ -248,6 +249,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-unchecked-optional-access"); CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>( "bugprone-undefined-memory-manipulation"); + CheckFactories.registerCheck<UndefinedSprintfOverlapCheck>( + "bugprone-undefined-sprintf-overlap"); CheckFactories.registerCheck<UndelegatedConstructorCheck>( "bugprone-undelegated-constructor"); CheckFactories.registerCheck<UnhandledSelfAssignmentCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index b0a2318acc0597..2403ed665fd5c7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -81,6 +81,7 @@ add_clang_library(clangTidyBugproneModule STATIC TooSmallLoopVariableCheck.cpp UncheckedOptionalAccessCheck.cpp UndefinedMemoryManipulationCheck.cpp + UndefinedSprintfOverlapCheck.cpp UndelegatedConstructorCheck.cpp UnhandledExceptionAtNewCheck.cpp UnhandledSelfAssignmentCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp new file mode 100644 index 00000000000000..301b6bce0dbbad --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp @@ -0,0 +1,93 @@ +//===--- UndefinedSprintfOverlapCheck.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 "UndefinedSprintfOverlapCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +AST_MATCHER_P(CallExpr, hasAnyOtherArgument, + ast_matchers::internal::Matcher<Expr>, InnerMatcher) { + for (const auto *Arg : llvm::drop_begin(Node.arguments())) { + ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder); + if (InnerMatcher.matches(*Arg, Finder, &Result)) { + *Builder = std::move(Result); + return true; + } + } + return false; +} + +AST_MATCHER_P(IntegerLiteral, hasSameValueAs, std::string, ID) { + return Builder->removeBindings( + [this, &Node](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto &BN = Nodes.getNode(ID); + if (const auto *Lit = BN.get<IntegerLiteral>()) { + return Lit->getValue() != Node.getValue(); + } + return true; + }); +} + +UndefinedSprintfOverlapCheck::UndefinedSprintfOverlapCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + SprintfRegex(Options.get("SprintfFunction", "(::std)?::(sn?printf)")) {} + +void UndefinedSprintfOverlapCheck::registerMatchers(MatchFinder *Finder) { + auto FirstArg = declRefExpr(to(varDecl().bind("firstArg"))); + auto OtherRefToArg = declRefExpr(to(varDecl(equalsBoundNode("firstArg")))) + .bind("overlappingArg"); + Finder->addMatcher( + callExpr(callee(functionDecl(matchesName(SprintfRegex)).bind("decl")), + allOf(hasArgument( + 0, anyOf(FirstArg.bind("firstArgExpr"), + arraySubscriptExpr( + hasBase(FirstArg), + hasIndex(integerLiteral().bind("index"))) + .bind("firstArgExpr"), + memberExpr(member(decl().bind("member")), + hasObjectExpression(FirstArg)) + .bind("firstArgExpr"))), + hasAnyOtherArgument(anyOf( + OtherRefToArg, + arraySubscriptExpr( + hasBase(OtherRefToArg), + hasIndex(integerLiteral(hasSameValueAs("index")))), + memberExpr(member(decl(equalsBoundNode("member"))), + hasObjectExpression(OtherRefToArg)))))), + this); +} + +void UndefinedSprintfOverlapCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *OverlappingArg = + Result.Nodes.getNodeAs<DeclRefExpr>("overlappingArg"); + const auto *FirstArg = Result.Nodes.getNodeAs<Expr>("firstArgExpr"); + const auto *FnDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl"); + + llvm::StringRef FirstArgText = + Lexer::getSourceText(CharSourceRange::getTokenRange( + FirstArg->getBeginLoc(), FirstArg->getEndLoc()), + *Result.SourceManager, getLangOpts()); + + diag(OverlappingArg->getLocation(), + "argument '%0' overlaps the first argument " + "in %1, which is undefined behavior") + << FirstArgText << FnDecl; +} + +void UndefinedSprintfOverlapCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "SprintfRegex", SprintfRegex); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h new file mode 100644 index 00000000000000..349b1156671ff0 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h @@ -0,0 +1,36 @@ +//===--- UndefinedSprintfOverlapCheck.h - clang-tidy ------------*- 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_BUGPRONE_UNDEFINEDSPRINTFOVERLAPCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNDEFINEDSPRINTFOVERLAPCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/undefined-sprintf-overlap.html +class UndefinedSprintfOverlapCheck : public ClangTidyCheck { +public: + UndefinedSprintfOverlapCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + const std::string SprintfRegex; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNDEFINEDSPRINTFOVERLAPCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ccebf74e8a67e7..7285343971189f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -131,6 +131,12 @@ New checks Gives warnings for tagged unions, where the number of tags is different from the number of data members inside the union. +- New :doc:`bugprone-undefined-sprintf-overlap + <clang-tidy/checks/bugprone/undefined-sprintf-overlap>` check. + + Finds calls to sprintf family of functions whose first argument + overlaps with a subsequent argument. + - New :doc:`portability-template-virtual-member-function <clang-tidy/checks/portability/template-virtual-member-function>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst new file mode 100644 index 00000000000000..775d99c9f1e0bb --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst @@ -0,0 +1,23 @@ +.. title:: clang-tidy - bugprone-undefined-sprintf-overlap + +bugprone-undefined-sprintf-overlap +================================== + +Warns if any arguments to the sprintf family of functions overlap with the +first argument. + +.. code-block:: c++ + + char buf[20] = {"hi"}; + sprintf(buf, "%s%d", buf, 0); + +C99 and POSIX.1-2001 states that if copying were to take place between objects +that overlap, the result is undefined. + +Options +------- + +.. option:: SprintfRegex + + A regex specifying the sprintf family of functions to match on. By default, + this is "(::std)?::sn?printf". diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index d731b13fc0df44..41b7d92f94f90c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -115,8 +115,8 @@ Clang-Tidy Checks :doc:`bugprone-multiple-new-in-one-expression <bugprone/multiple-new-in-one-expression>`, :doc:`bugprone-multiple-statement-macro <bugprone/multiple-statement-macro>`, :doc:`bugprone-no-escape <bugprone/no-escape>`, - :doc:`bugprone-nondeterministic-pointer-iteration-order <bugprone/nondeterministic-pointer-iteration-order>`, :doc:`bugprone-non-zero-enum-to-bool-conversion <bugprone/non-zero-enum-to-bool-conversion>`, + :doc:`bugprone-nondeterministic-pointer-iteration-order <bugprone/nondeterministic-pointer-iteration-order>`, :doc:`bugprone-not-null-terminated-result <bugprone/not-null-terminated-result>`, "Yes" :doc:`bugprone-optional-value-conversion <bugprone/optional-value-conversion>`, "Yes" :doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes" @@ -153,6 +153,7 @@ Clang-Tidy Checks :doc:`bugprone-too-small-loop-variable <bugprone/too-small-loop-variable>`, :doc:`bugprone-unchecked-optional-access <bugprone/unchecked-optional-access>`, :doc:`bugprone-undefined-memory-manipulation <bugprone/undefined-memory-manipulation>`, + :doc:`bugprone-undefined-sprintf-overlap <bugprone/undefined-sprintf-overlap>`, "Yes" :doc:`bugprone-undelegated-constructor <bugprone/undelegated-constructor>`, :doc:`bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new>`, :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp new file mode 100644 index 00000000000000..50095346da8a8d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp @@ -0,0 +1,59 @@ +// RUN: %check_clang_tidy %s bugprone-undefined-sprintf-overlap %t + +using size_t = decltype(sizeof(int)); + +extern "C" int sprintf(char *s, const char *format, ...); +extern "C" int snprintf(char *s, size_t n, const char *format, ...); + +namespace std { + int snprintf(char *s, size_t n, const char *format, ...); +} + +struct st_t { + char buf[10]; + char buf2[10]; +}; + +void first_arg_overlaps() { + char buf[10]; + sprintf(buf, "%s", buf); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: argument 'buf' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + snprintf(buf, sizeof(buf), "%s", buf); + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: argument 'buf' overlaps the first argument in 'snprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + std::snprintf(buf, sizeof(buf), "%s", buf); + // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: argument 'buf' overlaps the first argument in 'snprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + + char* c = &buf[0]; + sprintf(c, "%s", c); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: argument 'c' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + snprintf(c, sizeof(buf), "%s", c); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: argument 'c' overlaps the first argument in 'snprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + + snprintf(c, sizeof(buf), "%s%s", c, c); + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: argument 'c' overlaps the first argument in 'snprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + + char buf2[10]; + snprintf(buf, sizeof(buf), "%s", buf2); + + st_t st1, st2; + sprintf(st1.buf, "%s", st1.buf); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: argument 'st1.buf' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + sprintf(st1.buf, "%s", st1.buf2); + sprintf(st1.buf, "%s", st2.buf); + + st_t* stp; + sprintf(stp->buf, "%s", stp->buf); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: argument 'stp->buf' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + sprintf((stp->buf), "%s", stp->buf); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: argument 'stp->buf' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + stp = &st1; + sprintf(stp->buf, "%s", st1.buf); + + char bufs[10][10]; + sprintf(bufs[1], "%s", bufs[1]); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: argument 'bufs[1]' overlaps the first argument in 'sprintf', which is undefined behavior [bugprone-undefined-sprintf-overlap] + sprintf(bufs[0], "%s", bufs[1]); + + char bufss[10][10][10]; + sprintf(bufss[0][1], "%s", bufss[0][1]); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits