[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

Minor spelling nits inline




Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:30
+  callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"
+  .bind("aquisition"),
+  this);

Spelling nit: acquisition

But I wonder if there's a simpler word you can use for this, e.g. "alloc"



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:47
+
+  if ((Call = Result.Nodes.getNodeAs("aquisition")))
+Recommendation = "consider a container or a smart pointer";

"aquisition" as above


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D26167#622792, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D26167#621942, @JonasToth wrote:
>
> > what do you think about configuration of the allocating functions? e.g. for 
> > aligned memory you must use OS-specific allocation functions.
> >  should the check catch custom allocation functions as well?
>
>
> Sounds like a good idea to me.


I think this check should also warn about `strdup()`, `strndup()` and 
`wcsdup()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D26167#621942, @JonasToth wrote:

> what do you think about configuration of the allocating functions? e.g. for 
> aligned memory you must use OS-specific allocation functions.
>  should the check catch custom allocation functions as well?


Sounds like a good idea to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

alright, i will try to get the functionpointers as well.
what do you think about configuration of the allocating functions? e.g. for 
aligned memory you must use OS-specific allocation functions.
should the check catch custom allocation functions as well?

is there a way to make a list of allocation, reallocation, deallocation 
functions that the check shall look for (+aliases to these)?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D26167#621928, @malcolm.parsons wrote:

> I tried this check on my company's codebase.
>  It misses cases where malloc is used through a function pointer.
>  Should clang-tidy warn about making a function pointer to these functions?


Sounds like a good idea to me. I can't really imagine a scenario where you'd 
want to enable this check and not be warned about it.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

I tried this check on my company's codebase.
It misses cases where malloc is used through a function pointer.
Should clang-tidy warn about making a function pointer to these functions?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

nit: All of the new files had UTF-8 BOMs, which I don't think we usually add.




Comment at: test/clang-tidy/cppcoreguidelines-no-malloc.cpp:3
+
+using size_t = unsigned long;
+

This test did not pass on 32-bit Windows. I will change it to:
  using size_t = __SIZE_TYPE__;


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Committed in r289546.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D26167#621106, @alexfh wrote:

> Jonas, do you need someone to commit the patch for you?


i think so, yes. can i trigger somehow automatically, that this will go into 
the code base somehow?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

tbh. i dont know anything how the clang workflow works :)

do i need to ask someone to commit, or what be my next step?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Jonas, do you need someone to commit the patch for you?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons accepted this revision.
malcolm.parsons added a comment.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Hit "submit" too early.

LG, but please wait until Malcolm is happy with the change as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG, but please wait until Malcolm

> Whats not nice is, that there is no underlining (in test neither), but i 
> could not find out what is wrong,
>  since i supply a SourceRange in the diag()-call.

Please leave the range anyway. I suspect clang-tidy doesn't handle ranges in 
diagnostics correctly, but it's easier to fix this, if there is an example 
already.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 4 inline comments as done.
JonasToth added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

malcolm.parsons wrote:
> JonasToth wrote:
> > malcolm.parsons wrote:
> > > Do you mean `#r10-avoid-malloc-and-free`?
> > yes, but i think the wider view is more appropriate. RAII is a general 
> > strategy, and not specific to malloc etc.
> This checker is specific to R.10.
> The documentation for all the other cppcoreguidelines checks mention which 
> rule they're enforcing.
alright. i will adjust.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81208.
JonasToth added a comment.

adjusted doc.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* s = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index: clang-tidy/cppcoreguidelines/NoMallocCheck.h

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

JonasToth wrote:
> malcolm.parsons wrote:
> > Do you mean `#r10-avoid-malloc-and-free`?
> yes, but i think the wider view is more appropriate. RAII is a general 
> strategy, and not specific to malloc etc.
This checker is specific to R.10.
The documentation for all the other cppcoreguidelines checks mention which rule 
they're enforcing.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:4
+cppcoreguidelines-no-malloc
+===
+

A few `=` missing.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81204.
JonasToth added a comment.

fix missing variable in doc


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+`
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* s = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index: clang-tidy/cppcoreguidelines/NoMallocCheck.h

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:29
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"
+  .bind("aquisition"),

malcolm.parsons wrote:
> C memory management functions are also present in the `std` namespace.
in the clang codebase, std::malloc got catched as well. see the 
tidy_output_no_malloc.txt in one of my previous comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

malcolm.parsons wrote:
> Do you mean `#r10-avoid-malloc-and-free`?
yes, but i think the wider view is more appropriate. RAII is a general 
strategy, and not specific to malloc etc.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:26
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct 
some_struct));
+

malcolm.parsons wrote:
> Missing variable.
that slipped through. i fix it.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:29
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"
+  .bind("aquisition"),

C memory management functions are also present in the `std` namespace.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

Do you mean `#r10-avoid-malloc-and-free`?



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:26
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct 
some_struct));
+

Missing variable.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I did run clang-tidy with $(./run-clang-tidy.py 
-checks=-*,cppcoreguidelines-no-malloc). The resulting output is in the 
appended file.
Since all warnings came from header files (llvms custom containers?) there are 
always the same warnings for all source files.
Every function (calloc, malloc, realloc and free) was seen, i hope the output 
is sufficient.

Whats not nice is, that there is no underlining (in test neither), but i could 
not find out what is wrong, since i supply a SourceRange in the diag()-call.

F2703383: tidy_output_no_malloc.txt 


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81193.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

last fixes in the code, from alex


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+`
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

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

One important thing is missing. Please run this check on a large enough 
codebase (LLVM + Clang is a good choice for testing most of kinds of checks 
usually) and include a summary of results in the patch description. The most 
important things are: clang-tidy doesn't crash, results (or if there are too 
many, then a sufficiently large random sample - say, 100) look good on manual 
inspection.




Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:48
+  if ((Call = Result.Nodes.getNodeAs("aquisition")))
+Recommendation = "consider a container or smart pointer";
+

nit: `a smart pointer` (an article is missing).



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:49
+Recommendation = "consider a container or smart pointer";
+
+  else if ((Call = Result.Nodes.getNodeAs("realloc")))

Please remove empty lines before `else`


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81103.
JonasToth added a comment.

fixes last review comments from alex


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+`
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index: clang-tidy/cppcoreguidelines/NoMallocCheck.h

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 3 inline comments as done.
JonasToth added a comment.

removed the function calls, a little doc tidy as well


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:54
+
+void NoMallocCheck::handleAquisition(const CallExpr *AquisitionCall) {
+  diag(AquisitionCall->getLocStart(),

The handle.* methods now just add more bulk and bring no benefit. With a single 
statement in each function they might as well be inlined and the `check`method 
would be still quite compact:
```
CallExpr Call = nullptr;
StringRef Recommendation;
if ((Call = Result.Nodes.getNodeAs("aquisition")))
  Recommendation = "consider a container or smartpointer";
else if ((Call = ...))
  ...;

assert(Call && "Unhandled binding in the matcher);

diag(Call->getLocStart(),
 "do not allocate memory manually; %0")
  << Recommendation
  << SourceRange(...);
```



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:56
+  diag(AquisitionCall->getLocStart(),
+   "do not allocate memory manually; consider a container or smartpointer")
+  << SourceRange{AquisitionCall->getLocStart(),

"smart pointer" is two words.



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:57
+   "do not allocate memory manually; consider a container or smartpointer")
+  << SourceRange{AquisitionCall->getLocStart(),
+ AquisitionCall->getLocEnd()};

Please use parentheses instead of curly braces for constructor calls 
(`SourceRange(x, y)`), it's more common in LLVM code.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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