[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 2 inline comments as done.
Charusso added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:377-381
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), ") + 1");
+Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+  } else {
+const auto InsertPlusOne =
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " + 1");

lebedev.ri wrote:
> The fixits are incorrect.
> Increment by `int(1)` can result in overflow, which is then extended to 
> `size_t`
> It should be `+ 1UL`.
> https://godbolt.org/g/4nQiek
Could you show me a true example where it causes a problem? I think it is too 
rare to rewrite all code as you mentioned.



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c:158-180
+void bad_memset_1(char *dest, const char *src) {
+  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);
+}
+

lebedev.ri wrote:
> ??
> These look like false-negatives.
> How do you make an assumption about `dest` buffer from `src` string length?
My idea here is you do not want to set your null terminator to anything else, 
because then the result is not null-terminated.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

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 
+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 
+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 
+errno_t wcscpy_s(wchar_t (&dest)[size], const wchar_t *);
+errno_t wcscpy_s(wchar_t *, size_t, const wchar_t *);
+
+template 
+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];
+  wcsc

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45050#1127449, @Charusso wrote:

> In https://reviews.llvm.org/D45050#1119974, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45050#1119973, @Charusso wrote:
> >
> > > In https://reviews.llvm.org/D45050#1116446, @lebedev.ri wrote:
> > >
> > > > I would like to see more negative tests.
> > > >  What does it do if the len/size is a constant?
> > > >  Variable that wasn't just assigned with `strlen()` return?
> > >
> > >
> > > Thanks for the comment! What do you mean by negative test?
> >
> >
> > The tests where the check matches and issues a warning is a positive test.
> >  The tests where the check does not match and/or does not issue a warning 
> > is a negative test.
>
>
> @lebedev.ri there is all the false positive results from the last publicated 
> result-dump:
>
> 1. F6334659: curl-lib-curl_path-c.html 
> 2. second result: F6334660: ffmpeg-libavformat-sdp.c.html 
> 
> 3. all result: F6334663: openssl-apps-ca-c.html 
> 
> 4. F6334665: postgresql-src-interfaces-ecpg-preproc-pgc-c.html 
> 
> 5. F6334667: redis-src-redis-benchmark-c.html 
> 
> 6. may false positive: F6334693: ffmpeg-libavformat-rdt-c.html 
> 
>
>   (Note: the two `memchr()` result were false positive in that post and there 
> is no new result with the new matcher.)
>
>   Does the new test cases cover your advice?


Not exactly.
I want to see **tests**.
I.e. they should be in `test/clang-tidy/bugprone-not-null-terminated-result-*`.
E.g. i did not find the following tests:

  void test1(char* dst, char* src) {
strcpy(dst, src, 10);
  }
  void test2(char* dst, char* src, int len) {
strcpy(dst, src, len);
  }
  void test3(char* dst, char* src) {
memcpy(dst, src, 10);
  }
  void test4(char* dst, char* src, int len) {
memcpy(dst, src, len);
  }
  ...




Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:377-381
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), ") + 1");
+Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+  } else {
+const auto InsertPlusOne =
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " + 1");

The fixits are incorrect.
Increment by `int(1)` can result in overflow, which is then extended to `size_t`
It should be `+ 1UL`.
https://godbolt.org/g/4nQiek



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c:158-180
+void bad_memset_1(char *dest, const char *src) {
+  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);
+}
+

??
These look like false-negatives.
How do you make an assumption about `dest` buffer from `src` string length?


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 150645.
Charusso added a comment.

Fix some comment.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 150644.
Charusso added a comment.

Clang format.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

@lebedev.ri there is all the false positive results from the last publicated 
result-dump:

F6334659: curl-lib-curl_path-c.html 
second result: F6334660: ffmpeg-libavformat-sdp.c.html 

all result: F6334663: openssl-apps-ca-c.html 
F6334665: postgresql-src-interfaces-ecpg-preproc-pgc-c.html 

F6334667: redis-src-redis-benchmark-c.html 

(Note: the two `memchr()` result were false positive in that post and there is 
no new result with the new matcher.)

Does the new test cases cover your advice?




Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:105
+
+   An integer non-zero value specifying if the target version implements ``_s``
+   suffixed memory and character handler functions which is safer than older

whisperity wrote:
> Why is this an integer, rather than a bool?
This is how the other Tidy checkers are use their options, I do not know either.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 150643.
Charusso marked 4 inline comments as done.
Charusso added a comment.

