[clang-tools-extra] Add bugprone-sprintf-overlap (PR #114244)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Chris Cotter (ccotter)

<details>
<summary>Changes</summary>



---
Full diff: https://github.com/llvm/llvm-project/pull/114244.diff


8 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3) 
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) 
- (added) clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.cpp (+52) 
- (added) clang-tools-extra/clang-tidy/bugprone/UndefinedSprintfOverlapCheck.h (+33) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) 
- (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/undefined-sprintf-overlap.rst (+15) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+2-1) 
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-sprintf-overlap.cpp (+29) 


``````````diff
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]
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/114244