[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin updated this revision to Diff 458155.
Yoorkin marked 5 inline comments as done.
Yoorkin added a comment.

Maybe renamed to bugprone-sprintf-with-fixed-size-buffer would be better. I add 
another option `FlagAllCalls` to flag all calls to `sprintf` and `vsprintf`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-sprintf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
+
+void f4(const char * format, ... ){
+  char buff[100];
+  va_list args;
+  va_start(args, format);
+  vsprintf(buff, format, args);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use vsnprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: vsnprintf(buff, sizeof(buff), format, args);
+  va_end(args);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`bugprone-signed-char-misuse `_,
`bugprone-sizeof-container `_,
`bugprone-sizeof-expression `_,
+   `bugprone-sprintf-with-fixed-size-buffer `_, "Yes"
`bugprone-spuriously-wake-up-functions `_,
`bugprone-string-constructor `_, "Yes"
`bugprone-string-integer-assignment `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - bugprone-sprintf-with-fixed-size-buffer
+
+bugprone-sprintf-with-fixed-size-buffer
+===
+
+Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. 
+It will suggest the counted versions instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+Options
+---
+
+.. option:: PreferSafe
+
+  When `true`, prefer to use ``snprintf_s`` ``vsnprintf_s`` over ``snprintf`` ``vsnprintf``.
+  Default is `false`.
+
+.. option:: FlagAllCalls
+Flag all calls to ``sprintf`` and ``vsprintf``.
+Default is `false`
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-sprintf-with-fixed-size-buffer
+  ` check.
+
+  Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. 
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-ex

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin added a comment.

I want to enable this check in both c and c++, but didn't find option 
`LangOpt.C`. For c++, we have option `LangOpt.CPlusPlus`. There is only some 
specific c standard option like `C99` `C11` `C17` and `C2X` defined in 
`LangOptions.def`. Could you tell me how to do this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin created this revision.
Yoorkin added a reviewer: clang-tools-extra.
Yoorkin added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun, mgorny.
Herald added a project: All.
Yoorkin requested review of this revision.
Herald added a subscriber: cfe-commits.

Hi, I have added a clang-tidy check which proposed in this issue 
.
The check can finds sprintf usage like:

  char buff[50];
  sprintf(buff,"Hello, %s!\n","world");

And suggest counted version function instead:

  warning: string to be written may exceeds the size of buffer; use snprintf 
instead [bugprone-sprintf-with-fixed-size-buffer]
sprintf(buff,"Hello, %s!\n","world");
^~~~
snprintf(buff, sizeof(buff), "Hello, %s!\n", "world")


https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-sprintf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`bugprone-signed-char-misuse `_,
`bugprone-sizeof-container `_,
`bugprone-sizeof-expression `_,
+   `bugprone-sprintf-with-fixed-size-buffer `_, "Yes"
`bugprone-spuriously-wake-up-functions `_,
`bugprone-string-constructor `_, "Yes"
`bugprone-string-integer-assignment `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - bugprone-sprintf-with-fixed-size-buffer
+
+bugprone-sprintf-with-fixed-size-buffer
+===
+
+The check finds usage of `sprintf`, which write output string into a fixed-size 
+array. It will suggest `snprintf` instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
\ No newline at end of file
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-sprintf-with-fixed-size-buffer
+  ` check.
+
+  FIXME: add release notes.
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
@@ -0,0 +1,35 @@
+//===--- SprintfWithFixedSizeBufferCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 wit

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h:29
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+

Eugene.Zelenko wrote:
> Please add `isLanguageVersionSupported` (C and C++).
can i just simply use 
```
  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.C99 || LangOpts.CPlusPlus11;
  }
```
I think the check should be supported after C99 and Cpp11 standards. But i 
don't know whether it return true when language version is c++20 or C11


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin updated this revision to Diff 454226.
Yoorkin marked 10 inline comments as done.
Yoorkin added a comment.

I fixed them all


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-sprintf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`bugprone-signed-char-misuse `_,
`bugprone-sizeof-container `_,
`bugprone-sizeof-expression `_,
+   `bugprone-sprintf-with-fixed-size-buffer `_, "Yes"
`bugprone-spuriously-wake-up-functions `_,
`bugprone-string-constructor `_, "Yes"
`bugprone-string-integer-assignment `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - bugprone-sprintf-with-fixed-size-buffer
+
+bugprone-sprintf-with-fixed-size-buffer
+===
+
+Find usage of ``sprintf``, which write output string into a fixed-size array. It 
+will suggest ``snprintf`` instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-sprintf-with-fixed-size-buffer
+  ` check.
+
+  Find usage of ``sprintf``, which write output string into a fixed-size array.
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
@@ -0,0 +1,38 @@
+//===--- SprintfWithFixedSizeBufferCheck.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_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Check if `sprintf` is called with a fixed size buffer. Replaced with counted
+/// version `snprintf`.
+/

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin updated this revision to Diff 454228.
Yoorkin added a comment.

I fixed it. Thanks


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-sprintf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-sprintf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`bugprone-signed-char-misuse `_,
`bugprone-sizeof-container `_,
`bugprone-sizeof-expression `_,
+   `bugprone-sprintf-with-fixed-size-buffer `_, "Yes"
`bugprone-spuriously-wake-up-functions `_,
`bugprone-string-constructor `_, "Yes"
`bugprone-string-integer-assignment `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - bugprone-sprintf-with-fixed-size-buffer
+
+bugprone-sprintf-with-fixed-size-buffer
+===
+
+Finds usage of ``sprintf``, which write output string into a fixed-size array. It 
+will suggest ``snprintf`` instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-sprintf-with-fixed-size-buffer
+  ` check.
+
+  Finds usage of ``sprintf``, which write output string into a fixed-size array.
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
@@ -0,0 +1,38 @@
+//===--- SprintfWithFixedSizeBufferCheck.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_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Check if `sprintf` is called with a fixed size buffer. Replaced with counted
+/// version `snprintf`.
+///
+/// For the user-facing documentati

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin updated this revision to Diff 454722.
Yoorkin added a comment.

Now it can find usage of both `sprintf` and `vsprintf`, and then change a small 
piece of code. Here is an option `PreferSafe`, when value is true, the check 
prefers `snprintf_s` `vsnprintf_s` over `snprintf` `vsnprintf`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-printf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
+
+void f4(const char * format, ... ){
+  char buff[100];
+  va_list args;
+  va_start(args, format);
+  vsprintf(buff, format, args);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use vsnprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: vsnprintf(buff, sizeof(buff), format, args);
+  va_end(args);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -107,6 +107,7 @@
`bugprone-not-null-terminated-result `_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
+   `bugprone-printf-with-fixed-size-buffer `_, "Yes"
`bugprone-redundant-branch-condition `_, "Yes"
`bugprone-reserved-identifier `_, "Yes"
`bugprone-shared-ptr-array-mismatch `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - bugprone-printf-with-fixed-size-buffer
+
+
+bugprone-printf-with-fixed-size-buffer
+===
+
+Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. 
+It will suggest the counted versions instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+Options
+---
+
+.. option:: PreferSafe
+
+  When `true`, prefer to use ``snprintf_s`` ``vsnprintf_s`` over ``snprintf`` ``vsnprintf``.
+  Default is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-printf-with-fixed-size-buffer
+  ` check.
+
+  Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array.
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
@@ -0,0 +

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

[方程 via Phabricator via cfe-commits]

Yoorkin updated this revision to Diff 454724.
Yoorkin added a comment.

I forgot to update comments in `PrintfWithFixedSizeBufferCheck.h`, sorry


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132294/new/

https://reviews.llvm.org/D132294

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-printf-with-fixed-size-buffer %t
+
+#include 
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+char sbuff[80];
+
+void f2(){
+  sprintf(sbuff, "Hello, %s!\n", "world");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world");
+}
+
+void f3(){
+char *dynamic_sized_buff = nullptr;
+sprintf(dynamic_sized_buff, "Hello, %s!\n", "world");
+}
+
+void f4(const char * format, ... ){
+  char buff[100];
+  va_list args;
+  va_start(args, format);
+  vsprintf(buff, format, args);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use vsnprintf instead [bugprone-printf-with-fixed-size-buffer]
+  // CHECK-FIXES: vsnprintf(buff, sizeof(buff), format, args);
+  va_end(args);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -107,6 +107,7 @@
`bugprone-not-null-terminated-result `_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
+   `bugprone-printf-with-fixed-size-buffer `_, "Yes"
`bugprone-redundant-branch-condition `_, "Yes"
`bugprone-reserved-identifier `_, "Yes"
`bugprone-shared-ptr-array-mismatch `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - bugprone-printf-with-fixed-size-buffer
+
+
+bugprone-printf-with-fixed-size-buffer
+===
+
+Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. 
+It will suggest the counted versions instead.
+
+It's a common idiom to have a fixed-size buffer of characters allocated on 
+the stack and then to printf into the buffer. It may be leading to errors if the 
+string to be written exceeds the size of the array pointed to by buffer.
+
+Example:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  sprintf(buff, "Hello, %s!\n", "world");
+}
+
+Becomes:
+.. code-block:: c++
+
+void f(){
+  char buff[80];
+  snprintf(buff, sizeof(buff), "Hello, %s!\n", "world");
+}
+
+Options
+---
+
+.. option:: PreferSafe
+
+  When `true`, prefer to use ``snprintf_s`` ``vsnprintf_s`` over ``snprintf`` ``vsnprintf``.
+  Default is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@
 New checks
 ^^
 
+- New :doc:`bugprone-printf-with-fixed-size-buffer
+  ` check.
+
+  Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array.
+
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
@@ -0,0 +1,47 @@
+//===--- PrintfWithFixedSizeBufferCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v