`memchr()` revision: it is problematic if the second argument is `'\0'`, there 
is no other case. Added a new type of matcher, to match function calls. More 
static functions and test cases and huge refactor.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void g

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Whisperity via Phabricator via cfe-commits]

whisperity requested changes to this revision.
whisperity added a comment.
This revision now requires changes to proceed.

In general, make sure the documentation page renders well in a browser.

Mostly style and phrasing stuff inline:




Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:34-36
+  // If non-zero the target version implements _s suffixed memory and character
+  // handler functions which is safer than older versions (e.g. 'memcpy_s()').
+  const int IsSafeFunctionsAreAvailable;

More of a language or phrasing thing, but the singular/plural wording is 
anything but matched in this case: handler functions which **are** safer. What 
is a "target version" in this case? Shouldn't it be something like "target 
environment" or "target standard" or just simply "target"?

The variable name is also problematic. `AreSafeFunctionsAvailable` would be 
better.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:20-21
+the passed third argument, which is ``size_t length``. The proper length is
+``strlen(src) + 1`` because the null terminator need an extra space. The result
+is badly not null-terminated:
+

//badly?// Also, perhaps a half sentence of explanation would be nice here:

need an extra space, thus the result is not null-terminated, which can result 
in undefined behaviour when the string is read.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:39
+
+Otherwise fix-it will rewrite it to a safer function, that born before ``_s``
+suffixed functions:

That //existed//.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:105
+
+   An integer non-zero value specifying if the target version implements ``_s``
+   suffixed memory and character handler functions which is safer than older

Why is this an integer, rather than a bool?


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In https://reviews.llvm.org/D45050#1119973, @Charusso wrote:

> In https://reviews.llvm.org/D45050#1116446, @lebedev.ri wrote:
>
> > I would like to see more negative tests.
> >  What does it do if the len/size is a constant?
> >  Variable that wasn't just assigned with `strlen()` return?
>
>
> Thanks for the comment! What do you mean by negative test?


To prove we have no false warnings.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45050#1119973, @Charusso wrote:

> In https://reviews.llvm.org/D45050#1116446, @lebedev.ri wrote:
>
> > I would like to see more negative tests.
> >  What does it do if the len/size is a constant?
> >  Variable that wasn't just assigned with `strlen()` return?
>
>
> Thanks for the comment! What do you mean by negative test?


The tests where the check matches and issues a warning is a positive test.
The tests where the check does not match and/or does not issue a warning is a 
negative test.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In https://reviews.llvm.org/D45050#1116446, @lebedev.ri wrote:

> I would like to see more negative tests.
>  What does it do if the len/size is a constant?
>  Variable that wasn't just assigned with `strlen()` return?


Thanks for the comment! What do you mean by negative test?


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I would like to see more negative tests.
What does it do if the len/size is a constant?
Variable that wasn't just assigned with `strlen()` return?


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In https://reviews.llvm.org/D45050#1116396, @Charusso wrote:

> In https://reviews.llvm.org/D45050#1116361, @xbolva00 wrote:
>
> > memcpy(crypt_buf, passwd, passwd_len); <--- warning
> >  memcpy(crypt_buf + passwd_len, salt, salt_len);
> >
> > This is a false warning since it appends strings using memcpy. But no idea 
> > what to do and if it is possible to avoid these false warnings.
>
>
> I have just tested it because of the `malloc()` function. I'm using 
> CodeChecker and leaved the default settings, so `IsSafeFunctionsAreAvailable 
> = 1`. Because of the malloc `strncpy_s()` cannot handle this case, but if the 
> check would ran with `IsSafeFunctionsAreAvailable = 0`, it rewrites it to 
> `strncpy(crypt_buf, passwd, passwd_len + 1)` which is a good transformation, 
> as the official `memcpy()`'s result not null-terminated.


Yeah, it is a valid recommendation.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In https://reviews.llvm.org/D45050#1116361, @xbolva00 wrote:

> memcpy(crypt_buf, passwd, passwd_len); <--- warning
>  memcpy(crypt_buf + passwd_len, salt, salt_len);
>
> This is a false warning since it appends strings using memcpy. But no idea 
> what to do and if it is possible to avoid these false warnings.


