[Static Analyzer] Fix false positive in Clang Static Analyzer

[karthikthecool]

Summary

Hi All,
We are getting few false positive in our project when we use clang SA.
Consider the below code in mylib.c(library) and main.c -
In lib.c

int* myFn(const int* v){
  int* k = v;
  return k;
}

In main.c

int* myFn(const int* v);

int main() {
  int* p = (int*)malloc(sizeof(int));
  int* k = myFn(p);
  free(k);
  return 0;
}

in the above code we don't have any memory leak as free(k) free's the memory allocated by malloc.But we get a false positive (memory leak by 'p') here. The problem seems to be that when we encorter myFn(p) which is a lib call we should have marked p as escaped but we dont seem to do so for malloced region for some reason. Any particular reason we only mark ConstPointerEscaped when it is from NewOrNewArrayFamily?
In this patch have modified the checkConstPointerEscape to mark const pointer as escaped even if it is a malloced region.

Please let me know if this is good to commit or if this check was not handled specifically for some reason.
Awaiting your valuable inputs.

Thanks and Regards
Karthik Bhat

[karthikthecool]

ping.

[zaks.anna]

I'd say this works as expected. We base this heuristic on the fact that "free" cannot be called on "const int*".

Your sample code breaks this assumption, but:
warning: initializing 'int *' with an expression of type 'const int *' discards qualifiers

  [-Wincompatible-pointer-types-discards-qualifiers]
int* k = v;

[dblaikie]

You've got the right people on the 'to' list, but the wrong mailing list -
this should go to cfe-commits (this might explain the lack of response, too

• those people might deprioritize email to llvm-commits as they don't

actively work on the LLVM subproject)

[karthikthecool]

Thanks Anna i think it makes sense.
Yes David i noticed it just now after you pointed out i had incorrectly added llvm-commit insted of cfe.

Thanks
Karthik Bhat

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
   // Most system calls do not free the memory.
   return false;
 }
 
 static bool retTrue(const RefState *RS) {
   return true;
 }
 
-static bool checkIfNewOrNewArrayFamily(const RefState *RS) {
+static bool checkIfAllocatedFamily(const RefState *RS) {
   return (RS->getAllocationFamily() == AF_CXXNewArray ||
-          RS->getAllocationFamily() == AF_CXXNew);
+          RS->getAllocationFamily() == AF_CXXNew ||
+          RS->getAllocationFamily() == AF_Malloc);
 }
 
 ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
                                              const InvalidatedSymbols &Escaped,
                                              const CallEvent *Call,
                                              PointerEscapeKind Kind) const {
   return checkPointerEscapeAux(State, Escaped, Call, Kind, &retTrue);
 }
 
 ProgramStateRef MallocChecker::checkConstPointerEscape(ProgramStateRef State,
                                               const InvalidatedSymbols &Escaped,
                                               const CallEvent *Call,
                                               PointerEscapeKind Kind) const {
   return checkPointerEscapeAux(State, Escaped, Call, Kind,
-                               &checkIfNewOrNewArrayFamily);
+                               &checkIfAllocatedFamily);
 }
 
 ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State,
                                               const InvalidatedSymbols &Escaped,
                                               const CallEvent *Call,
                                               PointerEscapeKind Kind,
                                   bool(*CheckRefState)(const RefState*)) const {
   // If we know that the call does not free memory, or we want to process the

test/Analysis/malloc.c

void r11160612_1() {
   char *x = malloc(12);
   const_ptr_and_callback(0, x, 12, free); // no - warning
 }
 
 // Null is passed as callback.
 void r11160612_2() {
   char *x = malloc(12);
   const_ptr_and_callback(0, x, 12, 0);
-} // expected-warning {{leak}}
+}  // no - warning
 
 // Callback is passed to a function defined in a system header.
 void r11160612_4() {
   char *x = malloc(12);
   sqlite3_bind_text_my(0, x, 12, free); // no - warning
 }
 
 // Passing callbacks in a struct.
char *testLeakWithinReturn(char *str) {
   return strdup(strdup(str)); // expected-warning{{leak}}
 }
 
 void passConstPtr(const char * ptr);
 
 void testPassConstPointer() {
   char * string = malloc(sizeof(char)*10);
   passConstPtr(string);
-  return; // expected-warning {{leak}}
+  return;  //no-warning
 }
 
 void testPassConstPointerIndirectly() {
   char *p = malloc(1);
   p++;
   memcmp(p, p, sizeof(&p));
   return; // expected-warning {{leak}}
 }

test/Analysis/malloc.cpp

void r11160612_3() {
   char *x = (char*)malloc(12);
   const_ptr_and_callback_def_param(0, x, 12);
 }
 
 int const_ptr_and_callback_def_param_null(int, const char*, int n, void(*)(void*) = 0);
 void r11160612_no_callback() {
   char *x = (char*)malloc(12);
   const_ptr_and_callback_def_param_null(0, x, 12);
-} // expected-warning{{leak}}
+} // no-warning
 
 // Test member function pointer.
 struct CanFreeMemory {
   static void myFree(void*);
 };
 //This is handled because we look at the type of the parameter(not argument).
 void r11160612_3(CanFreeMemory* p) {
   char *x = (char*)malloc(12);

 void appendNested(NestedProperty x);
 
 void appendWrapperNested(char *getterName) {
   appendNested(NestedProperty(Property(getterName)));
 }
 void fooNested(const char* name) {
   char* getterName = strdup(name);
   appendWrapperNested(getterName); // no-warning
-}
+}