Charusso updated this revision to Diff 166802.
Charusso marked 28 inline comments as done.
Charusso added a comment.
- Refactor and better English thanks for @whisperity!
- Removed `InjectUL` option so the checker handles the special case if the
capacity of a buffer is `int.Max()`
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-strlen.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
Index: test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
@@ -0,0 +1,135 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+template <size_t size>
+errno_t wcsncpy_s(wchar_t (&dest)[size], const wchar_t *src, size_t length);
+errno_t wcsncpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+
+template <size_t size>
+wchar_t *wcsncpy(wchar_t (&dest)[size], const wchar_t *src, size_t length);
+wchar_t *wcsncpy(wchar_t *, const wchar_t *, size_t);
+
+template <size_t size>
+errno_t wcscpy_s(wchar_t (&dest)[size], const wchar_t *);
+errno_t wcscpy_s(wchar_t *, size_t, const wchar_t *);
+
+template <size_t size>
+wchar_t *wcscpy(wchar_t (&dest)[size], const wchar_t *);
+wchar_t *wcscpy(wchar_t *, const wchar_t *);
+
+errno_t wmemcpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+wchar_t *wmemcpy(wchar_t *, const wchar_t *, size_t);
+
+
+//===----------------------------------------------------------------------===//
+// wmemcpy() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_wmemcpy_known_dest(const wchar_t *src) {
+ wchar_t dest01[13];
+ wmemcpy(dest01, 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: wchar_t dest01[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest01, src);
+}
+
+void good_wmemcpy_known_dest(const wchar_t *src) {
+ wchar_t dst01[14];
+ wcscpy_s(dst01, src);
+}
+
+//===----------------------------------------------------------------------===//
+// wmemcpy() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_wmemcpy_full_source_length(const wchar_t *src) {
+ wchar_t dest20[13];
+ wmemcpy(dest20, 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: wchar_t dest20[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest20, src);
+}
+
+void good_wmemcpy_full_source_length(const wchar_t *src) {
+ wchar_t dst20[14];
+ wcscpy_s(dst20, src);
+}
+
+void bad_wmemcpy_partial_source_length(const wchar_t *src) {
+ wchar_t dest21[13];
+ wmemcpy(dest21, src, wcslen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t dest21[14];
+ // CHECK-FIXES-NEXT: wcsncpy_s(dest21, src, wcslen(src) - 1);
+}
+
+void good_wmemcpy_partial_source_length(const wchar_t *src) {
+ wchar_t dst21[14];
+ wcsncpy_s(dst21, src, wcslen(src) - 1);
+}
+
+//===----------------------------------------------------------------------===//
+// wmemcpy_s() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_wmemcpy_s_unknown_dest(wchar_t *dest40, const wchar_t *src) {
+ wmemcpy_s(dest40, 13, 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: wcscpy_s(dest40, 13, src);
+}
+
+void good_wmemcpy_s_unknown_dest(wchar_t *dst40, const wchar_t *src) {
+ wcscpy_s(dst40, 13, src);
+}
+
+void bad_wmemcpy_s_known_dest(const wchar_t *src) {
+ wchar_t dest41[13];
+ wmemcpy_s(dest41, 13, 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: wchar_t dest41[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest41, src);
+}
+
+void good_wmemcpy_s_known_dest(const wchar_t *src) {
+ wchar_t dst41[13];
+ wcscpy_s(dst41, src);
+}
+
+//===----------------------------------------------------------------------===//
+// wmemcpy_s() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_wmemcpy_s_full_source_length(const wchar_t *src) {
+ wchar_t dest60[13];
+ wmemcpy_s(dest60, 13, 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: wchar_t dest60[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest60, src);
+}
+
+void good_wmemcpy_s_full_source_length(const wchar_t *src) {
+ wchar_t dst60[13];
+ wcscpy_s(dst60, src);
+}
+
+void bad_wmemcpy_s_partial_source_length(const wchar_t *src) {
+ wchar_t dest61[13];
+ wmemcpy_s(dest61, 13, src, wcslen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t dest61[14];
+ // CHECK-FIXES-NEXT: wcsncpy_s(dest61, src, wcslen(src) - 1);
+}
+
+void good_wmemcpy_s_partial_source_length(const wchar_t *src) {
+ wchar_t dst61[13];
+ wcsncpy_s(dst61, src, wcslen(src) - 1);
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
@@ -0,0 +1,130 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+
+wchar_t *wcschr(const wchar_t *, int);
+errno_t wcsncpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, 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);
+
+
+void bad_wmemchr_1(wchar_t *position, const wchar_t *src) {
+ position = (wchar_t *)wmemchr(src, L'\0', wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: the length is too short for the last L'\0' [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: position = wcschr(src, L'\0');
+}
+
+void good_wmemchr_1(wchar_t *pos, const wchar_t *src) {
+ pos = wcschr(src, L'\0');
+}
+
+void bad_wmemchr_2(wchar_t *position) {
+ position = (wchar_t *)wmemchr(L"foobar", L'\0', 6);
+ // CHECK-MESSAGES: :[[@LINE-1]]:51: warning: the length is too short for the last L'\0' [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: position = wcschr(L"foobar", L'\0');
+}
+
+void good_wmemchr_2(wchar_t *pos) {
+ pos = wcschr(L"foobar", L'\0');
+}
+
+
+void bad_wmemmove(const wchar_t *src) {
+ wchar_t dest[13];
+ 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: wchar_t dest[14];
+ // CHECK-FIXES-NEXT: wmemmove_s(dest, 14, src, wcslen(src) + 1);
+}
+
+void good_wmemmove(const wchar_t *src) {
+ wchar_t dst[14];
+ wmemmove_s(dst, 13, src, wcslen(src) + 1);
+}
+
+void bad_wmemmove_s(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, 13, 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, 13, src, wcslen(src) + 1);
+}
+
+void good_wmemmove_s_1(wchar_t *dest, const wchar_t *src) {
+ wmemmove_s(dest, 13, src, wcslen(src) + 1);
+}
+
+void bad_wmemset(const wchar_t *src) {
+ wchar_t dest[13];
+ wmemset(dest, L'-', 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, L'-', wcslen(src));
+}
+
+void good_wmemset(const wchar_t *src) {
+ wchar_t dst[13];
+ wmemset(dst, L'-', wcslen(src));
+}
+
+int bad_wcsncmp_1(wchar_t *wcs0, const wchar_t *wcs1) {
+ return wcsncmp(wcs0, wcs1, (wcslen(wcs0) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncmp(wcs0, wcs1, wcslen(wcs0));
+}
+
+int bad_wcsncmp_2(wchar_t *wcs2, const wchar_t *wcs3) {
+ return wcsncmp(wcs2, wcs3, 1 + wcslen(wcs2));
+ // 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: wcsncmp(wcs2, wcs3, wcslen(wcs2));
+}
+
+int good_wcsncmp_1_2(wchar_t *wcs4, const wchar_t *wcs5) {
+ return wcsncmp(wcs4, wcs5, wcslen(wcs4));
+}
+
+int bad_wcsncmp_3(wchar_t *wcs6) {
+ return wcsncmp(wcs6, L"string", 7);
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wcsncmp(wcs6, L"string", 6);
+}
+
+int good_wcsncmp_3(wchar_t *wcs7) {
+ return wcsncmp(wcs7, L"string", 6);
+}
+
+void bad_wcsxfrm_1(const wchar_t *long_source_name) {
+ wchar_t long_destination_array_name[13];
+ wcsxfrm(long_destination_array_name, long_source_name,
+ wcslen(long_source_name));
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: the result from calling 'wcsxfrm' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t long_destination_array_name[14];
+ // CHECK-FIXES-NEXT: wcsxfrm(long_destination_array_name, long_source_name,
+ // CHECK-FIXES-NEXT: wcslen(long_source_name) + 1);
+}
+
+void good_wcsxfrm_1(const wchar_t *long_source_name) {
+ wchar_t long_destination_array_name[14];
+ wcsxfrm(long_destination_array_name, long_source_name,
+ wcslen(long_source_name) + 1);
+}
+
+void bad_wcsxfrm_2() {
+ wchar_t long_destination_array_name1[16];
+ wcsxfrm(long_destination_array_name1, L"long_source_name", 16);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wcsxfrm' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t long_destination_array_name1[17];
+ // CHECK-FIXES: wcsxfrm(long_destination_array_name1, L"long_source_name", 17);
+}
+
+void good_wcsxfrm_2() {
+ wchar_t long_destination_array_name2[17];
+ wcsxfrm(long_destination_array_name2, L"long_source_name", 17);
+}
Index: test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
@@ -0,0 +1,145 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+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 *, size_t, const char *, 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 *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);
+
+
+void bad_memchr_1(char *position, const char *src) {
+ position = (char *)memchr(src, '\0', strlen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: position = strchr(src, '\0');
+}
+
+void good_memchr_1(char *pos, const char *src) {
+ pos = strchr(src, '\0');
+}
+
+void bad_memchr_2(char *position) {
+ position = (char *)memchr("foobar", '\0', 6);
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: position = strchr("foobar", '\0');
+}
+
+void good_memchr_2(char *pos) {
+ pos = strchr("foobar", '\0');
+}
+
+
+void bad_memmove(const char *src) {
+ char dest[13];
+ 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: char dest[14];
+ // CHECK-FIXES-NEXT: memmove_s(dest, 14, src, strlen(src) + 1);
+}
+
+void good_memmove(const char *src) {
+ char dst[14];
+ memmove_s(dst, 13, src, strlen(src) + 1);
+}
+
+void bad_memmove_s(char *dest, const char *src) {
+ memmove_s(dest, 13, 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, 13, src, strlen(src) + 1);
+}
+
+void good_memmove_s_1(char *dest, const char *src) {
+ memmove_s(dest, 13, src, strlen(src) + 1);
+}
+
+void bad_memset(const char *src) {
+ char dest[13];
+ 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(const char *src) {
+ char dst[13];
+ memset(dst, '-', strlen(src));
+}
+
+void bad_strerror_s(int errno) {
+ char dest[13];
+ 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: char dest[14];
+ // CHECK-FIXES-NEXT: strerror_s(dest, strlen(strerror(errno)) + 1, errno);
+}
+
+void good_strerror_s(int errno) {
+ char dst[14];
+ strerror_s(dst, strlen(strerror(errno)) + 1, errno);
+}
+
+int bad_strncmp_1(char *str0, const char *str1) {
+ return strncmp(str0, str1, (strlen(str0) + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str0, str1, strlen(str0));
+}
+
+int bad_strncmp_2(char *str2, const char *str3) {
+ return strncmp(str2, str3, 1 + strlen(str2));
+ // 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(str2, str3, strlen(str2));
+}
+
+int good_strncmp_1_2(char *str4, const char *str5) {
+ return strncmp(str4, str5, strlen(str4));
+}
+
+int bad_strncmp_3(char *str6) {
+ return strncmp(str6, "string", 7);
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str6, "string", 6);
+}
+
+int good_strncmp_3(char *str7) {
+ return strncmp(str7, "string", 6);
+}
+
+void bad_strxfrm_1(const char *long_source_name) {
+ char long_destination_array_name[13];
+ strxfrm(long_destination_array_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: char long_destination_array_name[14];
+ // CHECK-FIXES-NEXT: strxfrm(long_destination_array_name, long_source_name,
+ // CHECK-FIXES-NEXT: strlen(long_source_name) + 1);
+}
+
+void good_strxfrm_1(const char *long_source_name) {
+ char long_destination_array_name[14];
+ strxfrm(long_destination_array_name, long_source_name,
+ strlen(long_source_name) + 1);
+}
+
+void bad_strxfrm_2() {
+ char long_destination_array_name1[16];
+ strxfrm(long_destination_array_name1, "long_source_name", 16);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'strxfrm' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char long_destination_array_name1[17];
+ // CHECK-FIXES: strxfrm(long_destination_array_name1, "long_source_name", 17);
+}
+
+void good_strxfrm_2() {
+ char long_destination_array_name2[17];
+ strxfrm(long_destination_array_name2, "long_source_name", 17);
+}
Index: test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
@@ -0,0 +1,136 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t strlen(const char *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+errno_t strncpy_s(char *, size_t, const char *, size_t);
+errno_t strcpy_s(char *, size_t, const char *);
+char *strcpy(char *, const char *);
+
+errno_t memcpy_s(void *, size_t, const void *, size_t);
+void *memcpy(void *, const void *, size_t);
+
+//===----------------------------------------------------------------------===//
+// memcpy() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dest00[13];
+ memcpy(dest00, 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: unsigned char dest00[14];
+ // CHECK-FIXES-NEXT: memcpy_s(dest00, 14, src, strlen(src) + 1);
+}
+
+void good_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dst00[14];
+ memcpy_s(dst00, 14, src, strlen(src) + 1);
+}
+
+void bad_memcpy_known_dest(const char *src) {
+ char dest01[13];
+ memcpy(dest01, 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: char dest01[14];
+ // CHECK-FIXES: strcpy_s(dest01, 14, src);
+}
+
+void good_memcpy_known_dest(const char *src) {
+ char dst01[14];
+ strcpy_s(dst01, 14, src);
+}
+
+//===----------------------------------------------------------------------===//
+// memcpy() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_full_source_length(const char *src) {
+ char dest20[13];
+ memcpy(dest20, 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: char dest20[14];
+ // CHECK-FIXES-NEXT: strcpy_s(dest20, 14, src);
+}
+
+void good_memcpy_full_source_length(const char *src) {
+ char dst20[14];
+ strcpy_s(dst20, 14, src);
+}
+
+void bad_memcpy_partial_source_length(const char *src) {
+ char dest21[13];
+ memcpy(dest21, src, strlen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char dest21[14];
+ // CHECK-FIXES-NEXT: strncpy_s(dest21, 14, src, strlen(src) - 1);
+}
+
+void good__memcpy_partial_source_length(const char *src) {
+ char dst21[14];
+ strncpy_s(dst21, 14, src, strlen(src) - 1);
+}
+
+
+//===----------------------------------------------------------------------===//
+// memcpy_s() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_s_unknown_dest(char *dest40, const char *src) {
+ memcpy_s(dest40, 13, 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: strcpy_s(dest40, 13, src);
+}
+
+void good_memcpy_s_unknown_dest(char *dst40, const char *src) {
+ strcpy_s(dst40, 13, src);
+}
+
+void bad_memcpy_s_known_dest(const char *src) {
+ char dest41[13];
+ memcpy_s(dest41, 13, 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: char dest41[14];
+ // CHECK-FIXES-NEXT: strcpy_s(dest41, 14, src);
+}
+
+void good_memcpy_s_known_dest(const char *src) {
+ char dst41[14];
+ strcpy_s(dst41, 14, src);
+}
+
+//===----------------------------------------------------------------------===//
+// memcpy_s() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_s_full_source_length(const char *src) {
+ char dest60[13];
+ memcpy_s(dest60, 13, 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: char dest60[14];
+ // CHECK-FIXES-NEXT: strcpy_s(dest60, 14, src);
+}
+
+void good_memcpy_s_full_source_length(const char *src) {
+ char dst60[14];
+ strcpy_s(dst60, 14, src);
+}
+
+void bad_memcpy_s_partial_source_length(const char *src) {
+ char dest61[13];
+ memcpy_s(dest61, 13, src, strlen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char dest61[14];
+ // CHECK-FIXES-NEXT: strncpy_s(dest61, 14, src, strlen(src) - 1);
+}
+
+void good_memcpy_s_partial_source_length(const char *src) {
+ char dst61[14];
+ strncpy_s(dst61, 14, src, strlen(src) - 1);
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: bugprone-not-null-terminated-result.AreSafeFunctionsAvailable, \
+// RUN: value: 1}]}" \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t strlen(const char *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+errno_t strncpy_s(char *, size_t, const char *, size_t);
+errno_t strcpy_s(char *, size_t, const char *);
+char *strcpy(char *, const char *);
+
+errno_t memcpy_s(void *, size_t, const void *, size_t);
+void *memcpy(void *, const void *, size_t);
+
+#define SRC_LENGTH 3
+#define SRC "foo"
+
+
+void good_memcpy_known_src() {
+ char dest[13];
+ char src[] = "foobar";
+ memcpy(dest, src, sizeof(src));
+}
+
+void good_memcpy_null_terminated(const char *src) {
+ char dest[13];
+ const int length = strlen(src);
+ memcpy(dest, src, length);
+ dest[length] = '\0';
+}
+
+void good_memcpy_proper_length(const char *src) {
+ char *dest = 0;
+ int length = strlen(src) + 1;
+ dest = (char *)malloc(length);
+ memcpy(dest, src, length);
+}
+
+void may_bad_memcpy_unknown_length(const char *src, int length) {
+ char dest[13];
+ memcpy(dest, src, length);
+}
+
+void may_bad_memcpy_const_length(const char *src) {
+ char dest[13];
+ memcpy(dest, src, 12);
+}
+
+void bad_memcpy_unknown_dest(char *dest01, const char *src) {
+ memcpy(dest01, 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: strcpy(dest01, src);
+}
+
+void good_memcpy_unknown_dest(char *dst01, const char *src) {
+ strcpy(dst01, src);
+}
+
+void bad_memcpy_variable_array(int dest_length) {
+ char dest02[dest_length + 1];
+ memcpy(dest02, "foobarbazqux", strlen("foobarbazqux"));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strcpy(dest02, "foobarbazqux");
+}
+
+void good_memcpy_variable_array(int dest_length) {
+ char dst02[dest_length + 1];
+ strcpy(dst02, "foobarbazqux");
+}
+
+void bad_memcpy_equal_src_length_and_length() {
+ char dest03[13];
+ const char *src = "foobarbazqux";
+ memcpy(dest03, src, 12);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strcpy(dest03, src);
+}
+
+void good_memcpy_equal_src_length_and_length() {
+ char dst03[13];
+ const char *src = "foobarbazqux";
+ strcpy(dst03, src);
+}
+
+void bad_memcpy_dest_size_overflows(const char *src) {
+ const int length = strlen(src);
+ char *dest04 = (char *)malloc(length);
+ memcpy(dest04, src, length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char *dest04 = (char *)malloc(length + 1);
+ // CHECK-FIXES-NEXT: strcpy(dest04, src);
+}
+
+void good_memcpy_dest_size_overflows(const char *src) {
+ const int length = strlen(src);
+ char *dst04 = (char *)malloc(length + 1);
+ strcpy(dst04, src);
+}
+
+void bad_memcpy_macro() {
+ unsigned char dest05[13];
+ memcpy(dest05, SRC, SRC_LENGTH);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memcpy_s(dest05, 13, SRC, SRC_LENGTH + 1);
+}
+
+void good_memcpy_macro() {
+ unsigned char dst05[13];
+ memcpy_s(dst05, 13, SRC, SRC_LENGTH + 1);
+}
Index: test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
@@ -0,0 +1,159 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+namespace std {
+template <typename T>
+struct basic_string {
+ basic_string();
+ const T *data() const;
+ unsigned long size() const;
+};
+typedef basic_string<char> string;
+}
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t strlen(const char *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+template <size_t size>
+errno_t strncpy_s(char (&dest)[size], const char *src, size_t length);
+errno_t strncpy_s(char *, size_t, const char *, size_t);
+
+template <size_t size>
+char *strncpy(char (&dest)[size], const char *src, size_t length);
+char *strncpy(char *, const char *, size_t);
+
+template <size_t size>
+errno_t strcpy_s(char (&dest)[size], const char *);
+errno_t strcpy_s(char *, size_t, const char *);
+
+template <size_t size>
+char *strcpy(char (&dest)[size], const char *);
+char *strcpy(char *, const char *);
+
+errno_t memcpy_s(void *, size_t, const void *, size_t);
+void *memcpy(void *, const void *, size_t);
+
+
+//===----------------------------------------------------------------------===//
+// memcpy() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dest00[13];
+ memcpy(dest00, 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: unsigned char dest00[14];
+ // CHECK-FIXES-NEXT: memcpy_s(dest00, 14, src, strlen(src) + 1);
+}
+
+void good_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dst00[14];
+ memcpy_s(dst00, 14, src, strlen(src) + 1);
+}
+
+void bad_memcpy_known_dest(const char *src) {
+ char dest01[13];
+ memcpy(dest01, 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: dest01[14];
+ // CHECK-FIXES-NEXT: strcpy_s(dest01, src);
+}
+
+void good_memcpy_known_dest(const char *src) {
+ char dst01[14];
+ strcpy_s(dst01, src);
+}
+
+//===----------------------------------------------------------------------===//
+// memcpy() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_full_source_length(std::string src) {
+ char *dest20;
+ dest20 = reinterpret_cast<char *>(malloc(src.size()));
+ memcpy(dest20, src.data(), src.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: dest20 = reinterpret_cast<char *>(malloc(src.size() + 1));
+ // CHECK-FIXES-NEXT: strcpy(dest20, src.data());
+}
+
+void good_memcpy_full_source_length(std::string src) {
+ char dst20[14];
+ strcpy_s(dst20, src.data());
+}
+
+void bad_memcpy_partial_source_length(const char *src) {
+ char dest21[13];
+ memcpy(dest21, src, strlen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char dest21[14];
+ // CHECK-FIXES-NEXT: strncpy_s(dest21, src, strlen(src) - 1);
+}
+
+void good_memcpy_partial_source_length(const char *src) {
+ char dst21[14];
+ strncpy_s(dst21, src, strlen(src) - 1);
+}
+
+
+//===----------------------------------------------------------------------===//
+// memcpy_s() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_s_unknown_dest(char *dest40, const char *src) {
+ memcpy_s(dest40, 13, 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: strcpy_s(dest40, 13, src);
+}
+
+void good_memcpy_s_unknown_dest(char *dst40, const char *src) {
+ strcpy_s(dst40, 13, src);
+}
+
+void bad_memcpy_s_known_dest(const char *src) {
+ char dest41[13];
+ memcpy_s(dest41, 13, 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: char dest41[14];
+ // CHECK-FIXES: strcpy_s(dest41, src);
+}
+
+void good_memcpy_s_known_dest(const char *src) {
+ char dst41[14];
+ strcpy_s(dst41, src);
+}
+
+//===----------------------------------------------------------------------===//
+// memcpy_s() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_s_full_source_length(const char *src) {
+ char dest60[13];
+ memcpy_s(dest60, 13, 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: char dest60[14];
+ // CHECK-FIXES-NEXT: strcpy_s(dest60, src);
+}
+
+void good_memcpy_s_full_source_length(const char *src) {
+ char dst60[14];
+ strcpy_s(dst60, src);
+}
+
+void bad_memcpy_s_partial_source_length(const char *src) {
+ char dest61[13];
+ memcpy_s(dest61, 13, src, strlen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: char dest61[14];
+ // CHECK-FIXES-NEXT: strncpy_s(dest61, src, strlen(src) - 1);
+}
+
+void good_memcpy_s_partial_source_length(const char *src) {
+ char dst61[14];
+ strncpy_s(dst61, src, strlen(src) - 1);
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+#define __STDC_WANT_LIB_EXT1__ 0
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t strlen(const char *);
+void *malloc(size_t);
+
+char *strcpy(char *, const char *);
+void *memcpy(void *, const void *, size_t);
+
+
+//===----------------------------------------------------------------------===//
+// memcpy() - destination array tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dest00[13];
+ memcpy(dest00, 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: memcpy(dest00, src, strlen(src) + 1);
+}
+
+void good_memcpy_not_just_char_dest(const char *src) {
+ unsigned char dst00[13];
+ memcpy(dst00, src, strlen(src) + 1);
+}
+
+void bad_memcpy_known_dest(const char *src) {
+ char dest01[13];
+ memcpy(dest01, 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: strcpy(dest01, src);
+}
+
+void good_memcpy_known_dest(const char *src) {
+ char dst01[13];
+ strcpy(dst01, src);
+}
+
+//===----------------------------------------------------------------------===//
+// memcpy() - length tests
+//===----------------------------------------------------------------------===//
+
+void bad_memcpy_full_source_length(const char *src) {
+ char dest20[13];
+ memcpy(dest20, 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: strcpy(dest20, src);
+}
+
+void good_memcpy_full_source_length(const char *src) {
+ char dst20[13];
+ strcpy(dst20, src);
+}
+
+void bad_memcpy_partial_source_length(const char *src) {
+ char dest21[13];
+ memcpy(dest21, src, strlen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncpy(dest21, src, strlen(src) - 1);
+ // CHECK-FIXES-NEXT: dest21[strlen(src) - 1] = '\0';
+}
+
+void good_memcpy_partial_source_length(const char *src) {
+ char dst21[13];
+ strncpy(dst21, src, strlen(src) - 1);
+ dst21[strlen(src) - 1] = '\0';
+}
+
Index: test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c
@@ -0,0 +1,106 @@
+// 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);
+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 *memchr(const void *, int, size_t);
+void *memset(void *, int, size_t);
+
+int getLengthWithInc(const char *str) {
+ return strlen(str) + 1;
+}
+
+
+void bad_memchr(char *position, const char *src) {
+ int length = strlen(src);
+ position = (char *)memchr(src, '\0', length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: position = strchr(src, '\0');
+}
+
+void good_memchr(char *pos, const char *src) {
+ pos = strchr(src, '\0');
+}
+
+void bad_memset_1(const char *src) {
+ char dest[13];
+ memset(dest, '-', getLengthWithInc(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memset' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memset(dest, '-', getLengthWithInc(src) - 1);
+}
+
+void good_memset1(const char *src) {
+ char dst[13];
+ memset(dst, '-', getLengthWithInc(src) - 1);
+}
+
+void bad_strerror_s(int errno) {
+ char dest[13];
+ 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: char dest[14];
+ // CHECK-FIXES-NEXT: int length = strlen(strerror(errno));
+ // CHECK-FIXES-NEXT: strerror_s(dest, length + 1, errno);
+}
+
+void good_strerror_s(int errno) {
+ char dst[14];
+ int length = strlen(strerror(errno));
+ strerror_s(dst, 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(char *str1, const char *str2) {
+ int length = strlen(str1) + 1;
+ return strncmp(str1, str2, length - 1);
+}
+
+int bad_strncmp_2(char *str2) {
+ return strncmp(str2, "foobar", (strlen("foobar") + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str2, "foobar", strlen("foobar"));
+}
+
+int bad_strncmp_3(char *str3) {
+ return strncmp(str3, "foobar", 1 + strlen("foobar"));
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: comparison length is too long and might lead to a buffer overflow [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: strncmp(str3, "foobar", strlen("foobar"));
+}
+
+int good_strncmp_2_3(char *str) {
+ return strncmp(str, "foobar", strlen("foobar"));
+}
+
+void bad_strxfrm(const char *long_source_name) {
+ char long_destination_name[13];
+ 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: char long_destination_name[14];
+ // CHECK-FIXES-NEXT: int very_long_length_definition_name = strlen(long_source_name);
+ // CHECK-FIXES-NEXT: strxfrm(long_destination_name, long_source_name,
+ // CHECK-FIXES-NEXT: very_long_length_definition_name + 1);
+}
+
+void good_strxfrm(const char *long_source_name) {
+ char long_destination_name[14];
+ 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
@@ -44,6 +44,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,138 @@
+.. title:: clang-tidy - bugprone-not-null-terminated-result
+
+bugprone-not-null-terminated-result
+===================================
+
+Finds function calls where it is possible to cause a not null-terminated result.
+Usually the proper length of a string is ``strlen(src) + 1`` or equal length of
+this expression, because the null terminator needs an extra space. Without the
+null terminator it can result in an undefined behaviour when the string is read.
+
+The following function calls are checked:
+
+``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``. That is why the length
+of the allocated memory is problematic too.
+
+ .. code-block:: c
+
+ static char *StringCpy(const std::string &str) {
+ char *result = reinterpret_cast<char *>(malloc(str.size()));
+ memcpy(result, str.data(), str.size());
+ return result;
+ }
+
+In addition to issuing warnings, fix-it rewrites all the necessary code. If it
+is necessary, the buffer size will be increased to hold the null terminator.
+
+ .. code-block:: c
+
+ static char *StringCpy(const std::string &str) {
+ char *result = reinterpret_cast<char *>(malloc(str.size() + 1));
+ strcpy(result, str.data());
+ return result;
+ }
+
+.. _MemcpyTransformation:
+
+Transformation rules of 'memcpy()'
+----------------------------------
+
+It is possible to rewrite the ``memcpy()`` and ``memcpy_s()`` calls as the
+following four functions: ``strcpy()``, ``strncpy()``, ``strcpy_s()``,
+``strncpy_s()``, where the latter two are the safer versions of the former two.
+Respectively it is possible to rewrite ``wmemcpy()`` functions in the same way.
+
+Rewrite to a string handler function is not possible:
+
+- If the type of the destination array is not just ``char`` (``unsigned char``
+ or ``signed char``), that means the new function is cannot be any string
+ handler function. Fix-it adds ``+ 1`` to the given length of copy function.
+
+Rewrite based on the destination array:
+
+- If copy to the destination array cannot *overflow then the new function should
+ be the older copy function (ending with ``cpy``), because it is more
+ efficient than the safe version.
+
+- If copy to the destination array can *overflow and
+ ``AreSafeFunctionsAvailable`` is set to ``Yes``, ``y`` or non-zero and it is
+ possible to obtain the capacity of the destination array then the new function
+ could be the safe version (ending with ``cpy_s``).
+
+- If the new function is could be safe version and C++ files are analysed then
+ the length of the destination array can be omitted.
+
+- *It is possible to overflow:
+ - Unknown the capacity of the destination array.
+ - If the given length is equal to the destination capacity.
+
+Rewrite based on the length of the source string:
+
+- If the given length is ``strlen(source)`` or equal length of this expression
+ then the new function should be the older copy function (ending with ``cpy``),
+ as it is more efficient than the safe version.
+
+- Otherwise we assume that the programmer wanted to copy `n` characters, so the
+ new function is ``ncpy``-like which is could be safe.
+
+Transformations with 'strlen()' or equal length of this expression
+------------------------------------------------------------------
+
+In general, the following transformations are could happen:
+
+(Note: If a wide-character handler function exists of the following functions
+it handled in the same way.)
+
+Memory handler functions
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+- ``memcpy``: See in the
+ :ref:`Transformation rules of 'memcpy()'<MemcpyTransformation>` section.
+
+- ``memchr``:
+ - Usually there is a C-style cast, and it is needed to be removed, because the
+ new function ``strchr``'s return type is correct.
+ - Also the given length is not needed in the new function.
+
+- ``memmove``:
+ - If safe functions are available the new function is ``memmove_s``, it has
+ four arguments:
+ - destination array,
+ - length of the destination array,
+ - source string,
+ - length of the source string which is incremented by one.
+ - If safe functions are not available the given length is incremented by one.
+
+- ``memmove_s``: given length is incremented by one.
+
+- ``memset``: given length has to be truncated without the ``+ 1``.
+
+String handler functions
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+- ``strerror_s``: given length is incremented by one.
+
+- ``strncmp``: 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``: given length is incremented by one.
+
+Options
+-------
+
+.. option:: AreSafeFunctionsAvailable
+
+ A string specifying if the target environment is considered to implement
+ ``_s`` suffixed memory and string handler functions which are safer than
+ older version (e.g. ``memcpy_s()``):
+ - If ``Default``, ``d``, or ``""`` (empty string) the checker relies on the
+ ``__STDC_WANT_LIB_EXT1__`` macro being defined.
+ - If ``Yes``, ``y``, or non-zero the ``_s`` suffixed functions are available.
+ - If ``No``, ``n``, or ``0`` (zero) the ``_s`` suffixed functions are not
+ available.
+ - The default value is ``""`` (empty string).
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,15 @@
Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
``absl::StrAppend()`` should be used instead.
+- New :doc:`bugprone-not-null-terminated-result
+ <clang-tidy/checks/bugprone-not-null-terminated-result>` check
+
+ Finds function calls where it is possible to cause a not null-terminated
+ result. Usually the proper length of a string is ``strlen(src) + 1`` or equal
+ length of this expression, because the null terminator needs an extra space.
+ Without the null terminator it can result in an undefined behaviour when the
+ string is read.
+
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -0,0 +1,73 @@
+//===--- 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 it is possible to cause a not null-terminated
+/// result. Usually the proper length of a string is ``strlen(src) + 1`` or
+/// equal length of this expression, because the null terminator needs an extra
+/// space. Without the null terminator it can result in an undefined behaviour
+/// when the string is read.
+///
+/// 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;
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+
+private:
+ enum class SafeFunctionsAvailableKind { Default, No, Yes };
+ SafeFunctionsAvailableKind SafeFunctionsAvailable;
+
+ // It is specifying if the target environment is considered to implement
+ // '_s' suffixed memory and string handler functions which are safer than
+ // older version (e.g. 'memcpy_s()'):
+ // - If "Default", "d", or "" (empty string) the checker relies on the
+ // '__STDC_WANT_LIB_EXT1__' macro being defined.
+ // - If "Yes", "y", or non-zero the '_s' suffixed functions are available.
+ // - If "No", "n", or 0 (zero) the '_s' suffixed functions are not available.
+ // - The default value is ``""`` (empty string).
+ const std::string AreSafeFunctionsAvailable;
+
+ void memoryHandlerFunctionFix(
+ 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);
+ void memmoveFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+ 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);
+};
+
+} // 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,1054 @@
+//===--- 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/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PPCallbacks.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 UnknownDestName = "destination-length-is-unknown";
+static const char *const NotJustCharTyName = "unsigned-or-signed-char";
+static const char *const DestArrayTyName = "destination-is-array-type";
+static const char *const DestVarDeclName = "destination-variable-declaration";
+static const char *const SrcVarDeclName = "source-variable-declaration";
+static const char *const UnknownLengthName = "given-length-is-unknown";
+static const char *const WrongLengthExprName = "strlen-or-size";
+static const char *const DestMallocExprName = "destination-malloc-expr";
+static const char *const DestExprName = "destination-decl-ref-expr";
+static const char *const SrcExprName = "source-expression-or-string-literal";
+static const char *const LengthExprName = "given-length-expression";
+
+enum class LengthHandleKind { LHK_Increase, LHK_Decrease };
+
+namespace {
+
+// Increment by integer '1' can result in overflow if it is the maximal value.
+// After that it will be extended to 'size_t' and its value will be wrong,
+// therefore we have to inject '+ 1UL' instead.
+static bool IsInjectUL = false;
+static std::map<StringRef, int> MacroDefinedMap;
+
+class SimpleMacroPPCallbacks : public PPCallbacks {
+public:
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override;
+};
+} // namespace
+
+// The destination array is where the result of the function call will be stored
+static const Expr *getDestExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(DestExprName);
+}
+
+// The source array is where the data come from
+static const Expr *getSrcExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(SrcExprName);
+}
+
+// The the given length (e.g. 'strlen(src)')
+static const Expr *getLengthExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(LengthExprName);
+}
+
+// The capacity: VariableArrayType, ConstantArrayType, argument of a 'malloc()'
+// family function or an argument of a custom memory allocation.
+static const Expr *getDestCapacityExpr(const MatchFinder::MatchResult &Result);
+
+static int getDestCapacity(const MatchFinder::MatchResult &Result);
+
+// Length is could be an IntegerLiteral or length of a StringLiteral
+static int getLength(const Expr *E, const MatchFinder::MatchResult &Result);
+
+static int getGivenLength(const MatchFinder::MatchResult &Result);
+
+static StringRef exprToStr(const Expr *E,
+ const MatchFinder::MatchResult &Result);
+
+static SourceLocation exprLocEnd(const Expr *E,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getLocForEndOfToken(E->getEndLoc(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+}
+
+//===---------------------------------------------------------------------===//
+// Rewrite decision helper functions
+//===---------------------------------------------------------------------===//
+
+// If the capacity of the destination array is not known it denoted as unknown
+static bool isKnownDest(const MatchFinder::MatchResult &Result) {
+ return !Result.Nodes.getNodeAs<Expr>(UnknownDestName);
+}
+
+// True if the capacity of the destination array is based on the given length,
+// therefore it looks like it cannot overflow (e.g. 'malloc(given_length + 1)'
+// Note: If the capacity and the given length is equal then the new function
+// is a simple 'cpy()' and because it returns true it prevents to increase
+// the given length.
+static bool
+isDestCapacityBasedOnGivenLength(const MatchFinder::MatchResult &Result);
+
+// If we write/read from the same array it should be already null-terminated
+static bool isDestAndSrcEquals(const MatchFinder::MatchResult &Result);
+
+// We catch integers as a given length so we have to see if the length of the
+// source array is the same length and that is not null-terminated
+static bool isNoWrongLength(const MatchFinder::MatchResult &Result);
+
+// Example: memcpy(dest, str.data(), str.length());
+static bool isStringDataAndLength(const MatchFinder::MatchResult &Result);
+
+static bool isDestCapacityOverflows(const MatchFinder::MatchResult &Result);
+
+static bool isGivenLengthEQToSrcLength(const MatchFinder::MatchResult &Result);
+
+//===---------------------------------------------------------------------===//
+// Code injection functions
+//===---------------------------------------------------------------------===//
+
+static void lengthDecrease(const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+static void lengthIncrease(const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+// Increase or decrease by one an expression
+static void lengthExprHandle(LengthHandleKind LengthHandle,
+ const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+// Increase or decrease by one a passed argument by its position
+static void lengthArgHandle(LengthHandleKind LengthHandle, int ArgPos,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+// If the destination array is the same length as the given length we have to
+// increase the capacity by one to create space for the the null terminator
+static bool destCapacityFix(const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+static void removeArg(int ArgPos, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+static void renameFunc(StringRef NewFuncName,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+static void insertDestCapacityArg(bool IsOverflows, StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+static void insertNullTerminatorExpr(StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag);
+
+NotNullTerminatedResultCheck::NotNullTerminatedResultCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ AreSafeFunctionsAvailable(Options.get("AreSafeFunctionsAvailable", "")) {
+ if (AreSafeFunctionsAvailable == "" || AreSafeFunctionsAvailable[0] == 'd' ||
+ AreSafeFunctionsAvailable[0] == 'D')
+ SafeFunctionsAvailable = SafeFunctionsAvailableKind::Default;
+ else if (AreSafeFunctionsAvailable[0] == 'y' ||
+ AreSafeFunctionsAvailable[0] == 'Y')
+ SafeFunctionsAvailable = SafeFunctionsAvailableKind::Yes;
+ else if (AreSafeFunctionsAvailable[0] == 'n' ||
+ AreSafeFunctionsAvailable[0] == 'N')
+ SafeFunctionsAvailable = SafeFunctionsAvailableKind::No;
+ else
+ SafeFunctionsAvailable = atoi(AreSafeFunctionsAvailable.c_str())
+ ? SafeFunctionsAvailableKind::Yes
+ : SafeFunctionsAvailableKind::No;
+}
+
+void NotNullTerminatedResultCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AreSafeFunctionsAvailable", AreSafeFunctionsAvailable);
+}
+
+void NotNullTerminatedResultCheck::registerPPCallbacks(
+ CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ llvm::make_unique<SimpleMacroPPCallbacks>());
+}
+
+namespace {
+AST_MATCHER_P(Expr, hasDefinition, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ const Expr *SimpleNode = &Node;
+ SimpleNode = SimpleNode->IgnoreParenCasts()->IgnoreImpCasts();
+
+ if (InnerMatcher.matches(*SimpleNode, Finder, Builder))
+ return true;
+
+ const auto DREHasInit = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(ignoringImpCasts(InnerMatcher))))));
+
+ if (DREHasInit.matches(*SimpleNode, Finder, Builder))
+ return true;
+
+ // - Example: int getLength(const char *str) { return strlen(str); }
+ const auto CallExprReturnInit = ignoringImpCasts(
+ callExpr(callee(functionDecl(hasBody(has(returnStmt(hasReturnValue(
+ ignoringImpCasts(anyOf(DREHasInit, InnerMatcher))))))))));
+
+ if (CallExprReturnInit.matches(*SimpleNode, Finder, Builder))
+ return true;
+
+ // - Example: int length = getLength(src);
+ const auto DREHasReturnInit = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(CallExprReturnInit)))));
+
+ if (DREHasReturnInit.matches(*SimpleNode, Finder, Builder))
+ return true;
+
+ const char *const VarDeclName = "variable-declaration";
+ const auto DREHasDefinition = ignoringImpCasts(declRefExpr(
+ allOf(to(varDecl().bind(VarDeclName)),
+ hasAncestor(compoundStmt(hasDescendant(binaryOperator(
+ hasLHS(declRefExpr(to(varDecl(equalsBoundNode(VarDeclName))))),
+ hasRHS(ignoringImpCasts(InnerMatcher)))))))));
+
+ if (DREHasDefinition.matches(*SimpleNode, Finder, Builder))
+ return true;
+
+ return false;
+}
+} // namespace
+
+void NotNullTerminatedResultCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IncOp =
+ binaryOperator(hasOperatorName("+"),
+ hasEitherOperand(ignoringParenImpCasts(integerLiteral())));
+
+ const auto DecOp =
+ binaryOperator(hasOperatorName("-"),
+ hasEitherOperand(ignoringParenImpCasts(integerLiteral())));
+
+ const auto HasIncOp = anyOf(ignoringImpCasts(IncOp), hasDescendant(IncOp));
+ const auto HasDecOp = anyOf(ignoringImpCasts(DecOp), hasDescendant(DecOp));
+
+ const auto StringTy = type(hasUnqualifiedDesugaredType(recordType(
+ hasDeclaration(cxxRecordDecl(hasName("::std::basic_string"))))));
+
+ const auto AnyOfStringTy =
+ anyOf(hasType(StringTy), hasType(qualType(pointsTo(StringTy))));
+
+ const auto CharTy =
+ anyOf(asString("char"), asString("wchar_t"),
+ allOf(anyOf(asString("unsigned char"), asString("signed char")),
+ type().bind(NotJustCharTyName)));
+
+ const auto CharTyArray = hasType(qualType(hasCanonicalType(
+ arrayType(hasElementType(CharTy)).bind(DestArrayTyName))));
+
+ const auto CharTyPointer =
+ hasType(qualType(hasCanonicalType(pointerType(pointee(CharTy)))));
+
+ const auto AnyOfCharTy = anyOf(CharTyArray, CharTyPointer);
+
+ //===--------------------------------------------------------------------===//
+ // The following six case match problematic length expressions
+ //===--------------------------------------------------------------------===//
+
+ // - Example: char src[] = "foo"; strlen(src);
+ const auto Strlen =
+ callExpr(callee(functionDecl(hasAnyName("::strlen", "::wcslen"))))
+ .bind(WrongLengthExprName);
+
+ // - Example: std::string str = "foo"; str.size();
+ const auto SizeOrLength =
+ cxxMemberCallExpr(
+ allOf(on(expr(AnyOfStringTy)),
+ has(memberExpr(member(hasAnyName("size", "length"))))))
+ .bind(WrongLengthExprName);
+
+ // - Example: char src[] = "foo"; sizeof(src);
+ const auto SizeOfCharExpr =
+ unaryExprOrTypeTraitExpr(has(expr(hasType(qualType(
+ hasCanonicalType(anyOf(arrayType(hasElementType(isAnyCharacter())),
+ pointerType(pointee(isAnyCharacter())))))))));
+
+ const auto WrongLength =
+ anyOf(ignoringImpCasts(Strlen), ignoringImpCasts(SizeOrLength),
+ hasDescendant(Strlen), hasDescendant(SizeOrLength));
+
+ // - Example: length = strlen(src);
+ const auto DREWithoutInc =
+ ignoringImpCasts(declRefExpr(to(varDecl(hasInitializer(WrongLength)))));
+
+ const auto AnyOfCallOrDREWithoutInc = anyOf(DREWithoutInc, WrongLength);
+
+ // - Example: int getLength(const char *str) { return strlen(str); }
+ const auto CallExprReturnWithoutInc =
+ ignoringImpCasts(callExpr(callee(functionDecl(hasBody(
+ has(returnStmt(hasReturnValue(AnyOfCallOrDREWithoutInc))))))));
+
+ // - Example: int length = getLength(src);
+ const auto DREHasReturnWithoutInc = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(CallExprReturnWithoutInc)))));
+
+ const auto AnyOfWrongLengthInit =
+ anyOf(AnyOfCallOrDREWithoutInc, CallExprReturnWithoutInc,
+ DREHasReturnWithoutInc);
+
+ enum class StrlenKind { WithInc, WithoutInc };
+
+ const auto AnyOfLengthExpr = [=](StrlenKind LengthKind) {
+ return ignoringImpCasts(allOf(
+ unless(hasDefinition(SizeOfCharExpr)),
+ anyOf(allOf((LengthKind == StrlenKind::WithoutInc)
+ ? ignoringImpCasts(unless(hasDefinition(HasIncOp)))
+ : ignoringImpCasts(
+ allOf(hasDefinition(HasIncOp),
+ unless(hasDefinition(HasDecOp)))),
+ AnyOfWrongLengthInit),
+ ignoringImpCasts(integerLiteral().bind(WrongLengthExprName))),
+ expr().bind(LengthExprName)));
+ };
+
+ const auto LengthWithoutInc = AnyOfLengthExpr(StrlenKind::WithoutInc);
+ const auto LengthWithInc = AnyOfLengthExpr(StrlenKind::WithInc);
+
+ //===--------------------------------------------------------------------===//
+ // The following five case match the 'destination' array length's
+ // expression which is used in memcpy() and memmove() matchers.
+ //===--------------------------------------------------------------------===//
+
+ const auto SizeExpr = anyOf(SizeOfCharExpr, integerLiteral(equals(1)));
+
+ const auto MallocLengthExpr = allOf(
+ anyOf(argumentCountIs(1), argumentCountIs(2)),
+ hasAnyArgument(allOf(unless(SizeExpr),
+ expr(ignoringImpCasts(anyOf(HasIncOp, anything())))
+ .bind(DestMallocExprName))));
+
+ // - Example: (char *)malloc(length);
+ const auto DestMalloc = anyOf(castExpr(has(callExpr(MallocLengthExpr))),
+ callExpr(MallocLengthExpr));
+
+ // - Example: new char[length];
+ const auto DestCXXNewExpr = ignoringImpCasts(
+ cxxNewExpr(hasArraySize(expr().bind(DestMallocExprName))));
+
+ const auto AnyOfDestInit = anyOf(DestMalloc, DestCXXNewExpr);
+
+ // - Example: char dest[13]; or char dest[length];
+ const auto DestArrayTyDecl = declRefExpr(
+ to(anyOf(varDecl(CharTyArray).bind(DestVarDeclName),
+ varDecl(hasInitializer(AnyOfDestInit)).bind(DestVarDeclName))));
+
+ // - Example: foo[bar[baz]].qux; (or just ParmVarDecl)
+ const auto DestUnknownDecl =
+ declRefExpr(allOf(to(varDecl(AnyOfCharTy).bind(DestVarDeclName)),
+ expr().bind(UnknownDestName)));
+
+ const auto AnyOfDestDecl =
+ allOf(anyOf(hasDefinition(anyOf(AnyOfDestInit, DestArrayTyDecl)),
+ DestUnknownDecl, anything()),
+ expr().bind(DestExprName));
+
+ const auto SrcDecl = declRefExpr(
+ allOf(to(decl().bind(SrcVarDeclName)),
+ anyOf(hasAncestor(cxxMemberCallExpr().bind(SrcExprName)),
+ expr().bind(SrcExprName))));
+
+ const auto SrcDeclMayInBinOp =
+ anyOf(ignoringImpCasts(SrcDecl), hasDescendant(SrcDecl));
+
+ const auto AnyOfSrcDecl = anyOf(
+ ignoringImpCasts(stringLiteral().bind(SrcExprName)), SrcDeclMayInBinOp);
+
+ const auto NullTerminatorExpr = binaryOperator(
+ hasLHS(hasDescendant(
+ declRefExpr(to(varDecl(equalsBoundNode(DestVarDeclName)))))),
+ hasRHS(ignoringImpCasts(
+ anyOf(characterLiteral(equals(static_cast<unsigned>(0))),
+ integerLiteral(equals(0))))));
+
+ //===--------------------------------------------------------------------===//
+ // The following nineteen case match problematic function calls.
+ //===--------------------------------------------------------------------===//
+
+ const auto WithoutSrc = [=](StringRef Name, int LengthPos,
+ StrlenKind LengthKind) {
+ return allOf(
+ callee(functionDecl(hasName(Name))),
+ hasArgument(
+ 0, allOf(AnyOfDestDecl, unless(hasAncestor(compoundStmt(
+ hasDescendant(NullTerminatorExpr)))))),
+ hasArgument(LengthPos, (LengthKind == StrlenKind::WithoutInc)
+ ? LengthWithoutInc
+ : LengthWithInc));
+ };
+
+ const auto WithSrc = [=](StringRef Name, int SourcePos, int LengthPos,
+ StrlenKind LengthKind) {
+ return allOf(callee(functionDecl(hasName(Name))),
+ hasArgument(SourcePos ? 0 : 1,
+ allOf(AnyOfDestDecl,
+ unless(hasAncestor(compoundStmt(
+ hasDescendant(NullTerminatorExpr)))))),
+ hasArgument(SourcePos, AnyOfSrcDecl),
+ hasArgument(LengthPos, (LengthKind == StrlenKind::WithoutInc)
+ ? LengthWithoutInc
+ : LengthWithInc));
+ };
+
+ const auto Memcpy = WithSrc("::memcpy", 1, 2, StrlenKind::WithoutInc);
+ const auto Wmemcpy = WithSrc("::wmemcpy", 1, 2, StrlenKind::WithoutInc);
+ const auto Memcpy_s = WithSrc("::memcpy_s", 2, 3, StrlenKind::WithoutInc);
+ const auto Wmemcpy_s = WithSrc("::wmemcpy_s", 2, 3, StrlenKind::WithoutInc);
+ const auto Memchr = WithSrc("::memchr", 0, 2, StrlenKind::WithoutInc);
+ const auto Wmemchr = WithSrc("::wmemchr", 0, 2, StrlenKind::WithoutInc);
+ const auto Memmove = WithSrc("::memmove", 1, 2, StrlenKind::WithoutInc);
+ const auto Wmemmove = WithSrc("::wmemmove", 1, 2, StrlenKind::WithoutInc);
+ const auto Memmove_s = WithSrc("::memmove_s", 2, 3, StrlenKind::WithoutInc);
+ const auto Wmemmove_s = WithSrc("::wmemmove_s", 2, 3, StrlenKind::WithoutInc);
+ const auto Memset = WithoutSrc("::memset", 2, StrlenKind::WithInc);
+ const auto Wmemset = WithoutSrc("::wmemset", 2, StrlenKind::WithInc);
+ const auto Strerror_s = WithoutSrc("::strerror_s", 1, StrlenKind::WithoutInc);
+ const auto StrncmpLHS = WithSrc("::strncmp", 1, 2, StrlenKind::WithInc);
+ const auto WcsncmpLHS = WithSrc("::wcsncmp", 1, 2, StrlenKind::WithInc);
+ const auto StrncmpRHS = WithSrc("::strncmp", 0, 2, StrlenKind::WithInc);
+ const auto WcsncmpRHS = WithSrc("::wcsncmp", 0, 2, StrlenKind::WithInc);
+ const auto Strxfrm = WithSrc("::strxfrm", 1, 2, StrlenKind::WithoutInc);
+ const auto Wcsxfrm = WithSrc("::wcsxfrm", 1, 2, StrlenKind::WithoutInc);
+
+ const auto AnyOfMatchers =
+ anyOf(Memcpy, Wmemcpy, Memcpy_s, Wmemcpy_s, Memchr, Wmemchr, Memmove,
+ Wmemmove, Memmove_s, Wmemmove_s, Memset, Wmemset, Strerror_s,
+ StrncmpLHS, WcsncmpLHS, StrncmpRHS, WcsncmpRHS, Strxfrm, Wcsxfrm);
+
+ Finder->addMatcher(callExpr(AnyOfMatchers).bind(FuncExprName), this);
+
+ Finder->addMatcher(
+ castExpr(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->getBeginLoc().isMacroID())
+ return;
+
+ IsInjectUL = getGivenLength(Result) == std::numeric_limits<int>::max();
+
+ if (SafeFunctionsAvailable == SafeFunctionsAvailableKind::Default) {
+ const auto It = MacroDefinedMap.find("__STDC_WANT_LIB_EXT1__");
+ if (It != MacroDefinedMap.end())
+ SafeFunctionsAvailable = It->second ? SafeFunctionsAvailableKind::Yes
+ : SafeFunctionsAvailableKind::No;
+ else
+ SafeFunctionsAvailable = SafeFunctionsAvailableKind::No;
+ }
+
+ StringRef Name = FuncExpr->getDirectCallee()->getName();
+ if (Name.startswith("mem") || Name.startswith("wmem"))
+ memoryHandlerFunctionFix(Name, Result);
+ else 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::memoryHandlerFunctionFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (isNoWrongLength(Result))
+ return;
+
+ if (Name.endswith("chr")) {
+ memchrFix(Name, Result);
+ return;
+ }
+
+ if ((Name.contains("cpy") || Name.contains("move")) &&
+ isDestAndSrcEquals(Result))
+ return;
+
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>(FuncExprName)->getBeginLoc(),
+ "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("move"))
+ memmoveFix(Name, Result, Diag);
+ else if (Name.endswith("move_s")) {
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 3, Result, Diag);
+ } else if (Name.endswith("set")) {
+ lengthArgHandle(LengthHandleKind::LHK_Decrease, 2, Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::memcpyFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool IsOverflows = destCapacityFix(Result, Diag);
+
+ // If it is cannot be rewritten to string handler function
+ if (Result.Nodes.getNodeAs<Type>(NotJustCharTyName)) {
+ if (SafeFunctionsAvailable == SafeFunctionsAvailableKind::Yes &&
+ isKnownDest(Result)) {
+ renameFunc((Name[0] != 'w') ? "memcpy_s" : "wmemcpy_s", Result, Diag);
+ insertDestCapacityArg(IsOverflows, Name, Result, Diag);
+ }
+
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 2, Result, Diag);
+ return;
+ }
+
+ const bool IsCpy = isGivenLengthEQToSrcLength(Result) ||
+ isDestCapacityBasedOnGivenLength(Result);
+ const bool IsSafe =
+ SafeFunctionsAvailable == SafeFunctionsAvailableKind::Yes &&
+ IsOverflows && isKnownDest(Result) &&
+ !isDestCapacityBasedOnGivenLength(Result);
+ const bool IsDestLengthNotRequired =
+ IsSafe && getLangOpts().CPlusPlus &&
+ Result.Nodes.getNodeAs<ArrayType>(DestArrayTyName);
+
+ SmallString<10> NewFuncName;
+ NewFuncName = (Name[0] != 'w') ? "str" : "wcs";
+ NewFuncName += IsCpy ? "cpy" : "ncpy";
+ NewFuncName += IsSafe ? "_s" : "";
+ renameFunc(NewFuncName, Result, Diag);
+
+ if (IsSafe && !IsDestLengthNotRequired)
+ insertDestCapacityArg(IsOverflows, Name, Result, Diag);
+
+ if (IsCpy)
+ removeArg(2, Result, Diag);
+
+ if (!IsCpy && !IsSafe)
+ insertNullTerminatorExpr(Name, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memcpy_sFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool IsOverflows = destCapacityFix(Result, Diag);
+
+ if (Result.Nodes.getNodeAs<Type>(NotJustCharTyName)) {
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 3, Result, Diag);
+ return;
+ }
+
+ const bool RemoveDestLength =
+ getLangOpts().CPlusPlus &&
+ Result.Nodes.getNodeAs<ArrayType>(DestArrayTyName);
+ const bool IsCpy = isGivenLengthEQToSrcLength(Result);
+ const bool IsSafe = IsOverflows;
+
+ SmallString<10> NewFuncName;
+ NewFuncName = (Name[0] != 'w') ? "str" : "wcs";
+ NewFuncName += IsCpy ? "cpy" : "ncpy";
+ NewFuncName += IsSafe ? "_s" : "";
+ renameFunc(NewFuncName, Result, Diag);
+
+ if (!IsSafe || (IsSafe && RemoveDestLength))
+ removeArg(1, Result, Diag);
+ else if (IsOverflows && isKnownDest(Result))
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 1, Result, Diag);
+
+ if (IsCpy)
+ removeArg(3, Result, Diag);
+
+ if (!IsCpy && !IsSafe)
+ insertNullTerminatorExpr(Name, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::memchrFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ if (const auto GivenCL =
+ dyn_cast_or_null<CharacterLiteral>(FuncExpr->getArg(1)))
+ if (GivenCL->getValue() != 0)
+ return;
+
+ auto Diag = diag(FuncExpr->getArg(2)->IgnoreParenCasts()->getBeginLoc(),
+ "the length is too short for the last %0")
+ << ((Name[0] != 'w') ? "\'\\0\'" : "L\'\\0\'");
+
+ if (const auto *CastExpr = Result.Nodes.getNodeAs<Expr>(CastExprName)) {
+ const auto CastRemoveFix = FixItHint::CreateRemoval(SourceRange(
+ CastExpr->getBeginLoc(), FuncExpr->getBeginLoc().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) {
+ const bool IsOverflows = destCapacityFix(Result, Diag);
+
+ if (SafeFunctionsAvailable == SafeFunctionsAvailableKind::Yes &&
+ isKnownDest(Result)) {
+ renameFunc((Name[0] != 'w') ? "memmove_s" : "wmemmove_s", Result, Diag);
+ insertDestCapacityArg(IsOverflows, Name, Result, Diag);
+ }
+
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 2, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::strerror_sFix(
+ const MatchFinder::MatchResult &Result) {
+ StringRef Name = "strerror_s";
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>(FuncExprName)->getBeginLoc(),
+ "the result from calling '%0' is not null-terminated and "
+ "missing the last character of the error message")
+ << Name;
+
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 1, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::ncmpFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const auto *FirstArgExpr = FuncExpr->getArg(0)->IgnoreImpCasts();
+ const auto *SecondArgExpr = FuncExpr->getArg(1)->IgnoreImpCasts();
+ bool IsLengthTooLong = false;
+
+ if (const auto *LengthExpr =
+ Result.Nodes.getNodeAs<CallExpr>(WrongLengthExprName)) {
+ const auto *LengthExprArg = LengthExpr->getArg(0);
+ StringRef FirstExprStr = exprToStr(FirstArgExpr, Result).trim(' ');
+ StringRef SecondExprStr = exprToStr(SecondArgExpr, Result).trim(' ');
+ StringRef LengthArgStr = exprToStr(LengthExprArg, Result).trim(' ');
+ IsLengthTooLong =
+ LengthArgStr == FirstExprStr || LengthArgStr == SecondExprStr;
+ } else {
+ const int SrcLength = getLength(getSrcExpr(Result), Result);
+ const int GivenLength = getGivenLength(Result);
+ IsLengthTooLong = GivenLength - 1 == SrcLength;
+ }
+
+ if (!IsLengthTooLong && !isStringDataAndLength(Result))
+ return;
+
+ auto Diag = diag(FuncExpr->getArg(2)->IgnoreParenCasts()->getBeginLoc(),
+ "comparison length is too long and might lead to a "
+ "buffer overflow");
+
+ lengthArgHandle(LengthHandleKind::LHK_Decrease, 2, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::xfrmFix(
+ StringRef Name, const MatchFinder::MatchResult &Result) {
+ if (!isDestCapacityOverflows(Result))
+ return;
+
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>(FuncExprName)->getBeginLoc(),
+ "the result from calling '%0' is not null-terminated")
+ << Name;
+
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LHK_Increase, 2, Result, Diag);
+}
+
+//===---------------------------------------------------------------------===//
+// Helper functions
+//===---------------------------------------------------------------------===//
+
+void SimpleMacroPPCallbacks::MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) {
+ const MacroInfo *MI = MD->getMacroInfo();
+ if (MI->isBuiltinMacro())
+ return;
+
+ if (!MI->tokens().size())
+ return;
+
+ const auto &T = MI->tokens().back();
+ if (T.isLiteral()) {
+ StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+ llvm::APInt IntValue;
+ ValueStr.getAsInteger(10, IntValue);
+
+ StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
+ MacroDefinedMap[MacroName] = IntValue.getZExtValue();
+ }
+}
+
+static StringRef exprToStr(const Expr *E,
+ const MatchFinder::MatchResult &Result) {
+ if (!E)
+ return "";
+
+ return Lexer::getSourceText(
+ CharSourceRange::getTokenRange(E->getSourceRange()),
+ *Result.SourceManager, Result.Context->getLangOpts(), 0);
+}
+
+static bool isDestAndSrcEquals(const MatchFinder::MatchResult &Result) {
+ if (const auto DestVD = Result.Nodes.getNodeAs<Decl>(DestVarDeclName))
+ if (const auto *SrcVD = Result.Nodes.getNodeAs<Decl>(SrcVarDeclName))
+ return DestVD->getCanonicalDecl() == SrcVD->getCanonicalDecl();
+
+ return false;
+}
+
+static bool isNoWrongLength(const MatchFinder::MatchResult &Result) {
+ if (Result.Nodes.getNodeAs<IntegerLiteral>(WrongLengthExprName))
+ return !isGivenLengthEQToSrcLength(Result);
+
+ return false;
+}
+
+static const Expr *getDestCapacityExpr(const MatchFinder::MatchResult &Result) {
+ if (const auto *DestMalloc = Result.Nodes.getNodeAs<Expr>(DestMallocExprName))
+ return DestMalloc;
+
+ if (const auto DestTy = Result.Nodes.getNodeAs<ArrayType>(DestArrayTyName))
+ if (const auto *DestVAT = dyn_cast_or_null<VariableArrayType>(DestTy))
+ return DestVAT->getSizeExpr();
+
+ if (const auto *DestVD = Result.Nodes.getNodeAs<VarDecl>(DestVarDeclName))
+ if (const auto DestTL = DestVD->getTypeSourceInfo()->getTypeLoc())
+ if (const auto DestCTL = DestTL.getAs<ConstantArrayTypeLoc>())
+ return DestCTL.getSizeExpr();
+
+ return nullptr;
+}
+
+static int getLength(const Expr *E, const MatchFinder::MatchResult &Result) {
+ llvm::APSInt Length;
+
+ if (const auto *LengthDRE = dyn_cast_or_null<DeclRefExpr>(E))
+ if (const auto *LengthVD = dyn_cast_or_null<VarDecl>(LengthDRE->getDecl()))
+ if (!isa<ParmVarDecl>(LengthVD))
+ if (const auto *LengthInit = LengthVD->getInit())
+ if (LengthInit->EvaluateAsInt(Length, *Result.Context))
+ return Length.getZExtValue();
+
+ if (const auto *LengthIL = dyn_cast_or_null<IntegerLiteral>(E))
+ return LengthIL->getValue().getZExtValue();
+
+ if (const auto *StrDRE = dyn_cast_or_null<DeclRefExpr>(E))
+ if (const auto *StrVD = dyn_cast_or_null<VarDecl>(StrDRE->getDecl()))
+ if (const auto *StrInit = StrVD->getInit())
+ if (const auto *StrSL =
+ dyn_cast_or_null<StringLiteral>(StrInit->IgnoreImpCasts()))
+ return StrSL->getLength();
+
+ if (const auto *SrcSL = dyn_cast_or_null<StringLiteral>(E))
+ return SrcSL->getLength();
+
+ return 0;
+}
+
+static int getDestCapacity(const MatchFinder::MatchResult &Result) {
+ if (const auto *DestCapacityExpr = getDestCapacityExpr(Result))
+ return getLength(DestCapacityExpr, Result);
+
+ return 0;
+}
+
+static int getGivenLength(const MatchFinder::MatchResult &Result) {
+ const auto *LengthExpr = Result.Nodes.getNodeAs<Expr>(LengthExprName);
+ if (const int Length = getLength(LengthExpr, Result))
+ return Length;
+
+ if (const auto *Strlen = dyn_cast_or_null<CallExpr>(LengthExpr))
+ if (Strlen->getNumArgs() > 0)
+ if (const auto *StrlenArg = Strlen->getArg(0)->IgnoreImpCasts())
+ if (const int StrlenArgLength = getLength(StrlenArg, Result))
+ return StrlenArgLength;
+
+ return 0;
+}
+
+static bool isStringDataAndLength(const MatchFinder::MatchResult &Result) {
+ StringRef DestStr = exprToStr(getDestExpr(Result), Result);
+ StringRef SrcStr = exprToStr(getSrcExpr(Result), Result);
+ StringRef GivenLengthStr = exprToStr(getLengthExpr(Result), Result);
+
+ const bool ProblematicLength =
+ GivenLengthStr.contains(".size") || GivenLengthStr.contains(".length");
+
+ return ProblematicLength &&
+ (SrcStr.contains(".data") || DestStr.contains(".data"));
+}
+
+static bool isGivenLengthEQToSrcLength(const MatchFinder::MatchResult &Result) {
+ if (isStringDataAndLength(Result))
+ return true;
+
+ const int GivenLength = getGivenLength(Result);
+
+ // It is the length without the null terminator.
+ const int SrcLength = getLength(getSrcExpr(Result), Result);
+
+ if (GivenLength != 0 && GivenLength == SrcLength)
+ return true;
+
+ // If 'strlen()' check if the VarDecl of the argument is equal to SrcVarDecl.
+ if (const auto *StrlenExpr = Result.Nodes.getNodeAs<CallExpr>(LengthExprName))
+ if (StrlenExpr->getNumArgs() > 0)
+ if (const auto *StrlenDRE = dyn_cast_or_null<DeclRefExpr>(
+ StrlenExpr->getArg(0)->IgnoreImpCasts()))
+ return dyn_cast_or_null<VarDecl>(StrlenDRE->getDecl()) ==
+ Result.Nodes.getNodeAs<VarDecl>(SrcVarDeclName);
+
+ return false;
+}
+
+static bool isDestCapacityOverflows(const MatchFinder::MatchResult &Result) {
+ if (!isKnownDest(Result))
+ return true;
+
+ const auto *DestCapacityExpr = getDestCapacityExpr(Result);
+ const auto *LengthExpr = getLengthExpr(Result);
+ const int DestCapacity = getLength(DestCapacityExpr, Result);
+ const int GivenLength = getGivenLength(Result);
+
+ if (GivenLength != 0 && DestCapacity != 0)
+ return isGivenLengthEQToSrcLength(Result) && DestCapacity == GivenLength;
+
+ StringRef DestCapacityExprStr = exprToStr(DestCapacityExpr, Result);
+ StringRef LengthExprStr = exprToStr(LengthExpr, Result);
+
+ // Assume that it cannot overflow if the expression of the destination
+ // capacity contains '+ 1'
+ if (DestCapacityExprStr.contains("+1") || DestCapacityExprStr.contains("+ 1"))
+ return false;
+
+ if (DestCapacityExprStr != "" && DestCapacityExprStr == LengthExprStr)
+ return true;
+
+ return true;
+}
+
+static bool
+isDestCapacityBasedOnGivenLength(const MatchFinder::MatchResult &Result) {
+ StringRef DestCapacityExprStr =
+ exprToStr(getDestCapacityExpr(Result), Result).trim(' ');
+ StringRef LengthExprStr = exprToStr(getLengthExpr(Result), Result).trim(' ');
+
+ return DestCapacityExprStr != "" && LengthExprStr != "" &&
+ DestCapacityExprStr.contains(LengthExprStr);
+}
+
+static void lengthDecrease(const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // InnerOpExpr: ~~~~~~~~~~~~^~~
+ // OuterOpExpr: ~~~~~~~~~~~~~~~~~~^~~
+ if (const auto *OuterOpExpr =
+ dyn_cast_or_null<BinaryOperator>(LengthExpr->IgnoreParenCasts())) {
+ const auto *LHSExpr = OuterOpExpr->getLHS();
+ const auto *RHSExpr = OuterOpExpr->getRHS();
+ const auto *InnerOpExpr =
+ isa<IntegerLiteral>(RHSExpr->IgnoreCasts()) ? LHSExpr : RHSExpr;
+
+ // This is the following structure: ((strlen(src) * 2) + 1)
+ // LHSRemoveRange: ~~
+ // RHSRemoveRange: ~~~~~~
+ const auto LHSRemoveRange =
+ SourceRange(LengthExpr->getBeginLoc(),
+ InnerOpExpr->getBeginLoc().getLocWithOffset(-1));
+ const auto RHSRemoveRange =
+ SourceRange(exprLocEnd(InnerOpExpr, Result), LengthExpr->getEndLoc());
+ const auto LHSRemoveFix = FixItHint::CreateRemoval(LHSRemoveRange);
+ const auto RHSRemoveFix = FixItHint::CreateRemoval(RHSRemoveRange);
+
+ if (LengthExpr->getBeginLoc() == InnerOpExpr->getBeginLoc())
+ Diag << RHSRemoveFix;
+ else if (LengthExpr->getEndLoc() == InnerOpExpr->getEndLoc())
+ Diag << LHSRemoveFix;
+ else
+ Diag << LHSRemoveFix << RHSRemoveFix;
+ } else {
+ const auto InsertDecreaseFix =
+ FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " - 1");
+ Diag << InsertDecreaseFix;
+ }
+}
+
+static void lengthIncrease(const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool NeedInnerParen =
+ dyn_cast_or_null<BinaryOperator>(LengthExpr) &&
+ cast<BinaryOperator>(LengthExpr)->getOpcode() != BO_Add;
+
+ if (NeedInnerParen) {
+ const auto InsertFirstParenFix =
+ FixItHint::CreateInsertion(LengthExpr->getBeginLoc(), "(");
+ const auto InsertPlusOneAndSecondParenFix = FixItHint::CreateInsertion(
+ exprLocEnd(LengthExpr, Result), !IsInjectUL ? ") + 1" : ") + 1UL");
+ Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+ } else {
+ const auto InsertPlusOne = FixItHint::CreateInsertion(
+ exprLocEnd(LengthExpr, Result), !IsInjectUL ? " + 1" : " + 1UL");
+ Diag << InsertPlusOne;
+ }
+}
+
+static void lengthExprHandle(LengthHandleKind LengthHandle,
+ const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ if (!LengthExpr)
+ return;
+
+ const auto It = MacroDefinedMap.find(exprToStr(LengthExpr, Result));
+ if (It == MacroDefinedMap.end()) {
+ if (const auto *LengthIL = dyn_cast_or_null<IntegerLiteral>(LengthExpr)) {
+ const size_t NewLength = LengthIL->getValue().getZExtValue() +
+ (LengthHandle == LengthHandleKind::LHK_Increase
+ ? (IsInjectUL ? 1UL : 1)
+ : -1);
+ const auto NewLengthFix = FixItHint::CreateReplacement(
+ LengthIL->getSourceRange(),
+ (Twine(NewLength) + (IsInjectUL ? "UL" : "")).str());
+ Diag << NewLengthFix;
+ return;
+ }
+
+ if (LengthHandle == LengthHandleKind::LHK_Increase)
+ lengthIncrease(LengthExpr, Result, Diag);
+ else
+ lengthDecrease(LengthExpr, Result, Diag);
+ } else {
+ if (LengthHandle == LengthHandleKind::LHK_Increase) {
+ const auto InsertPlusOne = FixItHint::CreateInsertion(
+ exprLocEnd(LengthExpr, Result), !IsInjectUL ? " + 1" : " + 1UL");
+ Diag << InsertPlusOne;
+ } else {
+ const auto InsertMinusOne =
+ FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " - 1");
+ Diag << InsertMinusOne;
+ }
+ }
+}
+
+static void lengthArgHandle(LengthHandleKind LengthHandle, int ArgPos,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const auto *LengthExpr = FuncExpr->getArg(ArgPos)->IgnoreImpCasts();
+ lengthExprHandle(LengthHandle, LengthExpr, Result, Diag);
+}
+
+static bool destCapacityFix(const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool IsOverflows = isDestCapacityOverflows(Result);
+ if (IsOverflows)
+ lengthExprHandle(LengthHandleKind::LHK_Increase,
+ getDestCapacityExpr(Result), Result, Diag);
+
+ return IsOverflows;
+}
+
+static void 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;
+}
+
+static void renameFunc(StringRef NewFuncName,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const int FuncNameLength =
+ FuncExpr->getDirectCallee()->getIdentifier()->getLength();
+ const auto FuncNameRange =
+ SourceRange(FuncExpr->getBeginLoc(),
+ FuncExpr->getBeginLoc().getLocWithOffset(FuncNameLength - 1));
+
+ const auto FuncNameFix =
+ FixItHint::CreateReplacement(FuncNameRange, NewFuncName);
+ Diag << FuncNameFix;
+}
+
+static void insertDestCapacityArg(bool IsOverflows, StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ SmallString<64> NewSecondArg;
+
+ if (const int DestLength = getDestCapacity(Result)) {
+ NewSecondArg = Twine(IsOverflows ? DestLength + 1 : DestLength).str();
+ } else {
+ NewSecondArg = exprToStr(getDestCapacityExpr(Result), Result);
+ NewSecondArg += IsOverflows ? (!IsInjectUL ? " + 1" : " + 1UL") : "";
+ }
+
+ NewSecondArg += ", ";
+ const auto InsertNewArgFix = FixItHint::CreateInsertion(
+ FuncExpr->getArg(1)->getBeginLoc(), NewSecondArg);
+ Diag << InsertNewArgFix;
+}
+
+static void insertNullTerminatorExpr(StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const int FuncLocStartColumn =
+ Result.SourceManager->getPresumedColumnNumber(FuncExpr->getBeginLoc());
+ const auto SpaceRange = SourceRange(
+ FuncExpr->getBeginLoc().getLocWithOffset(-FuncLocStartColumn + 1),
+ FuncExpr->getBeginLoc());
+ StringRef SpaceBeforeStmtStr = Lexer::getSourceText(
+ CharSourceRange::getCharRange(SpaceRange), *Result.SourceManager,
+ Result.Context->getLangOpts(), 0);
+
+ SmallString<128> NewAddNullTermExprStr;
+ NewAddNullTermExprStr = "\n";
+ NewAddNullTermExprStr += SpaceBeforeStmtStr;
+ NewAddNullTermExprStr += exprToStr(getDestExpr(Result), Result);
+ NewAddNullTermExprStr += "[";
+ NewAddNullTermExprStr += exprToStr(getLengthExpr(Result), Result);
+ NewAddNullTermExprStr += "] = ";
+ NewAddNullTermExprStr += (Name[0] != 'w') ? "\'\\0\';" : "L\'\\0\';";
+
+ const auto AddNullTerminatorExprFix = FixItHint::CreateInsertion(
+ exprLocEnd(FuncExpr, Result).getLocWithOffset(1), NewAddNullTermExprStr);
+ Diag << AddNullTerminatorExprFix;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tidy/bugprone/CMakeLists.txt
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -21,6 +21,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
@@ -30,6 +30,7 @@
#include "MisplacedWideningCastCheck.h"
#include "MoveForwardingReferenceCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "NotNullTerminatedResultCheck.h"
#include "ParentVirtualCallCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
@@ -98,6 +99,8 @@
"bugprone-multiple-statement-macro");
CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
"bugprone-narrowing-conversions");
+ 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