I have just tested it because of the `malloc()` function. I'm using CodeChecker 
and leaved the default settings, so `IsSafeFunctionsAreAvailable = 1`. Because 
of the malloc `strncpy_s()` cannot handle this case, but if the check would ran 
with `IsSafeFunctionsAreAvailable = 0`, it rewrites it to `strncpy(crypt_buf, 
passwd, passwd_len + 1)` which is a good transformation, as the official 
`memcpy()`'s result not null-terminated.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

memcpy(crypt_buf, passwd, passwd_len); <--- warning
memcpy(crypt_buf + passwd_len, salt, salt_len);

This is a false warning since it appends strings using memcpy. But no idea what 
to do and if it is possible to avoid these false warnings.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 149129.
Charusso added a comment.

Clang format.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: the result from calling 'wmemchr' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Thanks! Happy to see it is more powerful now :)


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 149116.
Charusso added a comment.

@xbolva00 idea implemented, doubled the checker's power. Now if the given 
argument is a DeclRefExpr this should handle it as it would be inlined as a 
simple CallExpr.

Results:
I have found 37 `memcpy()` and 2 `memchr()` errors in: curl, FFmpeg, git, 
memcached, OpenSSL, PostGreSQL, Redis, twin.

Example of this new DRE case:
F6291767: postgresql-src-common-md5-c.html 

Found `memchr()` errors:
F6291768: git-vcs-svn-fast_export-c.html 
F6291769: twin-clients-dialog-c.html 

All findings: F6291784: memcpy_and_memchr_errors.rar 



https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsn

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In https://reviews.llvm.org/D45050#1071897, @xbolva00 wrote:

> Shouldn't it catch in curl also this code?
>
> urllen = strlen(url_clone);
>
>   
>
> memcpy(newest, url_clone, urllen);
>
> Edit: if possible, report these bugs to project developers :)


Thanks for your idea @xbolva00, I will implement this feature, but currently I 
have problems with parens which cause ugly fix-its. After the review I will 
share the results with the devs.

In https://reviews.llvm.org/D45050#1071926, @xbolva00 wrote:

> Another idea if you want to implement it - check fopen.
>
> FILE *f = fopen("file", "r"); // read only
>  fputs("str", f); // we are writing -> boom, sigsegv or something like that.


Thanks for your sharing but I think I will move forward to Static Analyzer with 
my own projects.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 143825.
Charusso marked an inline comment as done.
Charusso added a comment.

