[clang-tools-extra] Add bugprone-sprintf-overlap (PR #114244)
Chris Cotter via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 30 09:53:34 PDT 2024
https://github.com/ccotter updated https://github.com/llvm/llvm-project/pull/114244
>From 835540fb51517eb2b9e80a7c3b23988419cfc962 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 1/3] Add bugprone-sprintf-overlap
---
.../bugprone/BugproneTidyModule.cpp | 3 ++
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../bugprone/UndefinedSprintfOverlapCheck.cpp | 52 +++++++++++++++++++
.../bugprone/UndefinedSprintfOverlapCheck.h | 33 ++++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 +++
.../bugprone/undefined-sprintf-overlap.rst | 15 ++++++
.../docs/clang-tidy/checks/list.rst | 3 +-
.../bugprone/undefined-sprintf-overlap.cpp | 29 +++++++++++
8 files changed, 141 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..5b5507c6263224
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp
@@ -0,0 +1,52 @@
+//===--- 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"
+
+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;
+}
+
+void UndefinedSprintfOverlapCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(
+ callee(
+ functionDecl(matchesName("(::std)?::(sn?printf)")).bind("decl")),
+ hasArgument(0, ignoringParenImpCasts(
+ declRefExpr(to(varDecl().bind("firstArg"))))),
+ hasAnyOtherArgument(
+ ignoringParenImpCasts(declRefExpr().bind("overlappingArg")))),
+ this);
+}
+
+void UndefinedSprintfOverlapCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *OverlappingArg =
+ Result.Nodes.getNodeAs<DeclRefExpr>("overlappingArg");
+ const auto *FirstArg = Result.Nodes.getNodeAs<VarDecl>("firstArg");
+ const auto *FnDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+
+ diag(OverlappingArg->getLocation(), "argument %0 overlaps the first argument "
+ "in %1, which is undefined behavior")
+ << FirstArg << FnDecl;
+}
+
+} // 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..0e100aa1a5bfdf
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
@@ -0,0 +1,33 @@
+//===--- 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)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // 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..020d18a0802198
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst
@@ -0,0 +1,15 @@
+.. 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.
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..24d2939aee2c74
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp
@@ -0,0 +1,29 @@
+// 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, ...);
+}
+
+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]
+}
>From 15e027e2d7a7911ee51433bc1e5835bd2e7be0d1 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14 at bloomberg.net>
Date: Wed, 30 Oct 2024 12:30:33 -0400
Subject: [PATCH 2/3] Make configurable
---
.../clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp | 9 ++++++++-
.../clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h | 7 +++++--
.../checks/bugprone/undefined-sprintf-overlap.rst | 8 ++++++++
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp
index 5b5507c6263224..b2452ca8a70a7f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp
@@ -25,11 +25,14 @@ AST_MATCHER_P(CallExpr, hasAnyOtherArgument,
return false;
}
+UndefinedSprintfOverlapCheck::UndefinedSprintfOverlapCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), SprintfRegex(Options.get("SprintfFunction", "(::std)?::(sn?printf)")) {}
+
void UndefinedSprintfOverlapCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(
callee(
- functionDecl(matchesName("(::std)?::(sn?printf)")).bind("decl")),
+ functionDecl(matchesName(SprintfRegex)).bind("decl")),
hasArgument(0, ignoringParenImpCasts(
declRefExpr(to(varDecl().bind("firstArg"))))),
hasAnyOtherArgument(
@@ -49,4 +52,8 @@ void UndefinedSprintfOverlapCheck::check(
<< FirstArg << 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
index 0e100aa1a5bfdf..291d956dc8bd2d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
@@ -19,13 +19,16 @@ namespace clang::tidy::bugprone {
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/undefined-sprintf-overlap.html
class UndefinedSprintfOverlapCheck : public ClangTidyCheck {
public:
- UndefinedSprintfOverlapCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ 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;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
+
+private:
+ const std::string SprintfRegex;
};
} // namespace clang::tidy::bugprone
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
index 020d18a0802198..775d99c9f1e0bb 100644
--- 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
@@ -13,3 +13,11 @@ first argument.
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".
>From db0163b55d27ff73b26db591182438c06c154566 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14 at bloomberg.net>
Date: Wed, 30 Oct 2024 12:47:40 -0400
Subject: [PATCH 3/3] Remove C++ specific check
---
.../clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
index 291d956dc8bd2d..e11fd33c56adc5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h
@@ -23,9 +23,6 @@ class UndefinedSprintfOverlapCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
- bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
- return LangOpts.CPlusPlus;
- }
private:
const std::string SprintfRegex;
More information about the cfe-commits
mailing list