Charusso updated this revision to Diff 143825.
Charusso marked an inline comment as done.
Charusso added a comment.
Defined an option to switch off the `_s` suffixed functions usage.
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-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_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);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+ dest = (wchar_t *)alloca(wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to 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: memory allocated by 'calloc' is insufficient to 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: memory allocated by 'malloc' is insufficient to 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: memory allocated by 'realloc' is insufficient to 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) {
+ wmemcpy(dest, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+ wcsncpy_s(dest, src, 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: the result from calling 'wmemcpy_s' 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: the result from calling 'wmemchr' 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: the result from calling 'wmemmove' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemmove_s(dest, wcslen(dest), src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_safe(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: the result from calling 'wmemmove_s' 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: the result from calling 'wmemset' 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]]:38: warning: comparison length is too long and might lead to a buffer overflow [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: the result from calling 'wcsxfrm' 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-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: IsSafeFunctionsAreAvailable, value: 0}]}" \
+// RUN: -- -std=c99
+
+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: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncpy(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemcpy_before_safe(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: the result from calling 'wmemmove' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wmemmove(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_before_safe_1(char *dest, const char *src) {
+ wmemmove(dest, src, (wcslen(src) + 1));
+}
+
+void good_wmemmove_before_safe_2(char *dest, const char *src) {
+ wmemmove(dest, (src + 1), wcslen(src));
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
@@ -0,0 +1,163 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+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);
+
+
+void bad_alloca(char *dest, const char *src) {
+ dest = (char *)alloca(strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: memory allocated by 'alloca' is insufficient to 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: memory allocated by 'calloc' is insufficient to 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: memory allocated by 'malloc' is insufficient to 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: memory allocated by 'realloc' is insufficient to 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) {
+ memcpy(dest, src, strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncpy_s(dest, src, strlen(src));
+}
+
+void good_memcpy_safe(char *dest, const char *src) {
+ strncpy_s(dest, src, 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: the result from calling 'memcpy_s' 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: the result from calling 'memchr' 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: the result from calling 'memmove' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memmove_s(dest, strlen(dest), src, (strlen(src) + 1));
+}
+
+void good_memmove_safe(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: the result from calling 'memmove_s' 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: the result from calling 'memset' 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: the result from calling 'strerror_s' 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]]:38: warning: comparison length is too long and might lead to a buffer overflow [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: the result from calling 'strxfrm' 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-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: IsSafeFunctionsAreAvailable, value: 0}]}" \
+// RUN: -- -std=c99
+
+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: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncpy(dest, src, (strlen(src) + 1));
+}
+
+void good_memcpy_before_safe(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: the result from calling 'memmove' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memmove(dest, src, (strlen(src) + 1));
+}
+
+void good_memmove_before_safe_1(char *dest, const char *src) {
+ memmove(dest, src, (strlen(src) + 1));
+}
+
+void good_memmove_before_safe_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-parent-virtual-call
bugprone-sizeof-container
bugprone-sizeof-expression
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,107 @@
+.. 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
+according to the given option. If the target version implements ``_s`` suffixed
+functions fix-it will rewrite it to the most safe function:
+
+.. code-block:: c
+
+ void good_memcpy_with_safer_functions(char *dest, const char *src) {
+ strncpy_s(dest, src, strlen(src));
+ }
+
+Otherwise fix-it will rewrite it to a safer function, that born before ``_s``
+suffixed functions:
+
+.. code-block:: c
+
+ void good_memcpy_before_safer_functions(char *dest, const char *src) {
+ strncpy(dest, src, (strlen(src) + 1));
+ }
+
+Transformations
+---------------
+
+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``:
+ - If ``_s`` functions are available: New function is
+ ``strncpy_s``/``wcsncpy_s``, where no ``+ 1`` needed.
+
+ - Else: 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``:
+ - If ``_s`` functions are available: New function is
+ ``memmove_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.
+
+ - Else: 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.
+
+Options
+-------
+
+.. option:: IsSafeFunctionsAreAvailable
+
+ An integer non-zero value specifying if the target version implements ``_s``
+ suffixed memory and character handler functions which is safer than older
+ versions (e.g. ``memcpy_s()``). The default value is ``1``.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,14 @@
- New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
+- 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-parent-virtual-call
<clang-tidy/checks/bugprone-parent-virtual-call>` check
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -0,0 +1,80 @@
+//===--------------- 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);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ // If non-zero the target version implements _s suffixed memory and character
+ // handler functions which is safer than older versions (e.g. 'memcpy_s()').
+ const int IsSafeFunctionsAreAvailable;
+
+ 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,
+ DiagnosticBuilder &Diag);
+ 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(int ArgPos,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void
+ lengthArgRemoveIncByOne(const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ void removeArg(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,416 @@
+//===------------- 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 StringRef exprToStr(const Expr *E,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getSourceText(
+ CharSourceRange::getTokenRange(E->getSourceRange()),
+ *Result.SourceManager, Result.Context->getLangOpts(), 0);
+}
+
+static SourceLocation exprLocEnd(const Expr *E,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getLocForEndOfToken(E->getLocEnd(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+}
+
+/// 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 isTrimMemmove(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: ~
+ // LengthExprArgStr: ~~~
+ if (const auto *BinOp = dyn_cast<BinaryOperator>(
+ FuncExpr->getArg(ArgPos)->IgnoreParenCasts())) {
+ StringRef LHSStr = exprToStr(BinOp->getLHS()->IgnoreParens(), Result);
+ StringRef RHSStr = exprToStr(BinOp->getRHS()->IgnoreParens(), Result);
+ StringRef LengthExprArgStr = exprToStr(
+ Result.Nodes.getNodeAs<CallExpr>("length-call")->getArg(0), Result);
+
+ return LengthExprArgStr == LHSStr || LengthExprArgStr == RHSStr;
+ }
+ return false;
+}
+
+NotNullTerminatedResultCheck::NotNullTerminatedResultCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), IsSafeFunctionsAreAvailable(Options.get(
+ "IsSafeFunctionsAreAvailable", 1)) {}
+
+void NotNullTerminatedResultCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IsSafeFunctionsAreAvailable",
+ IsSafeFunctionsAreAvailable);
+}
+
+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, int ArgCount, 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));
+ }
+ };
+
+ 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);
+
+ 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(),
+ "memory allocated by '%0' is insufficient to 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.contains("memmove") && isTrimMemmove(Result))
+ return;
+
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "the result from calling '%0' 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 == "memmove" || Name == "wmemmove")
+ memmoveFix(Name, Result, Diag);
+ else if (Name == "memmove_s" || Name == "wmemmove_s")
+ lengthArgInsertIncByOne(3, Result, Diag);
+ else if (Name == "memset" || Name == "wmemset")
+ lengthArgRemoveIncByOne(Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memcpyFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ if (IsSafeFunctionsAreAvailable) {
+ 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,
+ DiagnosticBuilder &Diag) {
+ if (IsSafeFunctionsAreAvailable) {
+ StringRef NewFuncName = (Name[0] != 'w') ? "memmove_s" : "wmemmove_s";
+ renameFunc(NewFuncName, Result, Diag);
+
+ const auto FirstArg = Result.Nodes.getNodeAs<CallExpr>("expr")->getArg(0);
+ SmallString<64> NewSecondArg;
+ NewSecondArg += " strlen(";
+ NewSecondArg += exprToStr(FirstArg, Result);
+ NewSecondArg += "),";
+
+ const auto InsertNewArgFix = FixItHint::CreateInsertion(
+ exprLocEnd(FirstArg, Result).getLocWithOffset(1), NewSecondArg);
+ Diag << InsertNewArgFix;
+ }
+ lengthArgInsertIncByOne(2, 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(),
+ "the result from calling 'strerror_s' 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");
+ const auto *LengthExprArg =
+ Result.Nodes.getNodeAs<CallExpr>("length-call")->getArg(0);
+ StringRef FirstArgStr = exprToStr(FuncExpr->getArg(0), Result);
+ StringRef SecondArgStr = exprToStr(FuncExpr->getArg(1), Result);
+ StringRef LengthExprArgStr = exprToStr(LengthExprArg, Result);
+
+ if (LengthExprArgStr == FirstArgStr || LengthExprArgStr == SecondArgStr) {
+ auto Diag = diag(
+ LengthExprArg->getLocStart(),
+ "comparison length is too long and might lead to a buffer overflow");
+ lengthArgRemoveIncByOne(Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::xfrmFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ auto Diag = diag(Result.Nodes.getNodeAs<CallExpr>("expr")->getLocStart(),
+ "the result from calling '%0' 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(
+ 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;
+ if (IsComplexCase) {
+ if (const auto *BinOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("length-wout-inc-comp")) {
+ NeedInnerParen = BinOp->getOpcode() != BO_Add;
+ }
+ }
+
+ if (NeedInnerParen) {
+ const auto InsertFirstParenFix =
+ FixItHint::CreateInsertion(StrLenExpr->getLocStart(), "(");
+ const auto InsertPlusOneAndSecondParenFix = FixItHint::CreateInsertion(
+ StrLenExpr->getLocEnd().getLocWithOffset(1), ") + 1");
+ Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+ } else {
+ const auto InsertPlusOne = FixItHint::CreateInsertion(
+ StrLenExpr->getLocEnd().getLocWithOffset(1), " + 1");
+ Diag << InsertPlusOne;
+ }
+}
+
+/// FIXME: If there is a ParenExpr then remove the extra parens.
+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");
+
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // LHSRemoveRange: ~
+ // RHSRemoveRange: ~~~~~
+ const auto LHSRemoveRange =
+ SourceRange(OuterOperatorExpr->getLocStart(),
+ InnerOperatorExpr->getLocStart().getLocWithOffset(-1));
+
+ const auto RHSRemoveRange =
+ SourceRange(InnerOperatorExpr->getLocEnd().getLocWithOffset(1),
+ OuterOperatorExpr->getLocEnd());
+
+ const auto LHSRemoveFix = FixItHint::CreateRemoval(LHSRemoveRange);
+ const auto RHSRemoveFix = FixItHint::CreateRemoval(RHSRemoveRange);
+ Diag << LHSRemoveFix << RHSRemoveFix;
+}
+
+void NotNullTerminatedResultCheck::removeArg(
+ 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
ParentVirtualCallCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.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 "ParentVirtualCallCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
@@ -91,6 +92,8 @@
"bugprone-move-forwarding-reference");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
+ "bugprone-not-null-terminated-result");
CheckFactories.registerCheck<ParentVirtualCallCheck>(
"bugprone-parent-virtual-call");
CheckFactories.registerCheck<SizeofContainerCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
