[PATCH] D97699: [analyzer] Add InvalidPtrChecker

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze added a comment.

@NoQ, thanks for the comments.

In D97699#2601804 <https://reviews.llvm.org/D97699#2601804>, @NoQ wrote:

> ... and whether flagged code is expected to be always invalid.

C standard says "may be overwritten", so I guess it's undefined behavior.

In D97699#2601804 <https://reviews.llvm.org/D97699#2601804>, @NoQ wrote:

> ... I want you to explain what *exactly* does the checker checks ...

There are two problems that are checked. 
First: if we have 'envp' argument of main present and we modify the environment in some way (say setenv, putenv...), it "may" reallocate the whole environment memory, but 'envp' will still point to the old location. 
Second: if we have a pointer pointing to the result of 'getenv' (and some other non-reentrant functions, there are 5 of them implemented now, but can be easily extended to others, since checker is general enough) and we call 'getenv' again the previous result "may" be overwritten.
The idea of the checker is simple and somewhat comparable to MallocChecker or UseAfterFree (in a sense that 'free' invalidates region).  We have a map of previous function call results and after a subsequent call, we invalidate the previous region. And we warn on dereference of such pointer or if we have it as an argument to function call (to avoid some false negatives), which is not inlined.
I will add a description in a checker file.

And about `checkDeadSymbols`,  I get your point, but I am interested why checker has different behavior on these two examples:

  int main(int argc, char **argv, char *envp[]) {
    putenv((char*) "NAME=VALUE"); // envp invalidated
    envp[0]; // gives error
  }



  int main(int argc, char **argv, char *envp[]) {
    char **e = envp;
    putenv((char*) "NAME=VALUE"); // envp invalidated
    e[0]; // does not give error
  } 

If I manually force keeping `envp` region in state when I check `if (!SymReaper.isLiveRegion(E)) {` , then it reports as expected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97699/new/

https://reviews.llvm.org/D97699