[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely marked 7 inline comments as done.
futogergely added a comment.

In D91000#3506605 , @whisperity wrote:

> Just one question if you could try this out for me: what happens if you run 
> `clang-tidy a.c b.c` (two TUs in the invocation) where **one of them** 
> (preferably the later one, i.e. **`b.c`**) does //NOT// have Annex K enabled? 
> I believe the cached `IsAnnexKAvailable` (like any other TU-specific state of 
> the check instance) should be invalidated/cleared in an overridden `void 
> onStartTranslationUnit()` function.
>
> Also, what happens if the check is run for C++ code?

It is working as is, a new ClangTidyCheck is created for every translation unit.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 440202.
futogergely added a comment.

updates based on comments.


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:41: warning: function 'asctime' is not bounds-checking and non-re

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

In D91000#3612090 , @Eugene.Zelenko 
wrote:

> Locations for tests and check documentation was changed recently. Please 
> rebase from `main` and adjust your code accordingly.

Done.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 440216.

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+}
+
+typedef v

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 440259.

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+}
+
+typedef v

[PATCH] D91000: [clang-tidy] Add cert-msc24-msc33-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.
Herald added a project: All.

In D91000#3313892 , @balazske wrote:

> Now it would be better to have a checker called `UnsafeFunctionsCheck` 
> (probably in bugprone) and add the cert checkers "msc24-c" and "msc33-c" as 
> aliases. This makes the check extendable if more (CERT rule related or not) 
> cases for unsafe functions are added.

Done


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 414359.
futogergely retitled this revision from "[clang-tidy] Add cert-msc24-msc33-c 
checker." to "[clang-tidy] Add bugprone-unsafe-functions checker.".
futogergely added a comment.

Checker has been moved to bugprone.


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-usafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asc

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 421435.

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+}
+
+typedef v

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely marked 14 inline comments as done and an inline comment as not done.
futogergely added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:41-42
+
+  // Matching the `gets` deprecated function without replacement.
+  auto DeprecatedFunctionNamesMatcher = hasAnyName("::gets");
+

