Charusso updated this revision to Diff 149129.
Charusso added a comment.
Clang format.
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-in-initialization.c
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: test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
@@ -0,0 +1,225 @@
+// 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) {
+ int length = strlen(src);
+ dest = (char *)alloca(length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: dest = (char *)alloca(length + 1);
+}
+
+void good_alloca(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)alloca(length + 1);
+}
+
+void bad_calloc(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)calloc(length, 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: dest = (char *)calloc(length + 1, sizeof(char));
+}
+
+void good_calloc(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)calloc(length + 1, sizeof(char));
+}
+
+void bad_malloc1(char *dest, const char *src) {
+ int length = strlen(src) * 2;
+ dest = (char *)malloc(length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: dest = (char *)malloc(length + 1);
+}
+
+void good_malloc1(char *dest, const char *src) {
+ int length = strlen(src) * 2;
+ dest = (char *)malloc(length + 1);
+}
+
+void bad_malloc2(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)malloc(length * 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: dest = (char *)malloc(length * 2 + 1);
+}
+
+void good_malloc2(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)malloc(length * 2 + 1);
+}
+
+void bad_realloc(char *dest, const char *src) {
+ int length = strlen(dest) + strlen(src);
+ dest = (char *)realloc(dest, length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: dest = (char *)realloc(dest, length + 1);
+}
+
+void good_realloc(char *dest, const char *src) {
+ int length = strlen(dest) + strlen(src);
+ dest = (char *)realloc(dest, length + 1);
+}
+
+void bad_memcpy(char *dest, const char *src) {
+ int length = strlen(src);
+ memcpy(dest, src, length);
+ // 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, length);
+}
+
+void good_memcpy_safe(char *dest, const char *src) {
+ int length = strlen(src);
+ strncpy_s(dest, src, length);
+}
+
+void bad_memcpy_s(char *dest, const char *src) {
+ int destLength = strlen(dest);
+ int srcLength = strlen(src);
+ memcpy_s(dest, destLength, src, srcLength);
+ // 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, srcLength);
+}
+
+void good_memcpy_s(char *dest, const char *src) {
+ int length = strlen(src);
+ strncpy_s(dest, src, length);
+}
+
+void bad_memchr(char *dest, const char *src) {
+ int length = strlen(src);
+ dest = (char *)memchr(src, '\0', length);
+ // 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) {
+ int length = strlen(src);
+ memmove(dest, src, length);
+ // 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, length + 1);
+}
+
+void good_memmove_safe(char *dest, const char *src) {
+ int length = strlen(src);
+ memmove_s(dest, strlen(dest), src, length + 1);
+}
+
+void bad_memmove_s(char *dest, const char *src) {
+ int length = strlen(src);
+ memmove_s(dest, strlen(dest), src, length);
+ // 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, length + 1);
+}
+
+void good_memmove_s_1(char *dest, const char *src) {
+ int length = strlen(src);
+ memmove_s(dest, strlen(dest), src, length + 1);
+}
+
+void good_memmove_s_2(char *dest, const char *src) {
+ int length = strlen(src);
+ memmove_s(dest, strlen(dest), (src + 1), length);
+}
+
+void bad_memset_1(char *dest, const char *src) {
+ int length = strlen(src) + 1;
+ memset(dest, '-', length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memset' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memset(dest, '-', length - 1);
+}
+
+void good_memset1(char *dest, const char *src) {
+ int length = strlen(src) + 1;
+ memset(dest, '-', length - 1);
+}
+
+void bad_memset_2(char *dest, const char *src) {
+ int length = strlen(src);
+ memset(dest, '-', length + 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, '-', length);
+}
+
+void good_memset_2(char *dest, const char *src) {
+ int length = strlen(src);
+ memset(dest, '-', length);
+}
+
+void bad_strerror_s(char *dest, int errno) {
+ int length = strlen(strerror(errno));
+ strerror_s(dest, length, 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, length + 1, errno);
+}
+
+void good_strerror_s(char *dest, int errno) {
+ int length = strlen(strerror(errno));
+ strerror_s(dest, length + 1, errno);
+}
+
+int bad_strncmp_1(char *str1, const char *str2) {
+ int length = strlen(str1) + 1;
+ return strncmp(str1, str2, length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str1, str2, length - 1);
+}
+
+int good_strncmp_1(const char *str1, const char *str2) {
+ int length = strlen(str1) + 1;
+ return strncmp(str1, str2, length - 1);
+}
+
+int bad_strncmp_2(char *str1, const char *str2) {
+ int length = strlen(str1);
+ return strncmp(str1, str2, length + 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str1, str2, length);
+}
+
+int good_strncmp_2(const char *str1, const char *str2) {
+ int length = strlen(str1);
+ return strncmp(str1, str2, length);
+}
+
+void bad_strxfrm(char *long_destination_name, const char *long_source_name) {
+ int very_long_length_definition_name = strlen(long_source_name);
+ strxfrm(long_destination_name, long_source_name,
+ very_long_length_definition_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: very_long_length_definition_name + 1);
+}
+
+void good_strxfrm(char *long_destination_name, const char *long_source_name) {
+ int very_long_length_definition_name = strlen(long_source_name);
+ strxfrm(long_destination_name, long_source_name,
+ very_long_length_definition_name + 1);
+}
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,79 @@
+//===--------------- 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(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,485 @@
+//===------------- 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 const char *const FuncExprName = "entire-called-function-expr";
+static const char *const CastExprName = "cast-expr";
+static const char *const LengthCallName = "length-call";
+static const char *const LengthDREName = "length-decl-ref-expr";
+static const char *const LengthOpHasDREName = "length-has-operator-has-dre";
+static const char *const LengthWoutIncSimpName = "length-without-inc-simple";
+static const char *const LengthWoutIncCompName = "length-without-inc-complex";
+static const char *const InnerOpName = "inner-operator";
+static const char *const OuterOpName = "outer-operator";
+
+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>(FuncExprName);
+ 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>(LengthCallName)->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) {
+ const auto LengthCall =
+ callExpr(callee(functionDecl(hasAnyName("::strlen", "::wcslen"))),
+ expr().bind(LengthCallName));
+ const auto HasIntOperand =
+ hasEitherOperand(ignoringParenImpCasts(integerLiteral()));
+ const auto HasInc = binaryOperator(hasOperatorName("+"), HasIntOperand);
+
+ // The following five case match 'strlen' or 'wcslen' function calls where no
+ // increase by one operation found.
+ // - Example: strlen(src) or wcslen(src)
+ const auto SimpleCallWithoutInc = LengthCall.bind(LengthWoutIncSimpName);
+
+ // - Example: (strlen(src) * 2) or (wcslen(src) * 2)
+ const auto ComplexCallWithoutInc =
+ binaryOperator(unless(hasOperatorName("+")),
+ hasEitherOperand(ignoringImpCasts(LengthCall)))
+ .bind(LengthWoutIncCompName);
+
+ // - Example: (strlen(dest) + strlen(src)) or (wcslen(dest) + wcslen(src))
+ const auto ComplexTwoCallWithoutInc =
+ binaryOperator(hasOperatorName("+"), unless(HasIntOperand),
+ hasLHS(ignoringImpCasts(LengthCall)),
+ hasRHS(ignoringImpCasts(LengthCall)))
+ .bind(LengthWoutIncCompName);
+
+ const auto AnyOfWithoutInc = ignoringImpCasts(anyOf(
+ SimpleCallWithoutInc, ComplexCallWithoutInc, ComplexTwoCallWithoutInc));
+
+ // - Example: length = strlen(src);
+ const auto DREWithoutInc =
+ ignoringImpCasts(declRefExpr(to(varDecl(hasInitializer(AnyOfWithoutInc))))
+ .bind(LengthDREName));
+
+ // - Example: malloc(length * 2)
+ const auto OpHasDREWithoutInc =
+ binaryOperator(unless(hasOperatorName("+")),
+ hasEitherOperand(DREWithoutInc))
+ .bind(LengthOpHasDREName);
+
+ const auto LengthWithoutInc =
+ anyOf(OpHasDREWithoutInc, DREWithoutInc, AnyOfWithoutInc);
+
+ // The following three case match 'strlen' or 'wcslen' function calls where
+ // increase by one operation found.
+ // - Example: (strlen(src) + 1) or (wcslen(src) + 1)
+ const auto SimpleCallWithInc =
+ allOf(HasInc, binaryOperator(hasEitherOperand(ignoringImpCasts(
+ LengthCall.bind(InnerOpName))))
+ .bind(OuterOpName));
+
+ // - Example: ((strlen(src) / 2) + 1) or ((wcslen(src) / 2) + 1)
+ const auto ComplexCallWithInc = allOf(
+ HasInc,
+ binaryOperator(
+ has(binaryOperator(unless(hasOperatorName("+")),
+ hasEitherOperand(ignoringImpCasts(LengthCall)))
+ .bind(InnerOpName)))
+ .bind(OuterOpName));
+
+ // - Example: length = strlen(src); memcpy(dest, src, length + 1)
+ const auto OpHasDREWithInc =
+ binaryOperator(hasEitherOperand(ignoringImpCasts(DREWithoutInc)))
+ .bind(LengthOpHasDREName);
+
+ const auto LengthWithInc = ignoringImpCasts(
+ anyOf(OpHasDREWithInc,
+ declRefExpr(to(varDecl(hasInitializer(ignoringImpCasts(
+ anyOf(SimpleCallWithInc, ComplexCallWithInc))))))
+ .bind(LengthDREName),
+ SimpleCallWithInc, ComplexCallWithInc));
+
+ 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, ignoringImpCasts(LengthWithoutInc)));
+ } else {
+ return allOf(callee(functionDecl(hasName(Name))),
+ argumentCountIs(ArgCount),
+ hasArgument(ArgPos, ignoringImpCasts(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(FuncExprName), this);
+ Finder->addMatcher(
+ binaryOperator(
+ has(cStyleCastExpr(
+ has(callExpr(anyOf(Memchr, Wmemchr)).bind(FuncExprName)))
+ .bind(CastExprName))),
+ this);
+}
+
+void NotNullTerminatedResultCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+
+ 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>(FuncExprName)->getLocStart(),
+ "memory allocated by '%0' is insufficient to hold the null "
+ "terminator")
+ << Name;
+
+ lengthArgInsertIncByOne(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>(FuncExprName)->getLocStart(),
+ "the result from calling '%0' is not null-terminated")
+ << Name;
+
+ if (Name.endswith("cpy"))
+ memcpyFix(Name, Result, Diag);
+ else if (Name.endswith("cpy_s"))
+ memcpy_sFix(Name, Result, Diag);
+ else if (Name.endswith("chr"))
+ memchrFix(Name, Result, Diag);
+ else if (Name.endswith("move"))
+ memmoveFix(Name, Result, Diag);
+ else if (Name.endswith("move_s"))
+ lengthArgInsertIncByOne(Result, Diag);
+ else if (Name.endswith("set"))
+ 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(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>(CastExprName)) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ 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>(FuncExprName)->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(Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::characterFuncFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (Name == "strerror_s")
+ strerror_sFix(Result);
+ else if (Name.endswith("ncmp"))
+ ncmpFix(Name, Result);
+ else if (Name.endswith("xfrm"))
+ xfrmFix(Name, Result);
+}
+
+void NotNullTerminatedResultCheck::strerror_sFix(
+ const MatchFinder::MatchResult &Result) {
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>(FuncExprName)->getLocStart(),
+ "the result from calling 'strerror_s' is not null-terminated and "
+ "missing the last character of the error message");
+ lengthArgInsertIncByOne(Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::ncmpFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const auto *LengthExprArg =
+ Result.Nodes.getNodeAs<CallExpr>(LengthCallName)->getArg(0);
+ StringRef FirstArgStr = exprToStr(FuncExpr->getArg(0), Result);
+ StringRef SecondArgStr = exprToStr(FuncExpr->getArg(1), Result);
+ StringRef LengthExprArgStr = exprToStr(LengthExprArg, Result);
+
+ SourceLocation LengthExprLocStart;
+ if (const auto *LengthDRE = Result.Nodes.getNodeAs<Expr>(LengthDREName))
+ LengthExprLocStart = LengthDRE->getLocStart();
+ else
+ LengthExprLocStart = LengthExprArg->getLocStart();
+
+ if (LengthExprArgStr == FirstArgStr || LengthExprArgStr == SecondArgStr) {
+ auto Diag = diag(
+ LengthExprLocStart,
+ "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>(FuncExprName)->getLocStart(),
+ "the result from calling '%0' is not null-terminated")
+ << Name;
+ lengthArgInsertIncByOne(Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::lengthArgInsertIncByOne(
+ const MatchFinder::MatchResult &Result, DiagnosticBuilder &Diag) {
+ if (const auto *LengthOpHasDRE =
+ Result.Nodes.getNodeAs<BinaryOperator>(LengthOpHasDREName)) {
+ const auto OpHasDREInsertPlusOne =
+ FixItHint::CreateInsertion(exprLocEnd(LengthOpHasDRE, Result), " + 1");
+ Diag << OpHasDREInsertPlusOne;
+ return;
+ }
+
+ if (const auto *LengthDRE = Result.Nodes.getNodeAs<Expr>(LengthDREName)) {
+ const auto DREInsertPlusOne =
+ FixItHint::CreateInsertion(exprLocEnd(LengthDRE, Result), " + 1");
+ Diag << DREInsertPlusOne;
+ return;
+ }
+
+ bool IsComplexCase = false;
+ const Expr *StrLenExpr = nullptr;
+ if (const auto *StrLen = Result.Nodes.getNodeAs<Expr>(LengthWoutIncSimpName))
+ StrLenExpr = StrLen;
+ else {
+ StrLenExpr = Result.Nodes.getNodeAs<Expr>(LengthWoutIncCompName);
+ IsComplexCase = true;
+ }
+
+ bool NeedInnerParen = false;
+ if (IsComplexCase) {
+ if (const auto *BinOp =
+ Result.Nodes.getNodeAs<BinaryOperator>(LengthWoutIncCompName)) {
+ 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) {
+ if (const auto *LengthOpHasDRE =
+ Result.Nodes.getNodeAs<BinaryOperator>(LengthOpHasDREName)) {
+ const auto RemovePlusOne = FixItHint::CreateRemoval(SourceRange(
+ exprLocEnd(LengthOpHasDRE->getLHS(), Result),
+ exprLocEnd(LengthOpHasDRE->getRHS(), Result).getLocWithOffset(-1)));
+ Diag << RemovePlusOne;
+ return;
+ }
+
+ if (const auto *LengthDRE = Result.Nodes.getNodeAs<Expr>(LengthDREName)) {
+ const auto InsertMinusOne =
+ FixItHint::CreateInsertion(exprLocEnd(LengthDRE, Result), " - 1");
+ Diag << InsertMinusOne;
+ return;
+ }
+
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // InnerOperatorExpr: ~~~~~~~~~~~~^~~
+ // OuterOperatorExpr: ~~~~~~~~~~~~~~~~~~^~~
+ const auto InnerOperatorExpr = Result.Nodes.getNodeAs<Expr>(InnerOpName);
+ const auto OuterOperatorExpr = Result.Nodes.getNodeAs<Expr>(OuterOpName);
+
+ // 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>(FuncExprName);
+ 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>(FuncExprName);
+ 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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
