Charusso updated this revision to Diff 140337.
Charusso marked 6 inline comments as done.
Charusso added a comment.
Truly fixed everything according to the comments.
https://reviews.llvm.org/D45050
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tidy/bugprone/NotNullTerminatedResultCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)alloca(wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)malloc(wcslen(src) * 2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+ std::wmemcpy(dest, src, std::wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+ std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+ wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+ wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'wmemchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+ dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+ wmemmove(dest, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemmove' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemmove_s(dest, wcslen(dest), src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_cxx11(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, wcslen(dest), src, (wcslen(src) + 1));
+}
+
+void bad_wmemmove_s(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, wcslen(dest), src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemmove_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemmove_s(dest, wcslen(dest), src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_s_1(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, wcslen(dest), src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_s_2(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, wcslen(dest), (src + 1), wcslen(src));
+}
+
+void bad_wmemset(wchar_t *dest, const wchar_t *src) {
+ wmemset(dest, '-', (wcslen(src) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemset' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemset(dest, '-', wcslen(src));
+}
+
+void good_wmemset(wchar_t *dest, const wchar_t *src) {
+ wmemset(dest, '-', wcslen(src));
+}
+
+int bad_wcsncmp(const wchar_t *str1, const wchar_t *str2) {
+ return wcsncmp(str1, str2, (wcslen(str1) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'wcsncmp' function's comparison's length is too long [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncmp(str1, str2, wcslen(str1));
+}
+
+int good_wcsncmp(const wchar_t *str1, const wchar_t *str2) {
+ return wcsncmp(str1, str2, wcslen(str1));
+}
+
+void bad_wcsxfrm(wchar_t *dest, const wchar_t *src) {
+ wcsxfrm(dest, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wcsxfrm' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsxfrm(dest, src, (wcslen(src) + 1));
+}
+
+void good_wcsxfrm(wchar_t *dest, const wchar_t *src) {
+ wcsxfrm(dest, src, (wcslen(src) + 1));
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++98
+
+typedef unsigned int size_t;
+size_t wcslen(const char *);
+char *wcsncpy(char *, const char *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+void *wmemmove(void *, const void *, size_t);
+
+void bad_wmemcpy(char *dest, const char *src) {
+ wmemcpy(dest, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncpy(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemcpy_not_cxx11(char *dest, const char *src) {
+ wcsncpy(dest, src, (wcslen(src) + 1));
+}
+
+void bad_wmemmove(char *dest, const char *src) {
+ wmemmove(dest, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemmove' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemmove(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_not_cxx11_1(char *dest, const char *src) {
+ wmemmove(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_not_cxx11_2(char *dest, const char *src) {
+ wmemmove(dest, (src + 1), wcslen(src));
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
@@ -0,0 +1,167 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t strlen(const char *);
+char *strerror(int);
+char *strchr(const char *, int);
+errno_t *strncpy_s(char *, const char *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *memcpy(void *, const void *, size_t);
+errno_t memcpy_s(void *, size_t, const void *, size_t);
+void *memchr(const void *, int, size_t);
+void *memmove(void *, const void *, size_t);
+errno_t memmove_s(void *, size_t, const void *, size_t);
+void *memset(void *, int, size_t);
+
+errno_t strerror_s(char *, size_t, int);
+int strncmp(const char *, const char *, size_t);
+size_t strxfrm(char *, const char *, size_t);
+
+namespace std {
+using ::memcpy;
+using ::strlen;
+using ::strncpy_s;
+} // namespace std
+
+void bad_alloca(char *dest, const char *src) {
+ dest = (char *)alloca(strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: alloca(strlen(src) + 1);
+}
+
+void good_alloca(char *dest, const char *src) {
+ dest = (char *)alloca(strlen(src) + 1);
+}
+
+void bad_calloc(char *dest, const char *src) {
+ dest = (char *)calloc(strlen(src), sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: calloc((strlen(src) + 1), sizeof(char));
+}
+
+void good_calloc(char *dest, const char *src) {
+ dest = (char *)calloc((strlen(src) + 1), sizeof(char));
+}
+
+void bad_malloc(char *dest, const char *src) {
+ dest = (char *)malloc(strlen(src) * 2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: malloc((strlen(src) * 2) + 1);
+}
+
+void good_malloc(char *dest, const char *src) {
+ dest = (char *)malloc((strlen(src) * 2) + 1);
+}
+
+void bad_realloc(char *dest, const char *src) {
+ dest = (char *)realloc(dest, (strlen(dest) + strlen(src)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: realloc(dest, (strlen(dest) + (strlen(src) + 1)));
+}
+
+void good_realloc(char *dest, const char *src) {
+ dest = (char *)realloc(dest, (strlen(dest) + (strlen(src) + 1)));
+}
+
+void bad_memcpy(char *dest, const char *src) {
+ std::memcpy(dest, src, std::strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: std::strncpy_s(dest, src, std::strlen(src));
+}
+
+void good_memcpy_cxx11(char *dest, const char *src) {
+ std::strncpy_s(dest, src, std::strlen(src));
+}
+
+void bad_memcpy_s(char *dest, const char *src) {
+ memcpy_s(dest, strlen(dest), src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncpy_s(dest, src, strlen(src));
+}
+
+void good_memcpy_s(char *dest, const char *src) {
+ strncpy_s(dest, src, strlen(src));
+}
+
+void bad_memchr(char *dest, const char *src) {
+ dest = (char *)memchr(src, '\0', strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'memchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strchr(src, '\0');
+}
+
+void good_memchr(char *dest, const char *src) {
+ dest = strchr(src, '\0');
+}
+
+void bad_memmove(char *dest, const char *src) {
+ memmove(dest, src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memmove' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memmove_s(dest, strlen(dest), src, (strlen(src) + 1));
+}
+
+void good_memmove_cxx11(char *dest, const char *src) {
+ memmove_s(dest, strlen(dest), src, (strlen(src) + 1));
+}
+
+void bad_memmove_s(char *dest, const char *src) {
+ memmove_s(dest, strlen(dest), src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memmove_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memmove_s(dest, strlen(dest), src, (strlen(src) + 1));
+}
+
+void good_memmove_s_1(char *dest, const char *src) {
+ memmove_s(dest, strlen(dest), src, (strlen(src) + 1));
+}
+
+void good_memmove_s_2(char *dest, const char *src) {
+ memmove_s(dest, strlen(dest), (src + 1), strlen(src));
+}
+
+void bad_memset(char *dest, const char *src) {
+ memset(dest, '-', (strlen(src) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memset(dest, '-', strlen(src));
+}
+
+void good_memset(char *dest, const char *src) {
+ memset(dest, '-', strlen(src));
+}
+
+void bad_strerror_s(char *dest, int errno) {
+ strerror_s(dest, strlen(strerror(errno)), errno);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'strerror_s' function's result is not null-terminated and missing the last character of the error message [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strerror_s(dest, (strlen(strerror(errno)) + 1), errno);
+}
+
+void good_strerror_s(char *dest, int errno) {
+ strerror_s(dest, (strlen(strerror(errno)) + 1), errno);
+}
+
+int bad_strncmp(char *str1, const char *str2) {
+ return strncmp(str1, str2, (strlen(str1) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'strncmp' function's comparison's length is too long [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str1, str2, strlen(str1));
+}
+
+int good_strncmp(const char *str1, const char *str2) {
+ return strncmp(str1, str2, strlen(str1));
+}
+
+void bad_strxfrm(char *long_destination_name, const char *long_source_name) {
+ strxfrm(long_destination_name, long_source_name,
+ strlen(long_source_name));
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'strxfrm' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strxfrm(long_destination_name, long_source_name,
+ // CHECK-FIXES-NEXT: strlen(long_source_name) + 1);
+}
+
+void good_strxfrm(char *long_destination_name, const char *long_source_name) {
+ strxfrm(long_destination_name, long_source_name,
+ (strlen(long_source_name) + 1));
+}
Index: test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++98
+
+typedef unsigned int size_t;
+size_t strlen(const char *);
+char *strncpy(char *, const char *, size_t);
+
+void *memcpy(void *, const void *, size_t);
+void *memmove(void *, const void *, size_t);
+
+void bad_memcpy(char *dest, const char *src) {
+ memcpy(dest, src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncpy(dest, src, (strlen(src) + 1));
+}
+
+void good_memcpy_not_cxx11(char *dest, const char *src) {
+ strncpy(dest, src, (strlen(src) + 1));
+}
+
+void bad_memmove(char *dest, const char *src) {
+ memmove(dest, src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memmove' function's result is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memmove(dest, src, (strlen(src) + 1));
+}
+
+void good_memmove_not_cxx11_1(char *dest, const char *src) {
+ memmove(dest, src, (strlen(src) + 1));
+}
+
+void good_memmove_not_cxx11_2(char *dest, const char *src) {
+ memmove(dest, (src + 1), strlen(src));
+}
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -36,6 +36,7 @@
bugprone-misplaced-widening-cast
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
+ bugprone-not-null-terminated-result
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
@@ -0,0 +1,94 @@
+.. title:: clang-tidy - bugprone-not-null-terminated-result
+
+bugprone-not-null-terminated-result
+===================================
+
+Finds function calls where ``strlen`` or ``wcslen`` function is passed as an
+argument and caused a not null-terminated result. Depending on the case of
+use, it may insert or remove the increase operation to ensure the result is
+null-terminated.
+
+The following function calls are checked:
+
+``alloca``, ``calloc``, ``malloc``, ``realloc``, ``memcpy``, ``wmemcpy``,
+``memcpy_s``, ``wmemcpy_s``, ``memchr``, ``wmemchr``, ``memmove``, ``wmemmove``,
+``memmove_s``, ``wmemmove_s``, ``memset``, ``wmemset``, ``strerror_s``,
+``strncmp``, ``wcsncmp``, ``strxfrm``, ``wcsxfrm``
+
+The following is a real-world example where the programmer forgot to increase
+the passed third argument, which is ``size_t length``. The proper length is
+``strlen(src) + 1`` because the null-terminator need an extra space. The result
+is badly not-null-terminated:
+
+.. code-block:: c
+
+ void bad_memcpy(char *dest, const char *src) {
+ memcpy(dest, src, strlen(src));
+ }
+
+In addition to issuing warnings, fix-it rewrites all the necessary code
+depending on the version number. The upper code would be the following:
+
+- If C++11 is the target fix-it will rewrite it to the most safe function:
+
+.. code-block:: c++
+
+ void good_memcpy_cxx11(char *dest, const char *src) {
+ strncpy_s(dest, src, strlen(src));
+ }
+
+- otherwise fix-it will rewrite it to a safer function, that born before C++11:
+
+.. code-block:: c
+
+ void good_memcpy_not_cxx11(char *dest, const char *src) {
+ strncpy(dest, src, (strlen(src) + 1));
+ }
+
+In general, the following transformations are could happen:
+
+**Memory allocation functions**
+
+- ``alloca`` function's argument gets a ``+ 1`` operation.
+
+- ``calloc`` function's first argument gets a ``+ 1`` operation.
+
+- ``malloc`` function's argument gets a ``+ 1`` operation.
+
+- ``realloc`` function's second argument gets a ``+ 1`` operation.
+
+**Memory handler functions**
+
+- ``memcpy``, ``wmemcpy``:
+ - C++11: New function is ``strncpy_s``/``wcsncpy_s``, where no ``+ 1`` needed.
+
+ - Not C++11: New function is ``strncpy``, where ``+ 1`` needed.
+
+- ``memchr``, ``wmemchr``:
+ - Usually there is a C-style cast, and it is needed to be removed, because the
+ new function ``strchr``/``wcschr``'s return type is correct.
+ - Also the third argument is not needed.
+
+- ``memmove``, ``wmemmove``:
+ - C++11: New function is ``memmmove_s``/``wmemmove_s``, it has four arguments,
+ - the new second argument is the first argument's length, and
+ - the third argument will be moved as the fourth, where ``+ 1`` needed.
+
+ - Not C++11: The third argument gets a ``+ 1`` operation.
+
+- ``memmove_s``/``wmemmove_s``:
+ - The function's fourth argument gets a ``+ 1`` operation.
+
+- ``memset``/``wmemset``:
+ - The function's third argument has to be truncated without the ``+ 1`` part.
+
+**Character functions**
+
+- ``strerror_s`` functions's second argument get a ``+ 1`` operation.
+
+- ``strncmp``/``wcsncmp``:
+ - If the third argument is the first or the second argument's ``length + 1``,
+ then it has to be truncated without the ``+ 1`` operation.
+
+- ``strxfrm``/``wcsxfrm`` function's third argument gets a ``+ 1`` operation.
+
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,126 +57,144 @@
Improvements to clang-tidy
--------------------------
+- New module `abseil` for checks related to the `Abseil <https://abseil.io>`_
+ library.
+
- New module ``portability``.
- New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
-- New `bugprone-throw-keyword-missing
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-throw-keyword-missing.html>`_ check
+- New :doc:`bugprone-not-null-terminated-result
+ <clang-tidy/checks/bugprone-not-null-terminated-result>` check
+
+ Finds function calls where ``strlen`` or ``wcslen`` function is passed as an
+ argument and caused a not null-terminated result. Depending on the case of
+ use, it may insert or remove the increase operation to ensure the result is
+ null-terminated.
+
+- New :doc:`bugprone-throw-keyword-missing
+ <clang-tidy/checks/bugprone-throw-keyword-missing>` check
Diagnoses when a temporary object that appears to be an exception is
constructed but not thrown.
-- New `cppcoreguidelines-avoid-goto
- <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_ check
+- New :doc:`bugprone-unused-return-value
+ <clang-tidy/checks/bugprone-unused-return-value>` check
+
+ Warns on unused function return values.
+
+- New :doc:`cppcoreguidelines-avoid-goto
+ <clang-tidy/checks/cppcoreguidelines-avoid-goto>` check
The usage of ``goto`` for control flow is error prone and should be replaced
with looping constructs. Every backward jump is rejected. Forward jumps are
only allowed in nested loops.
-- New `fuchsia-multiple-inheritance
- <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-multiple-inheritance.html>`_ check
+- New :doc:`fuchsia-multiple-inheritance
+ <clang-tidy/checks/fuchsia-multiple-inheritance>` check
Warns if a class inherits from multiple classes that are not pure virtual.
-- New `abseil` module for checks related to the `Abseil <https://abseil.io>`_
- library.
-
-- New `abseil-string-find-startswith
- <http://clang.llvm.org/extra/clang-tidy/checks/abseil-string-find-startswith.html>`_ check
+- New :doc:`abseil-string-find-startswith
+ <clang-tidy/checks/abseil-string-find-startswith>` check
Checks whether a ``std::string::find()`` result is compared with 0, and
suggests replacing with ``absl::StartsWith()``.
-- New `fuchsia-statically-constructed-objects
- <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-statically-constructed-objects.html>`_ check
+- New :doc:`fuchsia-statically-constructed-objects
+ <clang-tidy/checks/fuchsia-statically-constructed-objects>` check
Warns if global, non-trivial objects with static storage are constructed,
unless the object is statically initialized with a ``constexpr`` constructor
or has no explicit constructor.
-- New `fuchsia-trailing-return
- <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-trailing-return.html>`_ check
+- New :doc:`fuchsia-trailing-return
+ <clang-tidy/checks/fuchsia-trailing-return>` check
Functions that have trailing returns are disallowed, except for those
using ``decltype`` specifiers and lambda with otherwise unutterable
return types.
-- New `modernize-use-uncaught-exceptions
- <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-uncaught-exceptions.html>`_ check
+- New :doc:`hicpp-multiway-paths-covered
+ <clang-tidy/checks/hicpp-multiway-paths-covered>` check
+
+ Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
+
+- New :doc:`modernize-use-uncaught-exceptions
+ <clang-tidy/checks/modernize-use-uncaught-exceptions>` check
Finds and replaces deprecated uses of ``std::uncaught_exception`` to
``std::uncaught_exceptions``.
-- New `portability-simd-intrinsics
- <http://clang.llvm.org/extra/clang-tidy/checks/portability-simd-intrinsics.html>`_ check
+- New :doc:`portability-simd-intrinsics
+ <clang-tidy/checks/portability-simd-intrinsics>` check
Warns or suggests alternatives if SIMD intrinsics are used which can be replaced by
``std::experimental::simd`` operations.
-- New `zircon-temporary-objects
- <http://clang.llvm.org/extra/clang-tidy/checks/zircon-temporary-objects.html>`_ check
+- New :doc:`zircon-temporary-objects
+ <clang-tidy/checks/zircon-temporary-objects>` check
Warns on construction of specific temporary objects in the Zircon kernel.
-- New alias `hicpp-avoid-goto
- <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to
- `cppcoreguidelines-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_
+- New alias :doc:`hicpp-avoid-goto
+ <clang-tidy/checks/hicpp-avoid-goto>` to :doc:`cppcoreguidelines-avoid-goto
+ <clang-tidy/checks/cppcoreguidelines-avoid-goto>`
added.
-- The 'misc-forwarding-reference-overload' check was renamed to `bugprone-forwarding-reference-overload
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-forwarding-reference-overload.html>`_
+- The 'misc-forwarding-reference-overload' check was renamed to :doc:`bugprone-forwarding-reference-overload
+ <clang-tidy/checks/bugprone-forwarding-reference-overload>`
-- The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-incorrect-roundings.html>`_
+- The 'misc-incorrect-roundings' check was renamed to :doc:`bugprone-incorrect-roundings
+ <clang-tidy/checks/bugprone-incorrect-roundings>`
-- The 'misc-lambda-function-name' check was renamed to `bugprone-lambda-function-name
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-lambda-function-name.html>`_
+- The 'misc-lambda-function-name' check was renamed to :doc:`bugprone-lambda-function-name
+ <clang-tidy/checks/bugprone-lambda-function-name>`
-- The 'misc-macro-parentheses' check was renamed to `bugprone-macro-parentheses
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-macro-parentheses.html>`_
+- The 'misc-macro-parentheses' check was renamed to :doc:`bugprone-macro-parentheses
+ <clang-tidy/checks/bugprone-macro-parentheses>`
-- The 'misc-macro-repeated-side-effects' check was renamed to `bugprone-macro-repeated-side-effects
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-macro-repeated-side-effects.html>`_
+- The 'misc-macro-repeated-side-effects' check was renamed to :doc:`bugprone-macro-repeated-side-effects
+ <clang-tidy/checks/bugprone-macro-repeated-side-effects>`
-- The 'misc-misplaced-widening-cast' check was renamed to `bugprone-misplaced-widening-cast
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-widening-cast.html>`_
+- The 'misc-misplaced-widening-cast' check was renamed to :doc:`bugprone-misplaced-widening-cast
+ <clang-tidy/checks/bugprone-misplaced-widening-cast>`
-- The 'misc-sizeof-container' check was renamed to `bugprone-sizeof-container
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-container.html>`_
+- The 'misc-sizeof-container' check was renamed to :doc:`bugprone-sizeof-container
+ <clang-tidy/checks/bugprone-sizeof-container>`
-- The 'misc-sizeof-expression' check was renamed to `bugprone-sizeof-expression
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html>`_
+- The 'misc-sizeof-expression' check was renamed to :doc:`bugprone-sizeof-expression
+ <clang-tidy/checks/bugprone-sizeof-expression>`
-- The 'misc-string-compare' check was renamed to `readability-string-compare
- <http://clang.llvm.org/extra/clang-tidy/checks/readability-string-compare.html>`_
+- The 'misc-string-compare' check was renamed to :doc:`readability-string-compare
+ <clang-tidy/checks/readability-string-compare>`
-- The 'misc-string-integer-assignment' check was renamed to `bugprone-string-integer-assignment
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-string-integer-assignment.html>`_
+- The 'misc-string-integer-assignment' check was renamed to :doc:`bugprone-string-integer-assignment
+ <clang-tidy/checks/bugprone-string-integer-assignment>`
-- The 'misc-string-literal-with-embedded-nul' check was renamed to `bugprone-string-literal-with-embedded-nul
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-string-literal-with-embedded-nul.html>`_
+- The 'misc-string-literal-with-embedded-nul' check was renamed to :doc:`bugprone-string-literal-with-embedded-nul
+ <clang-tidy/checks/bugprone-string-literal-with-embedded-nul>`
-- The 'misc-suspicious-enum-usage' check was renamed to `bugprone-suspicious-enum-usage
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-enum-usage.html>`_
+- The 'misc-suspicious-enum-usage' check was renamed to :doc:`bugprone-suspicious-enum-usage
+ <clang-tidy/checks/bugprone-suspicious-enum-usage>`
-- The 'misc-suspicious-missing-comma' check was renamed to `bugprone-suspicious-missing-comma
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-missing-comma.html>`_
+- The 'misc-suspicious-missing-comma' check was renamed to :doc:`bugprone-suspicious-missing-comma
+ <clang-tidy/checks/bugprone-suspicious-missing-comma>`
-- The 'misc-suspicious-semicolon' check was renamed to `bugprone-suspicious-semicolon
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-semicolon.html>`_
+- The 'misc-suspicious-semicolon' check was renamed to :doc:`bugprone-suspicious-semicolon
+ <clang-tidy/checks/bugprone-suspicious-semicolon>`
-- The 'misc-suspicious-string-compare' check was renamed to `bugprone-suspicious-string-compare
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html>`_
+- The 'misc-suspicious-string-compare' check was renamed to :doc:`bugprone-suspicious-string-compare
+ <clang-tidy/checks/bugprone-suspicious-string-compare>`
-- The 'misc-swapped-arguments' check was renamed to `bugprone-swapped-arguments
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-swapped-arguments.html>`_
+- The 'misc-swapped-arguments' check was renamed to :doc:`bugprone-swapped-arguments
+ <clang-tidy/checks/bugprone-swapped-arguments>`
-- The 'misc-undelegated-constructor' check was renamed to `bugprone-undelegated-constructor
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-undelegated-constructor.html>`_
+- The 'misc-undelegated-constructor' check was renamed to :doc:`bugprone-undelegated-constructor
+ <clang-tidy/checks/bugprone-undelegated-constructor>`
-- The 'misc-unused-raii' check was renamed to `bugprone-unused-raii
- <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unused-raii.html>`_
+- The 'misc-unused-raii' check was renamed to :doc:`bugprone-unused-raii
+ <clang-tidy/checks/bugprone-unused-raii>`
Improvements to include-fixer
-----------------------------
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -0,0 +1,77 @@
+//===--------------- NotNullTerminatedResultCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOT_NULL_TERMINATED_RESULT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOT_NULL_TERMINATED_RESULT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds function calls where ``strlen`` or ``wcslen`` function is passed as an
+/// argument and caused a not null-terminated result. Depending on the case of
+/// use, it may insert or remove the increase operation to ensure the result is
+/// null-terminated.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-not-null-terminated-result.html
+class NotNullTerminatedResultCheck : public ClangTidyCheck {
+public:
+ NotNullTerminatedResultCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void memAllocFuncFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void memHandlerFuncFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void memcpyFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void memcpy_sFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void memchrFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void memmoveFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void memmove_sFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void characterFuncFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void strerror_sFix(const ast_matchers::MatchFinder::MatchResult &Result);
+ void ncmpFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void xfrmFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void
+ lengthArgInsertIncByOne(const int ArgPos,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void
+ lengthArgRemoveIncByOne(const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void removeArg(const int ArgPos,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void renameFunc(StringRef NewFuncName,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOT_NULL_TERMINATED_RESULT_H
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -0,0 +1,435 @@
+//===------------- NotNullTerminatedResultCheck.cpp - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NotNullTerminatedResultCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static std::string exprToStr(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Expr->getSourceRange()),
+ *Result.SourceManager, Result.Context->getLangOpts(), 0);
+}
+
+static SourceLocation exprLocEnd(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getLocForEndOfToken(Expr->getLocEnd(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+}
+
+static bool isInsideParentheses(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {
+ auto NewExpr = const_cast<clang::Expr *>(Expr);
+ ParenExpr *PE = new (Result.Context)
+ ParenExpr(NewExpr->getLocStart().getLocWithOffset(-1),
+ NewExpr->getLocEnd().getLocWithOffset(1), NewExpr);
+ return Expr == PE->getSubExpr();
+}
+
+/// Sometimes the 'memmove' function is in use to trim the text until it matches
+/// to a specific condition. For example a .CSV file's line would be trimmed
+/// from ' "data_name", "data", ' to just 'data' by "memmoving" it char-by-char.
+static bool isTrimFunc(const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const int ArgPos = FuncExpr->getNumArgs() - 2;
+
+ // This is the following structure: (dest, (src + 1), strlen(src))
+ // LHSStr: ~~~
+ // RHSStr: ~
+ // StrLenArgStr: ~~~
+ if (const auto *BinOp = dyn_cast<BinaryOperator>(
+ FuncExpr->getArg(ArgPos)->IgnoreParenCasts())) {
+ std::string LHSStr = exprToStr(BinOp->getLHS()->IgnoreParens(), Result);
+ std::string RHSStr = exprToStr(BinOp->getRHS()->IgnoreParens(), Result);
+ std::string StrLenArgStr = exprToStr(
+ Result.Nodes.getNodeAs<CallExpr>("length-call")->getArg(0), Result);
+
+ return StrLenArgStr == LHSStr || StrLenArgStr == RHSStr;
+ }
+ return false;
+}
+
+void NotNullTerminatedResultCheck::registerMatchers(MatchFinder *Finder) {
+ // The following function's expression binding is necessary because some of
+ // the functions create double binding on the same function call.
+ const auto LengthCall =
+ callExpr(callee(functionDecl(hasAnyName("strlen", "wcslen"))),
+ expr().bind("length-call"));
+
+ const auto IsNotPlusOperator = unless(hasOperatorName("+"));
+
+ const auto HasIntOperand =
+ hasEitherOperand(ignoringParenImpCasts(integerLiteral()));
+
+ const auto HasInc = binaryOperator(hasOperatorName("+"), HasIntOperand);
+
+ // The following 3 case match 'strlen' or 'wcslen' function calls where no
+ // increase by one operation found.
+ //- Simple case: strlen(src) or wcslen(src)
+ const auto SimpleWithoutInc = LengthCall.bind("length-wout-inc-simp");
+
+ //- Complex case: (strlen(src) * 2) or (wcslen(src) * 2)
+ const auto ComplexWithoutInc =
+ binaryOperator(IsNotPlusOperator, hasEitherOperand(LengthCall))
+ .bind("length-wout-inc-comp");
+
+ //- Complex case: (strlen(dest) + strlen(src)) or (wcslen(dest) + wcslen(src))
+ const auto ComplexTwoWithoutInc =
+ allOf(binaryOperator(hasOperatorName("+"), unless(HasIntOperand)),
+ binaryOperator(hasOperatorName("+"), hasLHS(LengthCall),
+ hasRHS(LengthCall))
+ .bind("length-wout-inc-comp"));
+
+ const auto LengthWithoutInc =
+ anyOf(SimpleWithoutInc, ComplexWithoutInc, ComplexTwoWithoutInc);
+
+ // The following 2 case match 'strlen' or 'wcslen' function calls where
+ // increase by one operation found.
+ //- Simple case: (strlen(src) + 1) or (wcslen(src) + 1)
+ const auto SimpleWithInc =
+ allOf(HasInc,
+ binaryOperator(hasEitherOperand(LengthCall.bind("inner-operator")))
+ .bind("outer-operator"));
+
+ //- Complex case: ((strlen(src) / 2) + 1) or ((wcslen(src) / 2) + 1)
+ const auto ComplexWithInc = allOf(
+ HasInc, binaryOperator(has(binaryOperator(unless(hasOperatorName("+")),
+ hasEitherOperand(LengthCall))
+ .bind("inner-operator")))
+ .bind("outer-operator"));
+
+ const auto LengthWithInc = anyOf(SimpleWithInc, ComplexWithInc);
+
+ enum StrLenKind { WithInc, WithoutInc };
+
+ const auto Matcher = [=](StringRef Name, const int ArgCount, const int ArgPos,
+ StrLenKind Kind) {
+ if (Kind == StrLenKind::WithoutInc) {
+ return allOf(callee(functionDecl(hasName(Name))),
+ argumentCountIs(ArgCount),
+ hasArgument(ArgPos, LengthWithoutInc));
+ } else {
+ return allOf(callee(functionDecl(hasName(Name))),
+ argumentCountIs(ArgCount),
+ hasArgument(ArgPos, LengthWithInc));
+ }
+ };
+
+ // clang-format off
+ const auto Alloca = Matcher("alloca", 1, 0, StrLenKind::WithoutInc);
+ const auto Calloc = Matcher("calloc", 2, 0, StrLenKind::WithoutInc);
+ const auto Malloc = Matcher("malloc", 1, 0, StrLenKind::WithoutInc);
+ const auto Realloc = Matcher("realloc", 2, 1, StrLenKind::WithoutInc);
+ const auto Memcpy = Matcher("memcpy", 3, 2, StrLenKind::WithoutInc);
+ const auto Wmemcpy = Matcher("wmemcpy", 3, 2, StrLenKind::WithoutInc);
+ const auto Memcpy_s = Matcher("memcpy_s", 4, 3, StrLenKind::WithoutInc);
+ const auto Wmemcpy_s = Matcher("wmemcpy_s", 4, 3, StrLenKind::WithoutInc);
+ const auto Memchr = Matcher("memchr", 3, 2, StrLenKind::WithoutInc);
+ const auto Wmemchr = Matcher("wmemchr", 3, 2, StrLenKind::WithoutInc);
+ const auto Memmove = Matcher("memmove", 3, 2, StrLenKind::WithoutInc);
+ const auto Wmemmove = Matcher("wmemmove", 3, 2, StrLenKind::WithoutInc);
+ const auto Memmove_s = Matcher("memmove_s", 4, 3, StrLenKind::WithoutInc);
+ const auto Wmemmove_s = Matcher("wmemmove_s", 4, 3, StrLenKind::WithoutInc);
+ const auto Memset = Matcher("memset", 3, 2, StrLenKind::WithInc);
+ const auto Wmemset = Matcher("wmemset", 3, 2, StrLenKind::WithInc);
+ const auto Strerror_s = Matcher("strerror_s", 3, 1, StrLenKind::WithoutInc);
+ const auto Strncmp = Matcher("strncmp", 3, 2, StrLenKind::WithInc);
+ const auto Wcsncmp = Matcher("wcsncmp", 3, 2, StrLenKind::WithInc);
+ const auto Strxfrm = Matcher("strxfrm", 3, 2, StrLenKind::WithoutInc);
+ const auto Wcsxfrm = Matcher("wcsxfrm", 3, 2, StrLenKind::WithoutInc);
+ // clang-format on
+
+ const auto AnyOfExpressions = anyOf(
+ Alloca, Calloc, Malloc, Realloc, Memcpy, Wmemcpy, Memcpy_s, Wmemcpy_s,
+ Memchr, Wmemchr, Memmove, Wmemmove, Memmove_s, Wmemmove_s, Memset,
+ Wmemset, Strerror_s, Strncmp, Wcsncmp, Strxfrm, Wcsxfrm);
+
+ Finder->addMatcher(callExpr(AnyOfExpressions).bind("expr"), this);
+ Finder->addMatcher(
+ binaryOperator(
+ has(cStyleCastExpr(has(callExpr(anyOf(Memchr, Wmemchr)).bind("expr")))
+ .bind("cast"))),
+ this);
+}
+
+void NotNullTerminatedResultCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+
+ if (FuncExpr->getLocStart().isMacroID())
+ return;
+
+ if (!FuncExpr->getDirectCallee())
+ return;
+
+ StringRef Name = FuncExpr->getDirectCallee()->getName();
+
+ if (Name.contains("alloc"))
+ memAllocFuncFix(Name, Result);
+ else if (Name.startswith("mem") || Name.startswith("wmem"))
+ memHandlerFuncFix(Name, Result);
+ else if (Name.startswith("str") || Name.startswith("wcs"))
+ characterFuncFix(Name, Result);
+}
+
+void NotNullTerminatedResultCheck::memAllocFuncFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "'%0' function's allocated memory cannot hold the null-terminator")
+ << Name;
+
+ if (Name == "alloca" || Name == "calloc" || Name == "malloc")
+ lengthArgInsertIncByOne(0, Result, Diag);
+ else if (Name == "realloc")
+ lengthArgInsertIncByOne(1, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memHandlerFuncFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (Name == "memmove" || Name == "wmemmove")
+ memmoveFix(Name, Result);
+ else if (Name == "memmove_s" || Name == "wmemmove_s")
+ memmove_sFix(Name, Result);
+ else {
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "'%0' function's result is not null-terminated")
+ << Name;
+ if (Name == "memcpy" || Name == "wmemcpy")
+ memcpyFix(Name, Result, Diag);
+ else if (Name == "memcpy_s" || Name == "wmemcpy_s")
+ memcpy_sFix(Name, Result, Diag);
+ else if (Name == "memchr" || Name == "wmemchr")
+ memchrFix(Name, Result, Diag);
+ else if (Name == "memset" || Name == "wmemset")
+ lengthArgRemoveIncByOne(Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::memcpyFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ if (getLangOpts().CPlusPlus11) {
+ StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";
+ renameFunc(NewFuncName, Result, Diag);
+ } else {
+ StringRef NewFuncName = (Name[0] != 'w') ? "strncpy" : "wcsncpy";
+ renameFunc(NewFuncName, Result, Diag);
+ lengthArgInsertIncByOne(2, Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::memcpy_sFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";
+ renameFunc(NewFuncName, Result, Diag);
+ removeArg(1, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memchrFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ if (const auto *CastExpr = Result.Nodes.getNodeAs<CStyleCastExpr>("cast")) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const auto CastRemoveFix = FixItHint::CreateRemoval(SourceRange(
+ CastExpr->getLocStart(), FuncExpr->getLocStart().getLocWithOffset(-1)));
+ Diag << CastRemoveFix;
+ }
+ StringRef NewFuncName = (Name[0] != 'w') ? "strchr" : "wcschr";
+ renameFunc(NewFuncName, Result, Diag);
+ removeArg(2, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memmoveFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (isTrimFunc(Result))
+ return;
+
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ auto Diag = diag(FuncExpr->getLocStart(),
+ "'%0' function's result is not null-terminated")
+ << Name;
+
+ if (getLangOpts().CPlusPlus11) {
+ StringRef NewFuncName = (Name[0] != 'w') ? "memmove_s" : "wmemmove_s";
+ renameFunc(NewFuncName, Result, Diag);
+
+ const auto FirstArg = FuncExpr->getArg(0);
+ std::string NewSecondArg = " strlen(" + exprToStr(FirstArg, Result) + "),";
+
+ const auto InsertNewArgFix = FixItHint::CreateInsertion(
+ exprLocEnd(FirstArg, Result).getLocWithOffset(1), NewSecondArg);
+ Diag << InsertNewArgFix;
+ }
+ lengthArgInsertIncByOne(2, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memmove_sFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (isTrimFunc(Result))
+ return;
+
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "'%0' function's result is not null-terminated")
+ << Name;
+ lengthArgInsertIncByOne(3, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::characterFuncFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (Name == "strerror_s")
+ strerror_sFix(Result);
+ else if (Name == "strncmp" || Name == "wcsncmp")
+ ncmpFix(Name, Result);
+ else if (Name == "strxfrm" || Name == "wcsxfrm")
+ xfrmFix(Name, Result);
+}
+
+void NotNullTerminatedResultCheck::strerror_sFix(
+ const MatchFinder::MatchResult &Result) {
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "'strerror_s' function's result is not null-terminated and "
+ "missing the last character of the error message");
+ lengthArgInsertIncByOne(1, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::ncmpFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ std::string FirstArg = exprToStr(FuncExpr->getArg(0), Result);
+ std::string SecondArg = exprToStr(FuncExpr->getArg(1), Result);
+ std::string StrLenArg = exprToStr(
+ Result.Nodes.getNodeAs<CallExpr>("length-call")->getArg(0), Result);
+
+ if (StrLenArg == FirstArg || StrLenArg == SecondArg) {
+ auto Diag = diag(FuncExpr->getLocStart(),
+ "'%0' function's comparison's length is too long")
+ << Name;
+ lengthArgRemoveIncByOne(Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::xfrmFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "'%0' function's result is not null-terminated")
+ << Name;
+ lengthArgInsertIncByOne(2, Result, Diag);
+}
+
+/// FIXME: It is probably a false-positive factory because of the complex cases.
+void NotNullTerminatedResultCheck::lengthArgInsertIncByOne(
+ const int ArgPos, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ bool isComplexCase = false;
+ const Expr *StrLenExpr = nullptr;
+ if (const auto *StrLen = Result.Nodes.getNodeAs<Expr>("length-wout-inc-simp"))
+ StrLenExpr = StrLen;
+ else {
+ StrLenExpr = Result.Nodes.getNodeAs<Expr>("length-wout-inc-comp");
+ isComplexCase = true;
+ }
+
+ bool NeedInnerParen = false, NeedOuterParen = false;
+ if (isComplexCase) {
+ if (const auto *BinOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("length-wout-inc-comp")) {
+ NeedInnerParen = BinOp->getOpcode() != BO_Add;
+ }
+ }
+ if (!isInsideParentheses(StrLenExpr, Result)) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ NeedOuterParen = FuncExpr->getNumArgs() > 1;
+ }
+
+ std::string LHSPartStr = NeedOuterParen ? "(" : "";
+ LHSPartStr += NeedInnerParen ? "(" : "";
+ std::string RHSPartStr = NeedInnerParen ? ")" : "";
+ RHSPartStr += " + 1";
+ RHSPartStr += NeedOuterParen ? ")" : "";
+
+ const auto InsertFirstParenFix =
+ FixItHint::CreateInsertion(StrLenExpr->getLocStart(), LHSPartStr);
+ const auto InsertPlusOneAndSecondParenFix = FixItHint::CreateInsertion(
+ StrLenExpr->getLocEnd().getLocWithOffset(1), RHSPartStr);
+ Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+}
+
+void NotNullTerminatedResultCheck::lengthArgRemoveIncByOne(
+ const MatchFinder::MatchResult &Result, DiagnosticBuilder &Diag) {
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // InnerOperatorExpr: ~~~~~~~~~~~~^~~
+ // OuterOperatorExpr: ~~~~~~~~~~~~~~~~~~^~~
+ const auto InnerOperatorExpr = Result.Nodes.getNodeAs<Expr>("inner-operator");
+ const auto OuterOperatorExpr = Result.Nodes.getNodeAs<Expr>("outer-operator");
+ int OuterOperatorOffset = 0;
+
+ if (isInsideParentheses(OuterOperatorExpr, Result))
+ ++OuterOperatorOffset;
+
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // LHSRemoveRange: ~~
+ // RHSRemoveRange: ~~~~~~
+ const auto LHSRemoveRange = SourceRange(
+ OuterOperatorExpr->getLocStart().getLocWithOffset(-OuterOperatorOffset),
+ InnerOperatorExpr->getLocStart().getLocWithOffset(-1));
+
+ const auto RHSRemoveRange = SourceRange(
+ InnerOperatorExpr->getLocEnd().getLocWithOffset(1),
+ OuterOperatorExpr->getLocEnd().getLocWithOffset(OuterOperatorOffset));
+
+ const auto LHSRemoveFix = FixItHint::CreateRemoval(LHSRemoveRange);
+ const auto RHSRemoveFix = FixItHint::CreateRemoval(RHSRemoveRange);
+ Diag << LHSRemoveFix << RHSRemoveFix;
+}
+
+void NotNullTerminatedResultCheck::removeArg(
+ const int ArgPos, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ // This is the following structure: (src, '\0', strlen(src))
+ // ArgToRemove: ~~~~~~~~~~~
+ // LHSArg: ~~~~
+ // RemoveArgFix: ~~~~~~~~~~~~~
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const auto ArgToRemove = FuncExpr->getArg(ArgPos);
+ const auto LHSArg = FuncExpr->getArg(ArgPos - 1);
+ const auto RemoveArgFix = FixItHint::CreateRemoval(
+ SourceRange(exprLocEnd(LHSArg, Result),
+ exprLocEnd(ArgToRemove, Result).getLocWithOffset(-1)));
+ Diag << RemoveArgFix;
+}
+
+void NotNullTerminatedResultCheck::renameFunc(
+ StringRef NewFuncName, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const auto FuncNameLength =
+ FuncExpr->getDirectCallee()->getIdentifier()->getLength();
+ const auto FirstArgLoc = FuncExpr->getArg(0)->getExprLoc();
+ const auto FuncNameRange = CharSourceRange::getCharRange(
+ FirstArgLoc.getLocWithOffset(-FuncNameLength - 1),
+ FirstArgLoc.getLocWithOffset(-1));
+
+ const auto FuncNameFix =
+ FixItHint::CreateReplacement(FuncNameRange, NewFuncName);
+ Diag << FuncNameFix;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tidy/bugprone/CMakeLists.txt
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -20,6 +20,7 @@
MisplacedWideningCastCheck.cpp
MoveForwardingReferenceCheck.cpp
MultipleStatementMacroCheck.cpp
+ NotNullTerminatedResultCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StringConstructorCheck.cpp
Index: clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -28,6 +28,7 @@
#include "MisplacedWideningCastCheck.h"
#include "MoveForwardingReferenceCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "NotNullTerminatedResultCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
#include "StringConstructorCheck.h"
@@ -89,6 +90,8 @@
"bugprone-move-forwarding-reference");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
+ "bugprone-not-null-terminated-result");
CheckFactories.registerCheck<SizeofContainerCheck>(
"bugprone-sizeof-container");
CheckFactories.registerCheck<SizeofExpressionCheck>(
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
