[clang-tools-extra] Add bugprone-sprintf-overlap (PR #114244)
Chris Cotter via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 30 19:16:12 PDT 2024
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 <ccotter14 at 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]);
+}
More information about the cfe-commits
mailing list