Page MenuHomePhabricator

[Static Analyzer] Fix false positive in Clang Static Analyzer
AbandonedPublic

Authored by karthikthecool on Dec 29 2014, 5:10 AM.

Details

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

Diff Detail

Repository
rL LLVM

Event Timeline

karthikthecool retitled this revision from to [Static Analyzer] Fix false positive in Clang Static Analyzer.
karthikthecool updated this object.
karthikthecool edited the test plan for this revision. (Show Details)
karthikthecool set the repository for this revision to rL LLVM.
karthikthecool added a subscriber: Unknown Object (MLST).
zaks.anna edited edge metadata.Jan 22 2015, 9:19 AM

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;

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 abandoned this revision.Jan 23 2015, 1:05 AM

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