whisperity wrote:
> whisperity wrote:
> > futogergely wrote:
> > > aaron.ballman wrote:
> > > > This comment is not accurate. `gets_s()` is a secure replacement for 
> > > > `gets()`.
> > > If gets is removed from C11, and gets_s is introduced in C11, then gets_s 
> > > cannot be a replacement or? Maybe fgets?
> > > 
> > > Also I was wondering if we would like to disable this check for C99, 
> > > maybe we should remove the check for gets all together.
> > Yes, it's strange territory. If I make my code safer but //stay// pre-C11, 
> > I actually can't, because the new function isn't there yet. If I also 
> > //upgrade// then I'll **have to** make my code "safer", because the old 
> > function is now missing...
> > 
> > Given how brutally dangerous `gets` is (you very rarely see a documentation 
> > page just going **`Never use gets()!`**), I would suggest keeping at least 
> > the warning.
> > 
> > Although even the CERT rule mentions `gets_s()`. We could have some wiggle 
> > room here: do the warning for `gets()`, and suggest two alternatives: 
> > `fgets()`, or `gets_s()` + Annex K? `fgets(stdin, ...)` is also safe, if 
> > the buffer's size is given appropriately.
> CERT mentions C99 TC3, which //seems to be// available here: 
> https://webstore.iec.ch/p-corrigenda/isoiec9899-cor3%7Bed2.0%7Den.pdf . I'm 
> not sure how normative this source is (`iec.ch` seems legit to me that this 
> isn't just a random WGML draft!), and on page 8, in point 46 it says: //"Add 
> new paragraph after paragraph 1: The `gets` function is obsolescent, and is 
> deprecated."//.
> 
> This seems like nitpicking, but maybe CppReference is outdated as it never 
> indicated the "deprecation" period?
> 
> (FYI: http://www.iso.org/standard/50510.html does not offer any purchase of 
> the older version of the standard, or this errata.)
I don't use 'deprecated' in the warning message, I issue 'is insecure, and it 
is removed from C11' for gets instead.

I added the following replacement suggestions for gets: gets_s if Annex K is 
available, fgets if Annex K is not available.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:49-59
+  "::asctime", "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen",
+  "::fscanf", "::fwprintf", "::fwscanf", "::getenv", "::gmtime",
+  "::localtime", "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove",
+  "::printf", "::qsort", "::snprintf", "::sprintf", "::sscanf", "::strcat",
+  "::strcpy", "::strerror", "::strncat", "::strncpy", "::strtok",
+  "::swprintf", "::swscanf", "::vfprintf", "::vfscanf", "::vfwprintf",
+  "::vfwscanf", "::vprintf", "::vscanf", "::vsnprintf", "::vsprintf",

futogergely wrote:
> aaron.ballman wrote:
> > This list appears to be missing quite a few functions with secure 
> > replacements in Annex K. For example: `tmpfile_s`, `tmpnam_s`, 
> > `strerrorlen_s`, `strlen_s`... can you check the list against the actual 
> > Annex K, as it seems the CERT recommendation is still out of date.
> Missing functions added: tmpfile/tmpfile_s, tmpnam/tmpnam_s, memset/memset_s, 
> scanf, strlen, wcslen
strlen/strnlen_s, wcslen/wcsnlen_s, memset/memset_s, scanf/scanf_s has been 
added.

I did not add tmpfile/tmpfile_s, tmpnam/tmpnam_s because there is a separate 
CERT rule for it: 
https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-msc33-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 407819.
futogergely marked an inline comment as done.
futogergely retitled this revision from "[clang-tidy] Add cert-msc24-c 
checker." to "[clang-tidy] Add cert-msc24-msc33-c checker.".
futogergely edited the summary of this revision.
futogergely added a comment.

I changed the class name: ObsolescentFunctionsCheck->UnsafeFunctionsCheck.
Since MSC33-C is also included, I changed the checker name to 
cert-msc24-msc33-c.
I added the following functions from CheckSecuritySyntaxOnly under option 
'ReportMoreUnsafeFunctions': bcmp, bcopy, bzero, getpw, vfork. Since there is a 
replacement suggested there, I added the replacement suggestions also.
I did not add tmpnam, tmpfile, mktemp, mkstemp, rand..() to the checker, 
because there are separate CERT rules for these.


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/cert/UnsafeFunctionsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc24-msc33-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-msc24-msc33-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-msc24-msc33-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-msc24-msc33-c.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s cert-msc24-msc33-c %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-msc33-c %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-msc33-c %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-msc33-c %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s cert-msc24-msc33-c %t -- \
+// RUN:   -config="{CheckOptions: [{key: cert-msc24-msc33-c.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and n

[PATCH] D103595: [clang] Correct MarkFunctionReferenced for local class

[Fütő Gergely via Phabricator via cfe-commits]

futogergely created this revision.
futogergely added a reviewer: rsmith.
futogergely requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Minor correction after commit 4a941e25f2b57f85eef00a9cbfbc2569639570ad.

If during the instantiation of a local class a method of the
local class is referenced, don't try to instantiate it.

PR48839


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103595

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/instantiate-local-class.cpp


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+  template 
+  void construct() {
+T(0);
+  }
+
+  template 
+  void tester() {
+struct d {
+  void test() {
+construct();
+}
+constexpr d(T b) : a(b) {}
+
+T a;
+};
+  }
+
+  void g() { tester(); }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,10 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass &&
   CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+  template 
+  void construct() {
+T(0);
+  }
+
+  template 
+  void tester() {
+struct d {
+  void test() {
+construct();
+}
+constexpr d(T b) : a(b) {}
+
+T a;
+};
+  }
+
+  void g() { tester(); }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,10 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass &&
   CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103595: [clang] Correct MarkFunctionReferenced for local class

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 349480.
futogergely added a comment.

formatting in instantiate-local-class.cpp


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

https://reviews.llvm.org/D103595

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/instantiate-local-class.cpp


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+  template 
+  void construct() {
+T(0);
+  }
+
+  template 
+  void tester() {
+struct d {
+  void test() {
+construct();
+  }
+  constexpr d(T b) : a(b) {}
+
+  T a;
+};
+  }
+
+  void g() { tester(); }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,10 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass &&
   CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+  template 
+  void construct() {
+T(0);
+  }
+
+  template 
+  void tester() {
+struct d {
+  void test() {
+construct();
+  }
+  constexpr d(T b) : a(b) {}
+
+  T a;
+};
+  }
+
+  void g() { tester(); }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,10 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass &&
   CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103595: [clang] Correct MarkFunctionReferenced for local class

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 349508.
futogergely added a comment.

clang-format


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

https://reviews.llvm.org/D103595

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/instantiate-local-class.cpp


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+template 
+void construct() {
+  T(0);
+}
+
+template 
+void tester() {
+  struct d {
+void test() {
+  construct();
+}
+constexpr d(T b) : a(b) {}
+
+T a;
+  };
+}
+
+void g() { tester(); }
+} // namespace PR48839
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,11 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
-  CodeSynthesisContexts.size())
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass && CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));
   else if (Func->isConstexpr())


Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -504,3 +504,24 @@
   }
   template void f();
 }
