[clang] [analyzer] [MallocChecker] Less aggressive analysis of functions (PR #116383)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Yan (likeamahoney)

<details>
<summary>Changes</summary>

It seems to me that the `unix.Malloc` checker is too aggressive when it comes to analyzing function arguments.

For example it warns on such code (there is no directly usage of freed memory - but just pointer copying):

```cpp
#include "stdlib.h"

static void test_empty(int* p) {
  return;
}

void test() {
  int *p = (int*)malloc(sizeof(int));
  free(p);

  test_empty(p);  //  use after free warning causing
}
```

it leads to fp like that:

```cpp
#include "stdlib.h"

static void test_realloc(int* p) { 
  p = (int*)malloc(sizeof(int));
  free(p);
  return;
}

void test() {
  int *p = (int*)malloc(sizeof(int));
  free(p);

  test_realloc(p);  //  use after free warning causing
}
```

I believe that when a function is from standard library and not external (it has a definition in the compilation unit) then checker can continue to analyze the function body further until an actual freed memory use is encountered but not warn on the call arguments in the case where there is simply copying of a pointer, and not its dereferencing.

---
Full diff: https://github.com/llvm/llvm-project/pull/116383.diff


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+7) 
- (modified) clang/test/Analysis/malloc-interprocedural.c (+36-2) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 4166cf14391e2d..71a66f3ad2544a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3156,12 +3156,16 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
     return;
   }
 
+  bool HasDef = false;
+
   // We will check for double free in the post visit.
   if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
     const FunctionDecl *FD = FC->getDecl();
     if (!FD)
       return;
 
+    HasDef = FD->getDefinition() != nullptr;
+
     if (ChecksEnabled[CK_MallocChecker] && isFreeingCall(Call))
       return;
   }
@@ -3173,6 +3177,9 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
       return;
   }
 
+  if (ChecksEnabled[CK_MallocChecker] && !Call.isInSystemHeader() && HasDef)
+    return;
+
   // Check arguments for being used after free.
   for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) {
     SVal ArgSVal = Call.getArgSVal(I);
diff --git a/clang/test/Analysis/malloc-interprocedural.c b/clang/test/Analysis/malloc-interprocedural.c
index 5e5232af7b46e8..b6979554a116d6 100644
--- a/clang/test/Analysis/malloc-interprocedural.c
+++ b/clang/test/Analysis/malloc-interprocedural.c
@@ -26,7 +26,7 @@ static void *my_malloc2(int elevel, size_t size) {
 }
 
 static void my_free1(void *p) {
-  free(p);
+  free(p); // expected-warning {{Attempt to free released memory}}
 }
 
 static void test1(void) {
@@ -65,7 +65,7 @@ int test4(void) {
 void test6(void) {
   int *data = (int *)my_malloc2(1, 4);
   my_free1((int*)data);
-  my_free1((int*)data); // expected-warning{{Use of memory after it is freed}}
+  my_free1((int*)data);
 }
 
 // TODO: We should warn here.
@@ -98,3 +98,37 @@ int uafAndCallsFooWithEmptyReturn(void) {
   fooWithEmptyReturn(12);
   return *x; // expected-warning {{Use of memory after it is freed}}
 }
+
+static void test_empty(int *p) { return; }
+
+void test7() {
+  int *p = malloc(sizeof(int));
+  free(p);
+
+  test_empty(p); // no-warning
+}
+
+static void test_realloc(int *p) {
+  p = (int *)malloc(12);
+  free(p);
+  return;
+}
+
+void test8() {
+  int *p = malloc(sizeof(int));
+  free(p);
+
+  test_realloc(p); // no-warning
+}
+
+static void test_interproc_ptr_use(int *p) {
+  *p = 1; // expected-warning {{Use of memory after it is freed}}
+  return;
+}
+
+void test9() {
+  int *p = malloc(sizeof(int));
+  free(p);
+
+  test_interproc_ptr_use(p);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/116383