[PATCH] D97699: [analyzer] Add InvalidPtrChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

CERT rules are typically very vague and don't map nicely into specific static analysis algorithms. A lot of CERT rules flag valid code as well as bugs. I want you to explain what *exactly* does the checker checks (say, in the form of a state machine, i.e. what //sequences of events in the program// does it find) and whether flagged code is expected to be always invalid.

> `// warnOnDeadSymbol reports 'envp' dead here`

If it reports the symbol as dead after the warning would have been emitted then the problem must be elsewhere. There must be other differences in the analysis *before* the buggy statement. Nothing that happens after the buggy statement should ever matter.

Also the behavior of `checkDeadSymbols` looks correct here, I don't see any problems from your description. The symbol is reported dead when there's a guarantee that it won't be encountered anymore during analysis. If the parameter variable and the aliasing variable aren't used anymore and the symbol isn't stored anywhere else then the symbol is correctly marked as dead.

Why are you even tracking `reg_$0<envp>`? It's obvious from the structure of the symbol that it's an environment pointer, there's no need to keep it in maps.



================
Comment at: clang/docs/analyzer/checkers.rst:2103-2104
+        puts(envp[i]);
+        // envp may no longer point to the current environment
+        // this program has unanticipated behavior.
+      }
----------------
It's very important to explain whether the "unanticipated behavior" is an entirely psychological concept ("most people don't understand how this works but this can occasionally also be a valid solution if you know what you're doing") or a 100% clear indication of a bug ("such code can never be correct", eg. it instantly causes undefined behavior, or the written value can never be read later so there's absolutely no point in writing it, or something like that).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97699