+
+namespace PR48839 {
+template 
+void construct() {
+  T(0);
+}
+
+template 
+void tester() {
+  struct d {
+void test() {
+  construct();
+}
+constexpr d(T b) : a(b) {}
+
+T a;
+  };
+}
+
+void g() { tester(); }
+} // namespace PR48839
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17057,11 +17057,12 @@
   PointOfInstantiation = Loc;
 }
 
+const bool isLocalClass =
+isa(Func->getDeclContext()) &&
+cast(Func->getDeclContext())->isLocalClass();
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
-Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
-  cast(Func->getDeclContext())->isLocalClass() &&
-  CodeSynthesisContexts.size())
+(!isLocalClass && Func->isConstexpr())) {
+  if (isLocalClass && CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));
   else if (Func->isConstexpr())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103595: [clang] Correct MarkFunctionReferenced for local class

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103595

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 390270.
futogergely retitled this revision from "[clang-tidy] CERT MSC24-C Obsolescent 
Functions check" to "[clang-tidy] Add cert-msc24-c checker.".
futogergely edited the summary of this revision.

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+
+typedef void *FILE;
+char *gets(char *s);
+void rewind(FILE *stream);
+void setbuf(FILE *stream, char *buf);
+
+void f1(char *s, FILE *f) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+
+  rewind(f);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+
+  setbuf(f, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:40: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:41: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+FILE *fopen(const char *filename, const char *mode);
+FILE *freopen(const char *filename, const char *mode, FILE *stream);
+int fscanf(FILE *stream, const char *format, ...);
+
+void f3(char *s, FILE *f) {
+  fopen(s, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fopen' has no exclusive access to file; 'fopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  freopen(s, s, f);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'freopen' has no exclusive access to file; 'freopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  int i;
+  fscanf(f, "%d", &i);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fscanf' is obsolescent; 'fscanf_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int time_t;
+char *ctime(const time_t *timer);
+
+void f4(const time_t *timer) {
+  ctime(timer);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'ctime' is non-reentrant; 'ctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int errno_t;
+typedef size_t rsize_t;
+errno_t asctime_s(char *s, rsize_t maxsize, const struct tm *timeptr);
+errno_t strcat_s(char *s1, rsize_t s1max, const char *s2);
+int fseek(FILE *stream, long int offset, int whence);
+int setvbuf(FILE *stream, char *buf, int mode, size_t size);
+
+void fUsingSafeFunctions(const struct tm *timeptr, FILE *f) {
+  const size_t BUFFSIZE = 32;
+  char buf[BUFFSIZE] = {0};
+
+  // no-warning, safe function from annex K is used
+  if (asctime_s(buf, BUFFSIZE, timeptr) != 0)
+return;
+
+  // no-warning, safe function from annex K is used
+  if (strcat_s(buf, BUFFSIZE, "something") != 0)
+return;
+
+  /

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 390309.
futogergely added a comment.

x64 debian failed


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
@@ -0,0 +1,97 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+
+typedef void *FILE;
+char *gets(char *s);
+void rewind(FILE *stream);
+void setbuf(FILE *stream, char *buf);
+
+void f1(char *s, FILE *f) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+
+  rewind(f);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+
+  setbuf(f, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:40: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:41: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+FILE *fopen(const char *filename, const char *mode);
+FILE *freopen(const char *filename, const char *mode, FILE *stream);
+int fscanf(FILE *stream, const char *format, ...);
+
+void f3(char *s, FILE *f) {
+  fopen(s, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fopen' has no exclusive access to file; 'fopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  freopen(s, s, f);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'freopen' has no exclusive access to file; 'freopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  int i;
+  fscanf(f, "%d", &i);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fscanf' is obsolescent; 'fscanf_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int time_t;
+char *ctime(const time_t *timer);
+
+void f4(const time_t *timer) {
+  ctime(timer);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'ctime' is non-reentrant; 'ctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int errno_t;
+typedef __SIZE_TYPE__ size_t;
+typedef size_t rsize_t;
+errno_t asctime_s(char *s, rsize_t maxsize, const struct tm *timeptr);
+errno_t strcat_s(char *s1, rsize_t s1max, const char *s2);
+int fseek(FILE *stream, long int offset, int whence);
+int setvbuf(FILE *stream, char *buf, int mode, size_t size);
+
+void fUsingSafeFunctions(const struct tm *timeptr, FILE *f) {
+  const size_t BUFFSIZE = 32;
+  char buf[BUFFSIZE] = {0};
+
+  // no-warning, safe function from annex K is used
+  if (asctime_s(buf, BUFFSIZE, timeptr) != 0)
+return;
+
+  // no-warning, safe function from annex K is used
+  if (strcat_s(buf, BUFFSIZE, "something") != 0)
+return;
+
+  // no-warning, fseeks supports error checking
+  if (fseek(f, 0, 0) != 0)
+return;
+
+  // no-warning, setvb

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 391604.

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c
@@ -0,0 +1,97 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+
+typedef void *FILE;
+char *gets(char *s);
+void rewind(FILE *stream);
+void setbuf(FILE *stream, char *buf);
+
+void f1(char *s, FILE *f) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'gets' is deprecated as of C99, removed from C11.
+
+  rewind(f);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead.
+
+  setbuf(f, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K::[[@LINE-1]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead.
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:40: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:41: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+FILE *fopen(const char *filename, const char *mode);
+FILE *freopen(const char *filename, const char *mode, FILE *stream);
+int fscanf(FILE *stream, const char *format, ...);
+
+void f3(char *s, FILE *f) {
+  fopen(s, s);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fopen' has no exclusive access to file; 'fopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  freopen(s, s, f);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'freopen' has no exclusive access to file; 'freopen_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+
+  int i;
+  fscanf(f, "%d", &i);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fscanf' is obsolescent; 'fscanf_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int time_t;
+char *ctime(const time_t *timer);
+
+void f4(const time_t *timer) {
+  ctime(timer);
+  // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'ctime' is non-reentrant; 'ctime_s' should be used instead.
+  // no-warning WITHOUT-ANNEX-K
+}
+
+typedef int errno_t;
+typedef __SIZE_TYPE__ size_t;
+typedef size_t rsize_t;
+errno_t asctime_s(char *s, rsize_t maxsize, const struct tm *timeptr);
+errno_t strcat_s(char *s1, rsize_t s1max, const char *s2);
+int fseek(FILE *stream, long int offset, int whence);
+int setvbuf(FILE *stream, char *buf, int mode, size_t size);
+
+void fUsingSafeFunctions(const struct tm *timeptr, FILE *f) {
+  const size_t BUFFSIZE = 32;
+  char buf[BUFFSIZE] = {0};
+
+  // no-warning, safe function from annex K is used
+  if (asctime_s(buf, BUFFSIZE, timeptr) != 0)
+return;
+
+  // no-warning, safe function from annex K is used
+  if (strcat_s(buf, BUFFSIZE, "something") != 0)
+return;
+
+  // no-warning, fseeks supports error checking
+  if (fseek(f, 0, 0) != 0)
+return;
+
+  // no-warning, setvbuf supports error checking
+  if (setvbuf(f, buf,

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely marked an inline comment as done.
futogergely added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:48
+  // Matching functions with safe replacements in annex K.
+  auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyName(
+  "::asctime", "::ctime", "::fopen", "::freopen", "::bsearch", "::fprintf",

whisperity wrote:
> Is this ordering specific in any way? Is the rule listing them in this order? 
> If not, can we have them listed alphabetically, for easier search and 
> potential later change?
done



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst:10
+For the listed functions, an alternative, more secure replacement is 
suggested, if available.
+The checker heavily relies on the functions from annex K (Bounds-checking 
interfaces) of C11.
+

whisperity wrote:
> (And consistent capitalisation later.)
done


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely marked an inline comment as done.
futogergely added a comment.

In D91000#3161296 , @whisperity wrote:

> Should/does this work in C++ mode for `std::whatever`?

Right now the checker finds the functions in the global namespace only. The 
recommendation is listed only in the C part of the CERT rules, and as far as I 
know, Annex K functions are defined in the global namespace only, (or at least 
based on the standard). I can't really decide if the checker should look for 
the functions in the std namespace as well or not...


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

Hi, sorry for the late answer, did not have time to check this in the last few 
weeks. I will try to address all of the remaining comments.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 485774.
futogergely marked 5 inline comments as done.
futogergely removed a reviewer: ktomi996.
futogergely added a comment.

Addressing review comments.


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -0,0 +1,155 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

In D91000#3862210 , @whisperity wrote:

> In D91000#3861942 , @aaron.ballman 
> wrote:
>
>> My concern with that approach was that we pay the full expense of doing the 
>> matches only get get into the `check()` function to bail out on all the 
>> Annex K functions, but now that there are replacements outside of Annex K, I 
>> don't see a way around paying that expense, so I think my concern has been 
>> addressed as well as it could have been.
>
> I think that Clang-Tidy checks are instantiated per AST. I will look into 
> whether we can somehow do the disabling of the check as early as possible! 
> (In that case, we could simply NOT register the matcher related to Annex-K 
> functions.) Either way, I'll do a rebase, re-run the tests and etc., and 
> likely take over the check.

I checked, and I think that at the point of ClangTidyCheck::registerMatchers 
the preprocessor has not been executed yet... (and we need the value of macros 
__STDC_LIB_EXT1__ and __STDC_WANT_LIB_EXT1__ to decide if we need to register 
some matchers or not) @whisperity could you maybe double-check it please?
What we could do is:

1. add a new checker option to decide if we suggest replacements from AnnexK. 
We could avoid registering matchers this way, but I don't really like this, 
having an option for something we could decide from the defined macros.
2. As a TODO, we could make possible to register checkers AFTER the 
preprocessor is executed. I have not looked into this, so I don't really know 
if it is possible at all in the current architecture.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

"L129 and L135 are uncovered by tests. The rest of the lines are covered by 
tests, according to lcov."
This happens if __STDC_WANT_LIB_EXT1__ is defined empty (L129) or 
__STDC_WANT_LIB_EXT1__ is not literal (numeric constant, ...).


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

"It seems like none of these projects actually use the annex K functions, which 
is not really a surprise.
VLC and lighttpd seems to use it. @futogergely could you please run your check 
on those projects?"

**lighttpd**: the checker issued 386 warnings. The reason is that 
__STDC_WANT_LIB_EXT1__ is defined in a header which is included everywhere. 
However, functions from Annex K are used only in one source file.
**VLC**: 2 warnings, usage of the rewind function.

Both of these projects can be compiled without Annex K as well, which makes the 
code cumbersome. Maybe this check can be really useful in a project where Annex 
K is a must.


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

Maybe we could remove the check for setbuf() and rewind() functions, making 
this a pure Annex K checker. There is an overlapping with another 
recommendation 
(https://wiki.sei.cmu.edu/confluence/display/c/ERR07-C.+Prefer+functions+that+support+error+checking+over+equivalent+functions+that+don%27t),
 these functions are also listed there.




Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:41-42
+
+  // Matching the `gets` deprecated function without replacement.
+  auto DeprecatedFunctionNamesMatcher = hasAnyName("::gets");
+

aaron.ballman wrote:
> This comment is not accurate. `gets_s()` is a secure replacement for `gets()`.
If gets is removed from C11, and gets_s is introduced in C11, then gets_s 
cannot be a replacement or? Maybe fgets?

Also I was wondering if we would like to disable this check for C99, maybe we 
should remove the check for gets all together.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:49-59
+  "::asctime", "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen",
+  "::fscanf", "::fwprintf", "::fwscanf", "::getenv", "::gmtime",
+  "::localtime", "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove",
+  "::printf", "::qsort", "::snprintf", "::sprintf", "::sscanf", "::strcat",
+  "::strcpy", "::strerror", "::strncat", "::strncpy", "::strtok",
+  "::swprintf", "::swscanf", "::vfprintf", "::vfscanf", "::vfwprintf",
+  "::vfwscanf", "::vprintf", "::vscanf", "::vsnprintf", "::vsprintf",

aaron.ballman wrote:
> This list appears to be missing quite a few functions with secure 
> replacements in Annex K. For example: `tmpfile_s`, `tmpnam_s`, 
> `strerrorlen_s`, `strlen_s`... can you check the list against the actual 
> Annex K, as it seems the CERT recommendation is still out of date.
Missing functions added: tmpfile/tmpfile_s, tmpnam/tmpnam_s, memset/memset_s, 
scanf, strlen, wcslen



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:84-86
+diag(Range.getBegin(),
+ "function '%0' is deprecated as of C99, removed from C11.")
+<< Deprecated->getName() << Range;

aaron.ballman wrote:
> Fixed a few nits with the code, but `gets()` was never deprecated, so the 
> message is not correct (it was present in C99 and removed in C11 with no 
> deprecation period). I think it may be better to say "function %0 was removed 
> in C11".
Done



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:103
+
+  diag(Range.getBegin(), "function '%0' %1; '%2' should be used instead.")
+  << FunctionName << getRationale(FunctionName)

aaron.ballman wrote:
> 
Done.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h:18
+
+/// Checks for deprecated and obsolescent function listed in
+/// CERT C Coding Standard Recommendation MSC24 - C. For the listed functions,

aaron.ballman wrote:
> The terminology used in the CERT recommendation is pretty unfortunate and I 
> don't think we should replicate it. Many of these are *not* deprecated or 
> obsolescent functions and calling them that will confuse users. The crux of 
> the CERT recommendation is that these functions have better replacements in 
> more modern versions of C. So I would probably try to focus our diagnostics 
> and documentation around modernization rather than deprecation.
> 
> FWIW, this is feedback that should also go onto the CERT recommendation 
> itself. I noticed someone already observed the same thing: 
> https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions?focusedCommentId=215482395#comment-215482395
Changed it to:
Checks for functions listed in CERT C Coding Standard Recommendation MSC24-C
that have safer, more secure replacements. The functions checked are considered 
unsafe because for
example they are missing bounds checking and/or non-reentrant. For the listed 
functions a
replacement function is suggested, if available. The checker heavily relies on 
the functions from
Annex K (Bounds-checking interfaces) of C11.

Also I changed the "is obsolescent" to "is not bounds-checking" in the 
getRationale function


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.

In D91000#3225369 , @balazske wrote:

> The functions `asctime` and `asctime_r` are discouraged according to CERT 
> MSC33-C rule. These could be added to this check as well. There is a clang SA 
> checker `SecuritySyntaxChecker` that contains other obsolete functions (and 
> the whole check looks like it can be done in clang-tidy).

The inclusion of CERT MSC33-C rule seems to be straightforward: check for 
asctime and asctime_r, and suggest asctime_s if Annex K is available, otherwise 
suggest strftime.

security.insecureAPI: the following functions could be added to the checker: 
bcmp, bcopy, bzero, getpw, mktemp, vfork, and if arc4random is available: 
drand48, erand48, jrand48, lcong48, lrand48, mrand48, nrand48, random, rand_r.
I think for now it is enough to issue a warning of using these functions, and 
not suggest a replacement. Should we add an option to the checker to also check 
for these functions?


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

https://reviews.llvm.org/D91000

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