Defined an option to switch off the `_s` suffixed functions usage.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: the result from calling 'wmemchr' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226
+DiagnosticBuilder &Diag) {
+  if (getLangOpts().CPlusPlus11) {
+StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";

Charusso wrote:
> aaron.ballman wrote:
> > Charusso wrote:
> > > aaron.ballman wrote:
> > > > What about C?
> > > The `else` part would fire.
> > I think this comment got moved to an unrelated area. I was talking about 
> > `NotNullTerminatedResultCheck::memmoveFix()`, where there is no `else` 
> > clause. However, I'm also not entirely certain why you are assuming the _s 
> > versions of those functions are available in C++ (they're in Annex K for C, 
> > which is an optional annex).
> The `else` clause is missing, according to the documentation: 
> 
> > - ``memmove``, ``wmemmove``:
> >   - C11: 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.
> > 
> >   - Before C11: The third argument gets a ``+ 1`` operation.
> 
> So the third argument as a token remains third if I add an extra second 
> argument as a string, that's why it's work well on both options.
I don't think you should assume the _s versions are available in C++, because 
they're not available everywhere. In fact, they're not available everywhere in 
C either (for instance, there is no Annex K implementation in glibc).

My recommendation would be to either gate this on the presence of a definition 
of the `__STDC_LIB_EXT1__` macro and that the ` __STDC_WANT_LIB_EXT1__` macro 
is defined but not defined to 0, or to make it a configurable option. I have a 
slight preference for the config option because not all implementations 
implement all of Annex K but still have a reasonable _s implementation of the 
Annex K APIs (like Microsoft's implementation) so I can picture users wanting a 
way to opt-in even without the macros.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 143698.
Charusso marked an inline comment as done.
Charusso added a comment.

Changed CXX11 to C11 and made a little refactor.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-c11.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-c11.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-c11.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-c11.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-c11.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-c11.c
@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_c11(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: the result from calling 'wmemchr' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result fro

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked an inline comment as done.
Charusso added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226
+DiagnosticBuilder &Diag) {
+  if (getLangOpts().CPlusPlus11) {
+StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";

aaron.ballman wrote:
> Charusso wrote:
> > aaron.ballman wrote:
> > > What about C?
> > The `else` part would fire.
> I think this comment got moved to an unrelated area. I was talking about 
> `NotNullTerminatedResultCheck::memmoveFix()`, where there is no `else` 
> clause. However, I'm also not entirely certain why you are assuming the _s 
> versions of those functions are available in C++ (they're in Annex K for C, 
> which is an optional annex).
The `else` clause is missing, according to the documentation: 

> - ``memmove``, ``wmemmove``:
>   - C11: 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.
> 
>   - Before C11: The third argument gets a ``+ 1`` operation.

So the third argument as a token remains third if I add an extra second 
argument as a string, that's why it's work well on both options.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226
+DiagnosticBuilder &Diag) {
+  if (getLangOpts().CPlusPlus11) {
+StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";

Charusso wrote:
> aaron.ballman wrote:
> > What about C?
> The `else` part would fire.
I think this comment got moved to an unrelated area. I was talking about 
`NotNullTerminatedResultCheck::memmoveFix()`, where there is no `else` clause. 
However, I'm also not entirely certain why you are assuming the _s versions of 
those functions are available in C++ (they're in Annex K for C, which is an 
optional annex).


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Another idea if you want to implement it - check fopen.

FILE *f = fopen("file", "r"); // read only
fputs("str", f); // we are writing -> boom, sigsegv or something like that.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Shouldn't it catch in curl also this code?

urllen = strlen(url_clone);
 
memcpy(newest, url_clone, urllen);


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

I tried my checker on 10 famous open source project, this is the outcome:

F5977628: curl_lib_transfer_c.html 
F5977629: ffmpeg_libavformat_sdp_c.html 
F5977631: openssl_apps_ca_c.html 
F5977633: postgresql_src_backend_libpq_auth_c.html 

F5977635: postgresql_src_backend_utils_adt_cash_c.html 

F5977636: postgresql_src_interfaces_ecpg_compatlib_informix_c.html 

F5977637: postgresql_src_interfaces_ecpg_pgtypeslib_datetime_c.html 



https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 143025.
Charusso marked an inline comment as done.
Charusso added a comment.

Everything fixed. Thanks you @lebedev.ri for the early comments, I forgot to 
submit my comment.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -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 *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  std::wmemcpy(dest, src, std::wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+  std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: the result from calling 'wmemchr' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+voi

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 14 inline comments as done.
Charusso added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226
+DiagnosticBuilder &Diag) {
+  if (getLangOpts().CPlusPlus11) {
+StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";

aaron.ballman wrote:
> What about C?
The `else` part would fire.



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp:11
+void bad_memcpy(char *dest, const char *src) {
+  memcpy(dest, src, strlen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is 
not null-terminated [bugprone-not-null-terminated-result]

lebedev.ri wrote:
> What about these functions, but in `std::` namespace?
It is covered in 
`test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp`, the 
test make sure my rename function doesn't touch the `std::` part.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:21
+
+static std::string exprToStr(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {

Do not name the parameter with the same name as a type. Same applies elsewhere 
in this file.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:21
+
+static std::string exprToStr(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {

aaron.ballman wrote:
> Do not name the parameter with the same name as a type. Same applies 
> elsewhere in this file.
Why returning a `std::string` rather than a `StringRef`? It seems like the 
callers don't always need the extra copy operation.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:36
+const MatchFinder::MatchResult &Result) {
+  auto NewExpr = const_cast(Expr);
+  ParenExpr *PE = new (Result.Context)

Can you sink the `const_cast` into the only place it's required, and remove the 
`clang::` from it?



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:40
+NewExpr->getLocEnd().getLocWithOffset(1), NewExpr);
+  return Expr == PE->getSubExpr();
+}

This seems really inefficient -- it's creating a new `ParentExpr` just to throw 
it away each time. Also, I'm not certain what it's accomplishing. The 
constructor for `ParenExpr` accepts an `Expr *` (in this case, `Expr`) which it 
returns from `ParenExpr::getSubExpr()`, so this looks like it's a noop that 
just tests `Expr == Expr`. What have I missed?



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:70
+  const auto LengthCall =
+  callExpr(callee(functionDecl(hasAnyName("strlen", "wcslen"))),
+   expr().bind("length-call"));

Please match on `::strlen` and `::wcslen` so that you don't accidentally catch 
random other function declarations.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:132-133
+
+  // clang-format off
+  const auto Alloca = Matcher("alloca", 1, 0, StrLenKind::WithoutInc);
+  const auto Calloc = Matcher("calloc", 2, 0, StrLenKind::WithoutInc);

Please use the fully-qualified names here as well. Also, I'm not keen on 
needing to shut off clang-format just to get the alignment to work below -- we 
don't typically try to keep things like this aligned unless there's a reason to 
need it (like making the code maintainable), which I don't think applies here.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:193
+  diag(Result.Nodes.getNodeAs("expr")->getLocStart(),
+   "'%0' function's allocated memory cannot hold the null-terminator")
+  << Name;

`null terminator` (remove the hyphen).

Also, it might be nice to reformulate to drop the possessive apostrophe on 
"function's". Perhaps something like: `"memory allocated by '%0' is 
insufficient to hold the null terminator"`

Same elsewhere in this file.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:210
+auto Diag = diag(Result.Nodes.getNodeAs("expr")->getLocStart(),
+ "'%0' function's result is not null-terminated")
+<< Name;

Similar here: `"the result from calling '%0' is not null-terminated"` (or 
something along those lines). Same elsewhere.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226
+DiagnosticBuilder &Diag) {
+  if (getLangOpts().CPlusPlus11) {
+StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s";

What about C?



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:306-307
+  auto Diag = diag(Result.Nodes.getNodeAs("expr")->getLocStart(),
+   "'strerror_s' function's result is not null-terminated and "
+   "missing the last character of the error message");
+  lengthArgInsertIncByOne(1, Result, Diag);

Similar rewording here to remove the possessive.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:321
+auto Diag = diag(FuncExpr->getLocStart(),
+ "'%0' function's comparison's length is too long")
+<< Name;

Given that this is a concern about the argument to the comparison function, I 
think this diagnostic should point to the argument rather than the function, 
and then it can drop the name of the function entirely to alleviate the awkward 
wording. Something like `"comparison length is too long and might lead to a 
buffer overflow"`.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:59
+  void
+ 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:60
 
+- New module `abseil` for checks related to the `Abseil `_
+  library.

Looks like you copied new version of file over old one. svn update and svn diff 
should be made instead.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140337.
Charusso marked 6 inline comments as done.
Charusso added a comment.

Truly fixed everything according to the comments.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -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 *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  std::wmemcpy(dest, src, std::wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+  std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'wmemchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:46
+/// from ' "data_name", "data", ' to just 'data' by "memmoving" it 
char-by-char.
+bool isTrimFunc(const MatchFinder::MatchResult &Result) {
+  const auto *FuncExpr = Result.Nodes.getNodeAs("expr");

Should be static too.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140330.
Charusso marked 2 inline comments as done.
Charusso added a comment.

Fixed everything according to the comments.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -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 *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  std::wmemcpy(dest, src, std::wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+  std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'wmemchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src))

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:21
+
+std::string exprToStr(const Expr *Expr,
+  const MatchFinder::MatchResult &Result) {

Please make it static. Same for other functions.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:56
+  FuncExpr->getArg(ArgPos)->IgnoreParenCasts())) {
+const auto LHSStr = exprToStr(BinOp->getLHS()->IgnoreParens(), Result);
+const auto RHSStr = exprToStr(BinOp->getRHS()->IgnoreParens(), Result);

Please don't use auto when type could not be deduced from statement itself 
(new, cast, iterator). Same for other places.



Comment at: docs/ReleaseNotes.rst:62
 
+- New `bugprone-not-null-terminated-result
+  
`_
 check

Please rebase for trunk, place check in alphabetical order in new checks list, 
use //:doc:// and proper link.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:4
+bugprone-not-null-terminated-result
+=
+

Is length same as length of title?



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:6
+
+This check can be used to find function calls where ``strlen`` or ``wcslen``
+are passed as an argument and cause a not null-terminated result. Depending on

Please synchronize with statement in Release Notes.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:29
+
+In addition to issuing warnings, "Fix-it" rewrite all the necessary code
+depending on the version number. The upper code would be the following:

Please use fix-it. Same for other places.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140294.
Charusso marked 5 inline comments as done.
Charusso added a comment.

Added the whole diff.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -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 *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  std::wmemcpy(dest, src, std::wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+  std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'wmemchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:30
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,

Please insert empty line above.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

you should always upload the whole diff, not just the last changes.


https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140288.
Charusso added a comment.

Fixed the unseen diff issues, removed redundant parentheses, the class got a 
private part.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -65,9 +65,6 @@
   Finds function calls where ``strlen`` or ``wcslen`` passed as argument and
   cause a not null-terminated result.
 
-- The 'misc-incorrect-roundings' check was renamed to 
`bugprone-incorrect-roundings
-  
`_
-
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
 - New `bugprone-throw-keyword-missing
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.h
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -27,6 +27,7 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,
const ast_matchers::MatchFinder::MatchResult &Result);
   void memHandlerFuncFix(StringRef Name,
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -1,3 +1,12 @@
+//===- 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"
@@ -49,7 +58,7 @@
 const auto StrLenArgStr = exprToStr(
 Result.Nodes.getNodeAs("length-call")->getArg(0), Result);
 
-return (StrLenArgStr == LHSStr || StrLenArgStr == RHSStr);
+return StrLenArgStr == LHSStr || StrLenArgStr == RHSStr;
   }
   return false;
 }


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -65,9 +65,6 @@
   Finds function calls where ``strlen`` or ``wcslen`` passed as argument and
   cause a not null-terminated result.
 
-- The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings
-  `_
-
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
 - New `bugprone-throw-keyword-missing
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.h
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -27,6 +27,7 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,
const ast_matchers::MatchFinder::MatchResult &Result);
   void memHandlerFuncFix(StringRef Name,
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -1,3 +1,12 @@
+//===- 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"
@@ -49,7 +58,7 @@
 const auto StrLenArgStr = exprToStr(
 Result.Nodes.getNodeAs("length-call")->getArg(0), Result);
 
-return (StrLenArgStr == LHSStr || StrLenArgStr == RHSStr);
+return StrLenArgStr == LHSStr || StrLenArgStr == RHSStr;
   }
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1
+#include "NotNullTerminatedResultCheck.h"
+#include "clang/AST/ASTContext.h"

Missing header blurb



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:14
+  const MatchFinder::MatchResult &Result) {
+  return Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Expr->getSourceRange()),

Doesn't `Lexer::getSourceText()` return `StringRef`?



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:264
+const auto FirstArg = FuncExpr->getArg(0);
+std::string NewSecondArg = " strlen(" + exprToStr(FirstArg, Result) + "),";
+

This should probably be `SmallString<32>`



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:40
+   DiagnosticBuilder &Diag);
+  void memchrFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,

Why are all these internal functions `public`?
They should be either in anonymous namespace (best), or at least have `private` 
visibility.



Comment at: docs/ReleaseNotes.rst:68
+
+- The 'misc-incorrect-roundings' check was renamed to 
`bugprone-incorrect-roundings
+  
`_

This seems out-of-place. Why is this in the diff?



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp:11
+void bad_memcpy(char *dest, const char *src) {
+  memcpy(dest, src, strlen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is 
not null-terminated [bugprone-not-null-terminated-result]

What about these functions, but in `std::` namespace?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added reviewers: alexfh, aaron.ballman, xazax.hun.
Charusso added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, rnkovacs, mgorny.

New checker called bugprone-not-null-terminated-result. This check can be used 
to find function calls where `strlen` or `wcslen` are passed as an argument and 
cause a not null-terminated result. Usually the proper length is the source's 
own length + 1: `strlen(src) + 1` because the null-terminator need an extra 
space. Depending on the case of use, it may insert or remove the increase 
operation to ensure the result is null-terminated.

The following function calls are checked:
`alloca`, `calloc`, `malloc`, `realloc`, `memcpy`, `wmemcpy`, `memcpy_s`, 
`wmemcpy_s`, `memchr`, `wmemchr`, `memmove`, `wmemmove`, `memmove_s`, 
`wmemmove_s`, `memset`, `wmemset`, `strerror_s`, `strncmp`, `wcsncmp`, 
`strxfrm`, `wcsxfrm`

Example problematic code:

  void bad_memcpy(char *dest, const char *src) {
memcpy(dest, src, strlen(src));
  }

After running the tool it would be the following if the target is C++11:

  void good_memcpy_cxx11(char *dest, const char *src) {
strncpy_s(dest, src, strlen(src));
  }

or if the target is older, then it would be following:

  void good_memcpy_not_cxx11(char *dest, const char *src) {
strncpy(dest, src, (strlen(src) + 1));
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -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 *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t