Charusso updated this revision to Diff 162942.
Charusso marked an inline comment as done.
Charusso retitled this revision from "[clang-tidy] New checker for not
null-terminated result caused by strlen or wcslen" to "[clang-tidy] New checker
for not null-terminated result caused by strlen(), size() or equal length".
Charusso edited the summary of this revision.
Charusso added a comment.
- The checker by default search for `__STDC_WANT_LIB_EXT1__` macro to determine
if `_s` suffixed functions are available. This behaviour could be overridden by
specifying `AreSafeFunctionsAvailable`, which is became a string (it was an
integer, but it can be used in the same way).
- If the destination array length is based on a custom `malloc()` the current
approach only want to rewrite it if it is 100% sure the chosen argument is
specifying the length.
- If the checker works with `size()` function now it is only match for
`std::string` (e.g. it matched to StringRefs).
- Now it handles macro defined lengths properly.
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,116 @@
+// 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
@@ -37,6 +37,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,137 @@
+.. 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 with 'memcpy()'
+------------------------------------
+
+It is possible to rewrite the ``memcpy()`` and ``memcpy_s()`` calls as the
+following four function: ``strcpy()``, ``strncpy()``, ``strcpy_s()``,
+``strncpy_s()``, where the latter two is the safe version. Analogly it is
+possible to rewrite ``wmemcpy()`` related functions handled in the same way.
+
+Rewrite to a string handler function is not possible:
+
+- If the type of the destination array is not just ``char``, that means it
+ cannot be any string handler function. But if the given length is
+ ``strlen(source)`` then fix-it adds ``+ 1`` to it, so the result will be
+ null-terminated.
+
+Rewrite based on the destination array:
+
+- If the destination cannot overflow then the new function is should be the
+ simple ``cpy()``, because it is more efficient.
+
+- If the destination can overflow and ``AreSafeFunctionsAvailable = 1`` and it
+ is possible to read the length of the destination array then the new function
+ could be safe (``cpy_s()``).
+
+- If the new function is could be safe and the target environment is C++ then
+ it is not necessary to pass the length of the destination array.
+
+and based on the length of the source string:
+
+- If the given length is ``strlen(source)`` (or equals length) then the new
+ function is should be the simple ``cpy()``, because 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()``.
+
+Transformations with 'strlen()' length
+--------------------------------------
+
+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 with 'memcpy()'<MemcpyTransformation>` section.
+
+- ``memchr``:
+ - 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``:
+ - 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 function's fourth argument gets a ``+ 1`` operation.
+
+- ``memset`` function's third argument has to be truncated without the ``+ 1``.
+
+String handler functions
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+- ``strerror_s`` functions's second argument get a ``+ 1`` operation.
+
+- ``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`` function's third argument gets a ``+ 1`` operation.
+
+Options
+-------
+
+.. option:: AreSafeFunctionsAvailable
+
+ A string which is can be used as an integer. The default value is ``""``.
+ If ``Default``, ``d`` or not set the checker rely on the existence/value of
+ the ``__STDC_WANT_LIB_EXT1__`` macro.
+ If ``Yes``, ``y`` or non-zero value the target environment implements ``_s``
+ suffixed memory and character handler functions which are safer than older
+ version (e.g. ``memcpy_s()``).
+ If ``No``, ``n`` or zero value the ``_s`` suffixed functions are not
+ available.
+
+.. option:: InjectUL
+
+ An integer non-zero value specifying if the checker injects ``UL`` suffix to
+ every ``+ 1`` operation. The default value is ``0``.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,15 @@
Diagnoses comparisons that appear to be incorrectly placed in the argument to
the ``TEMP_FAILURE_RETRY`` macro.
+- 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:`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,74 @@
+//===--- 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;
+
+ // If 'Default', 'd' or not set the checker rely on the existence/value of the
+ // __STDC_WANT_LIB_EXT1__ macro.
+ // If 'Yes', 'y' or non-zero value the target environment implements '_s'
+ // suffixed memory and character handler functions which are safer than older
+ // version (e.g. 'memcpy_s()').
+ // If 'No', 'n' or zero value the '_s' suffixed functions are not available.
+ const std::string AreSafeFunctionsAvailable;
+
+ // If non-zero the checker injects 'UL' suffix to every '+ 1' operation.
+ const int InjectUL;
+
+ 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,976 @@
+//===--- 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"
+#include "clang/Lex/Preprocessor.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 BinOpName = "has-binary-operator";
+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 { LH_Increase, LH_Decrease };
+
+namespace {
+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
+
+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 SourceLocation exprLocEnd(const Expr *E,
+ const MatchFinder::MatchResult &Result) {
+ return Lexer::getLocForEndOfToken(E->getLocEnd(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+}
+
+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 isKnownDest(const MatchFinder::MatchResult &Result) {
+ return !Result.Nodes.getNodeAs<Expr>(UnknownDestName);
+}
+
+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 const Expr *getDestExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(DestExprName);
+}
+
+static const Expr *getSrcExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(SrcExprName);
+}
+
+static const Expr *getLengthExpr(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Expr>(LengthExprName);
+}
+
+static bool isGivenLengthEQToSrcLength(const MatchFinder::MatchResult &Result);
+static bool isNoWrongLength(const MatchFinder::MatchResult &Result) {
+ if (Result.Nodes.getNodeAs<IntegerLiteral>(WrongLengthExprName))
+ return !isGivenLengthEQToSrcLength(Result);
+
+ return !Result.Nodes.getNodeAs<Expr>(WrongLengthExprName);
+}
+
+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;
+}
+
+//===---------------------------------------------------------------------===//
+// Rewrite decision helper functions
+//===---------------------------------------------------------------------===//
+
+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;
+
+ // Assume that it cannot overflow if the destination length contains '+ 1'.
+ if (Result.Nodes.getNodeAs<BinaryOperator>(BinOpName))
+ return false;
+
+ 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);
+ if (DestCapacityExprStr != "" && DestCapacityExprStr == LengthExprStr)
+ return true;
+
+ if (const auto DestTy = Result.Nodes.getNodeAs<ArrayType>(DestArrayTyName))
+ if (const auto *DestVAT = dyn_cast_or_null<VariableArrayType>(DestTy)) {
+ StringRef SizeExprStr = exprToStr(DestVAT->getSizeExpr(), Result);
+ if (SizeExprStr.contains("+1") || SizeExprStr.contains("+ 1"))
+ return false;
+ }
+
+ return true;
+}
+
+// True if the capacity of the destination array is based on the given length,
+// therefore it looks like it cannot overflow.
+static bool isProperDestCapacity(const MatchFinder::MatchResult &Result) {
+ StringRef DestCapacityExprStr =
+ exprToStr(getDestCapacityExpr(Result), Result).trim(' ');
+ StringRef LengthExprStr = exprToStr(getLengthExpr(Result), Result).trim(' ');
+
+ return DestCapacityExprStr != "" && LengthExprStr != "" &&
+ DestCapacityExprStr.contains(LengthExprStr);
+}
+
+//===---------------------------------------------------------------------===//
+// Code injection functions
+//===---------------------------------------------------------------------===//
+
+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->getLocStart(),
+ InnerOpExpr->getLocStart().getLocWithOffset(-1));
+ const auto RHSRemoveRange =
+ SourceRange(exprLocEnd(InnerOpExpr, Result), LengthExpr->getLocEnd());
+ const auto LHSRemoveFix = FixItHint::CreateRemoval(LHSRemoveRange);
+ const auto RHSRemoveFix = FixItHint::CreateRemoval(RHSRemoveRange);
+
+ if (LengthExpr->getLocStart() == InnerOpExpr->getLocStart())
+ Diag << RHSRemoveFix;
+ else if (LengthExpr->getLocEnd() == InnerOpExpr->getLocEnd())
+ Diag << LHSRemoveFix;
+ else
+ Diag << LHSRemoveFix << RHSRemoveFix;
+ } else {
+ const auto InsertDecreaseFix = FixItHint::CreateInsertion(
+ exprLocEnd(LengthExpr, Result), !IsInjectUL ? " - 1" : " - 1UL");
+ 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->getLocStart(), "(");
+ 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 int NewLength =
+ LengthIL->getValue().getZExtValue() +
+ (LengthHandle == LengthHandleKind::LH_Increase ? 1 : -1);
+ const auto NewLengthFix = FixItHint::CreateReplacement(
+ LengthIL->getSourceRange(), Twine(NewLength).str());
+ Diag << NewLengthFix;
+ return;
+ }
+
+ if (LengthHandle == LengthHandleKind::LH_Increase)
+ lengthIncrease(LengthExpr, Result, Diag);
+ else
+ lengthDecrease(LengthExpr, Result, Diag);
+ } else {
+ if (LengthHandle == LengthHandleKind::LH_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::LH_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->getLocStart(),
+ FuncExpr->getLocStart().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)->getLocStart(), 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->getLocStart());
+ const auto SpaceRange = SourceRange(
+ FuncExpr->getLocStart().getLocWithOffset(-FuncLocStartColumn + 1),
+ FuncExpr->getLocStart());
+ 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;
+}
+
+NotNullTerminatedResultCheck::NotNullTerminatedResultCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ AreSafeFunctionsAvailable(Options.get("AreSafeFunctionsAvailable", "")),
+ InjectUL(Options.get("InjectUL", 0)) {
+ 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);
+ Options.store(Opts, "InjectUL", InjectUL);
+}
+
+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())))
+ .bind(BinOpName);
+
+ 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 three case match 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 six 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->getLocStart().isMacroID())
+ return;
+
+ IsInjectUL = InjectUL;
+ 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)->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("move"))
+ memmoveFix(Name, Result, Diag);
+ else if (Name.endswith("move_s")) {
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LH_Increase, 3, Result, Diag);
+ } else if (Name.endswith("set")) {
+ lengthArgHandle(LengthHandleKind::LH_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::LH_Increase, 2, Result, Diag);
+ return;
+ }
+
+ const bool IsCpy =
+ isGivenLengthEQToSrcLength(Result) || isProperDestCapacity(Result);
+ const bool IsSafe =
+ SafeFunctionsAvailable == SafeFunctionsAvailableKind::Yes &&
+ IsOverflows && isKnownDest(Result) && !isProperDestCapacity(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::LH_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::LH_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()->getLocStart(),
+ "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->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) {
+ 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::LH_Increase, 2, Result, Diag);
+}
+
+void NotNullTerminatedResultCheck::strerror_sFix(
+ const MatchFinder::MatchResult &Result) {
+ StringRef Name = "strerror_s";
+ auto Diag =
+ diag(Result.Nodes.getNodeAs<CallExpr>(FuncExprName)->getLocStart(),
+ "the result from calling '%0' is not null-terminated and "
+ "missing the last character of the error message")
+ << Name;
+
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LH_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()->getLocStart(),
+ "comparison length is too long and might lead to a "
+ "buffer overflow");
+
+ lengthArgHandle(LengthHandleKind::LH_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)->getLocStart(),
+ "the result from calling '%0' is not null-terminated")
+ << Name;
+
+ destCapacityFix(Result, Diag);
+ lengthArgHandle(LengthHandleKind::LH_Increase, 2, Result, Diag);
+}
+
+} // 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
@@ -29,6 +29,7 @@
#include "MisplacedWideningCastCheck.h"
#include "MoveForwardingReferenceCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "NotNullTerminatedResultCheck.h"
#include "ParentVirtualCallCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
@@ -95,6 +96,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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
