Charusso updated this revision to Diff 157916.
Charusso added a comment.
Excuse me for the huge patch, but what I can done wrongly, I did, so I have
made tons of revision.
I would like to show the current direction of the checker. Currently it has too
much overlapping function, so please don't go too deep into the core.
Major fixes in every function:
- Equal length of `strlen(src)` is now handled.
- It can read out the length of the destination buffer.
- If the length increases in the passed argument, it also increases the
destination buffer (if overflow is looks like to possible).
Major fixes in `memcpy()` and `memmove()`:
- It can decide which function is the best in performance and safety. (The last
implementation was rely on C++11 version, which was a huge mistake.)
- Handles cases where the destination is `unsigned char` or `signed char`,
which cannot be passed to any string handler function. (It haven't checked the
type so that made wrong fix-its.)
Minor fixes:
- Removed all memory allocation matchers in general, but the current functions
behave the same to increase the buffer length.
- Now the test files' `RUN` command working well. (CheckOptions was wrong.)
Problematic:
1. It allows all custom memory allocation function. Sometimes the read buffer
size is not the size parameter.
2. May it is not a good idea to heuristically read and increase buffer lengths.
3. If the new function transformed from `memcpy()` to `strncpy()`, the checker
adds the null terminator expression in the next line, like `dest[length] =
'\0'`, which is looks like a quite big injection.
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,133 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+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,128 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+
+wchar_t *wcschr(const wchar_t *, int);
+errno_t wcsncpy_s(wchar_t *, 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,143 @@
+// 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 *, 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,134 @@
+// 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 *);
+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,107 @@
+// 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 *);
+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 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);
+}
+
+// FIXME: Should be 'SRC_LENGTH + 1' instead of the constant value.
+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, 4);
+}
+
+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,156 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+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 = 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: char *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,73 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: bugprone-not-null-terminated-result.AreSafeFunctionsAvailable, \
+// RUN: value: 0}]}" \
+// RUN: -- -std=c11
+
+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,123 @@
+// 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) {
+ int length = strlen(str) + 1;
+ return length;
+}
+
+
+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];
+ int length = getLengthWithInc(src);
+ memset(dest, '-', length);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memset' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memset(dest, '-', length - 1);
+}
+
+void good_memset1(const char *src) {
+ char dst[13];
+ int length = getLengthWithInc(src);
+ memset(dst, '-', length - 1);
+}
+
+void bad_memset_2(const char *src) {
+ char dest[13];
+ int length = strlen(src);
+ memset(dest, '-', length + 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memset' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: memset(dest, '-', length);
+}
+
+void good_memset_2(const char *src) {
+ char dst[13];
+ int length = strlen(src);
+ memset(dst, '-', length);
+}
+
+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,127 @@
+.. 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 neccesary, 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 neccesary 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
+
+ An integer non-zero value specifying if the target environment implements
+ ``_s`` suffixed memory and character handler functions which are safer than
+ older versions (e.g. ``memcpy_s()``). The default value is ``1``.
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,63 @@
+//===--------------- NotNullTerminatedResultCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOT_NULL_TERMINATED_RESULT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOT_NULL_TERMINATED_RESULT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds function calls where ``strlen`` or ``wcslen`` function is passed as an
+/// argument and caused a not null-terminated result. Depending on the case of
+/// use, it may insert or remove the increase operation to ensure the result is
+/// null-terminated.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-not-null-terminated-result.html
+class NotNullTerminatedResultCheck : public ClangTidyCheck {
+public:
+ NotNullTerminatedResultCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ // If non-zero the target environment implements '_s' suffixed memory and
+ // character handler functions which are safer than older versions
+ // (e.g. 'memcpy_s()').
+ const int 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,937 @@
+//===------------- NotNullTerminatedResultCheck.cpp - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NotNullTerminatedResultCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static const char *const FuncExprName = "entire-called-function-expr";
+static const char *const CastExprName = "cast-expr";
+static const char *const ProperDestLengthName = "destination-cannot-overflow";
+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 LengthOpName = "length-call-has-binary-operator";
+static const char *const StrlenCallName = "simple-strlen-or-wcslen-call";
+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";
+
+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 isNoStrlen(const MatchFinder::MatchResult &Result) {
+ return !Result.Nodes.getNodeAs<CallExpr>(StrlenCallName);
+}
+
+static bool isNotPartialLength(const MatchFinder::MatchResult &Result) {
+ if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>(LengthOpName))
+ return BinOp->getOpcode() != BO_Sub && BinOp->getOpcode() != BO_Div;
+
+ return true;
+}
+
+static bool isKnownDest(const MatchFinder::MatchResult &Result) {
+ return !Result.Nodes.getNodeAs<Expr>(UnknownDestName);
+}
+
+static bool isDestAndSrcEquals(const MatchFinder::MatchResult &Result) {
+ return Result.Nodes.getNodeAs<Decl>(DestVarDeclName) ==
+ Result.Nodes.getNodeAs<Decl>(SrcVarDeclName);
+}
+
+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 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 isGivenLengthEQToSrcLength(const MatchFinder::MatchResult &Result) {
+ 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;
+
+ const auto *SrcExpr = getSrcExpr(Result);
+ const auto *LengthExpr = getLengthExpr(Result);
+ StringRef SrcStr = exprToStr(SrcExpr, Result);
+ StringRef GivenLengthStr = exprToStr(LengthExpr, Result);
+
+ if (SrcStr.contains(".data") && (GivenLengthStr.contains(".size") ||
+ (GivenLengthStr.contains(".length"))))
+ return true;
+
+ if (const auto *StrlenExpr = Result.Nodes.getNodeAs<CallExpr>(LengthExprName))
+ 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);
+ if (DestCapacityExprStr != "" && DestCapacityExprStr == LengthExprStr)
+ return true;
+
+ // Assume that it cannot overflow if the destination length contains '+ 1'.
+ if (Result.Nodes.getNodeAs<BinaryOperator>(ProperDestLengthName))
+ return false;
+
+ 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;
+}
+
+static bool isProperDestCapacity(const MatchFinder::MatchResult &Result) {
+ if (Result.Nodes.getNodeAs<BinaryOperator>(ProperDestLengthName))
+ return true;
+
+ StringRef DestCapacityExprStr =
+ exprToStr(getDestCapacityExpr(Result), Result);
+ StringRef LengthExprStr = exprToStr(getLengthExpr(Result), Result);
+ return DestCapacityExprStr != "" && LengthExprStr != "" &&
+ DestCapacityExprStr.contains(LengthExprStr);
+}
+
+static bool isMemcpyRewrite(const MatchFinder::MatchResult &Result) {
+ if (isDestAndSrcEquals(Result))
+ return false;
+
+ StringRef GivenLengthStr = exprToStr(getLengthExpr(Result), Result);
+ if (isNoStrlen(Result) && !GivenLengthStr.contains(".size") &&
+ !GivenLengthStr.contains(".length") &&
+ !isGivenLengthEQToSrcLength(Result))
+ return false;
+
+ return true;
+}
+
+//===---------------------------------------------------------------------===//
+// Code injection functions
+//===---------------------------------------------------------------------===//
+
+static void lengthIncrease(const Expr *LengthExpr,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ if (!LengthExpr)
+ return;
+
+ if (const auto *LengthIL = dyn_cast_or_null<IntegerLiteral>(LengthExpr)) {
+ const int NewLength = LengthIL->getValue().getZExtValue() + 1;
+ const auto NewValueFix = FixItHint::CreateReplacement(
+ LengthExpr->getSourceRange(), Twine(NewLength).str());
+ Diag << NewValueFix;
+ return;
+ }
+
+ 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), ") + 1");
+ Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+ } else {
+ const auto InsertPlusOne =
+ FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " + 1");
+ Diag << InsertPlusOne;
+ }
+}
+
+static bool destCapacityFix(const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool IsOverflows = isDestCapacityOverflows(Result);
+ if (IsOverflows)
+ lengthIncrease(getDestCapacityExpr(Result), Result, Diag);
+
+ return IsOverflows;
+}
+
+static void lengthArgInsertIncrease(int ArgPos, StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const auto *LengthExpr =
+ FuncExpr->getArg(ArgPos)->IgnoreParenCasts()->IgnoreImpCasts();
+ lengthIncrease(LengthExpr, Result, Diag);
+}
+
+static void lengthArgRemoveInc(int ArgPos, StringRef Name,
+ const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const auto *FuncExpr = Result.Nodes.getNodeAs<CallExpr>(FuncExprName);
+ const auto *LengthExpr = FuncExpr->getArg(ArgPos)->IgnoreImpCasts();
+
+ if (const auto *LengthIL = dyn_cast_or_null<IntegerLiteral>(LengthExpr)) {
+ const int NewLength = LengthIL->getValue().getZExtValue() - 1;
+
+ const auto NewLengthFix = FixItHint::CreateReplacement(
+ LengthIL->getSourceRange(), Twine(NewLength).str());
+ Diag << NewLengthFix;
+
+ return;
+ }
+
+ // 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), " - 1");
+ Diag << InsertDecreaseFix;
+ }
+}
+
+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 ? " + 1" : "";
+ }
+
+ 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", 1)) {}
+
+void NotNullTerminatedResultCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AreSafeFunctionsAvailable", AreSafeFunctionsAvailable);
+}
+
+void NotNullTerminatedResultCheck::registerMatchers(MatchFinder *Finder) {
+ const auto StrlenCallWithoutInc =
+ callExpr(callee(functionDecl(hasAnyName("::strlen", "::wcslen"))))
+ .bind(StrlenCallName);
+
+ const auto HasIntOperand =
+ hasEitherOperand(ignoringParenImpCasts(integerLiteral()));
+
+ const auto HasIncOp = binaryOperator(hasOperatorName("+"), HasIntOperand);
+
+ 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);
+
+ 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 seven case match 'strlen' or 'wcslen' function calls
+ // where no increase by one operation found.
+ //===--------------------------------------------------------------------===//
+
+ // - Example: (strlen(src) * 2)
+ const auto ComplexCallWithoutInc =
+ binaryOperator(unless(hasOperatorName("+")),
+ hasEitherOperand(ignoringImpCasts(StrlenCallWithoutInc)))
+ .bind(LengthOpName);
+
+ // - Example: (strlen(dest) + strlen(src))
+ const auto ComplexTwoCallWithoutInc =
+ binaryOperator(hasOperatorName("+"), unless(HasIntOperand),
+ hasLHS(ignoringImpCasts(StrlenCallWithoutInc)),
+ hasRHS(ignoringImpCasts(StrlenCallWithoutInc)));
+
+ const auto AnyOfCallWithoutInc = ignoringImpCasts(anyOf(
+ StrlenCallWithoutInc, ComplexCallWithoutInc, ComplexTwoCallWithoutInc));
+
+ // - Example: length = strlen(src);
+ const auto DREWithoutInc = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(AnyOfCallWithoutInc)))));
+
+ // - Example: malloc(length * 2);
+ const auto OpHasDREWithoutInc = binaryOperator(
+ unless(hasOperatorName("+")), hasEitherOperand(DREWithoutInc));
+
+ const auto AnyOfCallOrDREWithoutInc =
+ anyOf(OpHasDREWithoutInc, DREWithoutInc, AnyOfCallWithoutInc);
+
+ // - Example: int getLength(const char *str) { return strlen(str); }
+ const auto CallExprReturnWithoutInc =
+ ignoringImpCasts(callExpr(callee(functionDecl(hasBody(
+ has(returnStmt(hasReturnValue(AnyOfCallOrDREWithoutInc))))))));
+
+ // - Example: malloc(getLength(str) * 2);
+ const auto CallExprOpHasReturnWithoutInc = binaryOperator(
+ unless(hasOperatorName("+")), hasEitherOperand(CallExprReturnWithoutInc));
+
+ const auto AnyOfCallExprReturnWithoutInc =
+ anyOf(CallExprReturnWithoutInc, CallExprOpHasReturnWithoutInc);
+
+ // - Example: int length = getLength(src);
+ const auto DREHasReturnWithoutInc = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(AnyOfCallExprReturnWithoutInc)))));
+
+ const auto LengthWithoutInc =
+ anyOf(AnyOfCallOrDREWithoutInc, AnyOfCallExprReturnWithoutInc,
+ DREHasReturnWithoutInc);
+
+ //===--------------------------------------------------------------------===//
+ // The following five case match 'strlen' or 'wcslen' function calls
+ // where increase by one operation found.
+ //===--------------------------------------------------------------------===//
+
+ // - Example: (strlen(src) + 1)
+ const auto SimpleCallWithInc = allOf(
+ HasIncOp,
+ binaryOperator(hasEitherOperand(ignoringImpCasts(StrlenCallWithoutInc))));
+
+ // - Example: ((strlen(src) / 2) + 1)
+ const auto ComplexCallWithInc =
+ allOf(HasIncOp,
+ binaryOperator(has(binaryOperator(
+ unless(hasOperatorName("+")),
+ hasEitherOperand(ignoringImpCasts(StrlenCallWithoutInc))))));
+
+ // - Example: int length = strlen(src); memcpy(dest, src, length + 1);
+ const auto OpHasDREWithInc =
+ binaryOperator(hasEitherOperand(ignoringImpCasts(DREWithoutInc)));
+
+ const auto AnyOfCallWithInc = anyOf(SimpleCallWithInc, ComplexCallWithInc);
+
+ const auto AnyOfCallOrDREWithInc = ignoringImpCasts(
+ anyOf(OpHasDREWithInc,
+ declRefExpr(to(
+ varDecl(hasInitializer(ignoringImpCasts(AnyOfCallWithInc))))),
+ AnyOfCallWithInc));
+
+ // - Example: int getLength(const char *str) { return strlen(str) + 1; }
+ const auto CallExprReturnWithInc =
+ ignoringImpCasts(callExpr(callee(functionDecl(
+ hasBody(has(returnStmt(hasReturnValue(AnyOfCallOrDREWithInc))))))));
+
+ // - Example: int length = getLength(src);
+ const auto DREHasReturnWithInc = ignoringImpCasts(
+ declRefExpr(to(varDecl(hasInitializer(CallExprReturnWithInc)))));
+
+ const auto LengthWithInc =
+ anyOf(AnyOfCallOrDREWithInc, CallExprReturnWithInc, DREHasReturnWithInc);
+
+ //===--------------------------------------------------------------------===//
+ // The following six case match the 'destination' array length's
+ // expression which is used in memcpy() and memmove() matchers.
+ //===--------------------------------------------------------------------===//
+
+ // - Example: (strlen(src) + IntegerLiteral)
+ const auto ProperLengthExpr = HasIncOp.bind(ProperDestLengthName);
+
+ const auto MallocLengthExpr = hasArgument(
+ 0, anyOf(allOf(ProperLengthExpr, expr().bind(DestMallocExprName)),
+ expr().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))))
+ .bind(DestExprName);
+
+ const auto DestArrayTyDeclMayInBinOp =
+ anyOf(DestArrayTyDecl, hasDescendant(DestArrayTyDecl));
+
+ // - Example: foo[bar[baz]].qux; (or just ParmVarDecl)
+ const auto DestUnknownDecl =
+ declRefExpr(allOf(to(varDecl(AnyOfCharTy).bind(DestVarDeclName)),
+ expr().bind(UnknownDestName)))
+ .bind(DestExprName);
+
+ const auto DestUnknownDeclMayInBinOp =
+ anyOf(DestUnknownDecl, hasDescendant(DestUnknownDecl));
+
+ // - Example: char *dest; dest = (char *)malloc(length);
+ const auto HasArgZeroDestDefinitionBinOp = allOf(
+ hasArgument(0,
+ declRefExpr(to(varDecl(AnyOfCharTy).bind(DestVarDeclName)))),
+ hasAncestor(compoundStmt(hasDescendant(binaryOperator(
+ hasLHS(declRefExpr(to(varDecl(equalsBoundNode(DestVarDeclName))))
+ .bind(DestExprName)),
+ hasRHS(AnyOfDestInit))))));
+
+ //===--------------------------------------------------------------------===//
+ // The following match the 'destination', 'source' and 'length'
+ // expressions in memcpy() and memmove() calls.
+ //===--------------------------------------------------------------------===//
+
+ const auto HasArgZeroAnyOfDestDecl = allOf(
+ anyOf(hasArgument(0, DestArrayTyDeclMayInBinOp),
+ HasArgZeroDestDefinitionBinOp,
+ hasArgument(0, DestUnknownDeclMayInBinOp)),
+ unless(hasAncestor(compoundStmt(hasDescendant(NullTerminatorExpr)))));
+
+ const auto SrcDecl = declRefExpr(
+ allOf(to(decl().bind(SrcVarDeclName)),
+ anyOf(hasAncestor(cxxMemberCallExpr().bind(SrcExprName)),
+ expr().bind(SrcExprName))));
+
+ const auto SrcDeclMayInBinOp = anyOf(SrcDecl, hasDescendant(SrcDecl));
+
+ const auto AnyOfSrcDecl = ignoringImpCasts(
+ anyOf(stringLiteral().bind(SrcExprName), SrcDeclMayInBinOp));
+
+ // - Example: char src[] = "foo"; sizeof(src);
+ const auto SizeOfCharExpr =
+ unaryExprOrTypeTraitExpr(has(expr(hasType(qualType(
+ hasCanonicalType(anyOf(arrayType(hasElementType(isAnyCharacter())),
+ pointerType(pointee(isAnyCharacter())))))))));
+
+ const auto AnyOfLengthExpr = ignoringImpCasts(
+ allOf(unless(SizeOfCharExpr), unless(hasDescendant(SizeOfCharExpr)),
+ anyOf(integerLiteral(), ProperLengthExpr, LengthWithoutInc,
+ expr().bind(UnknownLengthName)),
+ expr().bind(LengthExprName)));
+
+ //===--------------------------------------------------------------------===//
+ // The following two + eleven case match problematic function calls.
+ //===--------------------------------------------------------------------===//
+
+ const auto MemcpyAndMemmoveMatcher =
+ allOf(callee(functionDecl(hasAnyName("::memcpy", "::wmemcpy", "::memmove",
+ "::wmemmove"))),
+ HasArgZeroAnyOfDestDecl, hasArgument(1, AnyOfSrcDecl),
+ hasArgument(2, AnyOfLengthExpr));
+
+ const auto Memcpy_sAndMemmove_sMatcher =
+ allOf(callee(functionDecl(hasAnyName("::memcpy_s", "::wmemcpy_s",
+ "::memmove_s", "::wmemmove_s"))),
+ HasArgZeroAnyOfDestDecl, hasArgument(2, AnyOfSrcDecl),
+ hasArgument(3, AnyOfLengthExpr));
+
+ enum class StrlenKind { WithInc, WithoutInc };
+ enum class SourceArgExistsKind { Exists, NotExists };
+
+ const auto Matcher = [=](StringRef Name, int ArgCount, int SourcePos,
+ int LengthPos, StrlenKind LengthKind,
+ SourceArgExistsKind SrcKind) {
+ return allOf(
+ callee(functionDecl(hasName(Name))), argumentCountIs(ArgCount),
+ anyOf(HasArgZeroAnyOfDestDecl, anything()),
+ hasArgument(SourcePos, (SrcKind == SourceArgExistsKind::Exists)
+ ? AnyOfSrcDecl
+ : anything()),
+ hasArgument(LengthPos,
+ (SrcKind == SourceArgExistsKind::Exists)
+ ? ignoringImpCasts(allOf(
+ anyOf(integerLiteral(),
+ (LengthKind == StrlenKind::WithoutInc)
+ ? ignoringImpCasts(LengthWithoutInc)
+ : ignoringImpCasts(LengthWithInc)),
+ expr().bind(LengthExprName)))
+ : (LengthKind == StrlenKind::WithoutInc)
+ ? ignoringImpCasts(LengthWithoutInc)
+ : ignoringImpCasts(LengthWithInc)));
+ };
+
+ const auto Memchr = Matcher("::memchr", 3, 0, 2, StrlenKind::WithoutInc,
+ SourceArgExistsKind::Exists);
+ const auto Wmemchr = Matcher("::wmemchr", 3, 0, 2, StrlenKind::WithoutInc,
+ SourceArgExistsKind::Exists);
+ const auto Memset = Matcher("::memset", 3, 0, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::NotExists);
+ const auto Wmemset = Matcher("::wmemset", 3, 0, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::NotExists);
+ const auto Strerror_s =
+ Matcher("::strerror_s", 3, 0, 1, StrlenKind::WithoutInc,
+ SourceArgExistsKind::NotExists);
+ const auto StrncmpLHS = Matcher("::strncmp", 3, 0, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::Exists);
+ const auto WcsncmpLHS = Matcher("::wcsncmp", 3, 0, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::Exists);
+ const auto StrncmpRHS = Matcher("::strncmp", 3, 1, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::Exists);
+ const auto WcsncmpRHS = Matcher("::wcsncmp", 3, 1, 2, StrlenKind::WithInc,
+ SourceArgExistsKind::Exists);
+ const auto Strxfrm = Matcher("::strxfrm", 3, 1, 2, StrlenKind::WithoutInc,
+ SourceArgExistsKind::Exists);
+ const auto Wcsxfrm = Matcher("::wcsxfrm", 3, 1, 2, StrlenKind::WithoutInc,
+ SourceArgExistsKind::Exists);
+
+ const auto AnyOfMatchers =
+ anyOf(MemcpyAndMemmoveMatcher, Memcpy_sAndMemmove_sMatcher, Memchr,
+ Wmemchr, 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;
+
+ 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 (!Name.contains("cpy") && !isGivenLengthEQToSrcLength(Result) &&
+ isNoStrlen(Result))
+ return;
+
+ if (Name.endswith("chr")) {
+ memchrFix(Name, Result);
+ return;
+ }
+
+ if (Name.contains("cpy") && !isMemcpyRewrite(Result))
+ return;
+
+ if (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);
+ lengthArgInsertIncrease(3, Name, Result, Diag);
+ } else if (Name.endswith("set")) {
+ lengthArgRemoveInc(2, Name, Result, Diag);
+ }
+}
+
+void NotNullTerminatedResultCheck::memcpyFix(
+ StringRef Name, const MatchFinder::MatchResult &Result,
+ DiagnosticBuilder &Diag) {
+ const bool IsOverflows = destCapacityFix(Result, Diag);
+
+ // If 'memcpy()' cannot be rewritten to any string handler function
+ if (Result.Nodes.getNodeAs<Type>(NotJustCharTyName)) {
+
+ // If it is could be 'memcpy_s()'
+ if (AreSafeFunctionsAvailable && isKnownDest(Result)) {
+ renameFunc("memcpy_s", Result, Diag);
+ insertDestCapacityArg(IsOverflows, Name, Result, Diag);
+ }
+
+ lengthArgInsertIncrease(2, Name, Result, Diag);
+
+ return;
+ }
+
+ const bool IsCpy = isGivenLengthEQToSrcLength(Result) ||
+ (!isNoStrlen(Result) && isNotPartialLength(Result));
+ const bool IsSafe = AreSafeFunctionsAvailable && 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)) {
+ lengthArgInsertIncrease(3, Name, Result, Diag);
+ return;
+ }
+
+ const bool RemoveDestLength =
+ getLangOpts().CPlusPlus &&
+ Result.Nodes.getNodeAs<ArrayType>(DestArrayTyName);
+ const bool IsCpy = !isNoStrlen(Result) && isNotPartialLength(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))
+ lengthArgInsertIncrease(1, Name, 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 (AreSafeFunctionsAvailable && isKnownDest(Result)) {
+ renameFunc((Name[0] != 'w') ? "memmove_s" : "wmemmove_s", Result, Diag);
+ insertDestCapacityArg(IsOverflows, Name, Result, Diag);
+ }
+
+ lengthArgInsertIncrease(2, Name, 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);
+ lengthArgInsertIncrease(1, Name, 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 (!isNoStrlen(Result)) {
+ const auto *LengthExprArgExpr =
+ Result.Nodes.getNodeAs<CallExpr>(StrlenCallName)->getArg(0);
+ StringRef FirstExprStr = exprToStr(FirstArgExpr, Result);
+ StringRef SecondExprStr = exprToStr(SecondArgExpr, Result);
+ StringRef LengthArgStr = exprToStr(LengthExprArgExpr, Result);
+ IsLengthTooLong =
+ LengthArgStr == FirstExprStr || LengthArgStr == SecondExprStr;
+ } else {
+ const int FirstExprLength = getLength(FirstArgExpr, Result);
+ const int SecondExprLength = getLength(SecondArgExpr, Result);
+ const int GivenLength =
+ getLength(FuncExpr->getArg(2)->IgnoreImpCasts(), Result);
+ IsLengthTooLong = GivenLength - 1 == FirstExprLength ||
+ GivenLength - 1 == SecondExprLength;
+ }
+
+ if (!IsLengthTooLong)
+ return;
+
+ auto Diag = diag(FuncExpr->getArg(2)->IgnoreParenCasts()->getLocStart(),
+ "comparison length is too long and might lead to a "
+ "buffer overflow");
+
+ lengthArgRemoveInc(2, "ncmp", 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);
+ lengthArgInsertIncrease(2, Name, 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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
