[PATCH] D77062: [analyzer] Added check for unacceptable equality operation between Loc and NonLoc types

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:273
   Optional<DefinedSVal> val = V.getAs<DefinedSVal>();
-  if (!val)
-    return std::pair<ProgramStateRef , ProgramStateRef >(state, state);
+  if (val && !V.getAs<nonloc::LazyCompoundVal>()) {
+     // return pair shall be {null, non-null} so reorder states
----------------
Basically `SVal::getAs<>` should not be used for discovering the type of the value; only the exact representation that's used for the value of the type that's already in your possession. Say, it's ok to use it to discriminate between, say, compound value and lazy compound value. It's ok to use it to discriminate between a concrete integer and a symbol. It's ok to use it do discriminate between a known value and an unknown value. But if it's used for discriminating between a compound value and a numeric symbol, i'm 99% sure it's incorrect. You should already know the type from the AST before you even obtain the value. It doesn't make sense to run the checker at all if the function receives a structure. And if it doesn't receive the structure but the run-time value is of structure type, then either the checker isn't obtaining the value correctly or there's bug in path-sensitive analysis. That's why i still believe you're only treating the symptoms. There's nothing normal in the situation where "strcpy suddenly accepts a structure (or an array) by value".


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

https://reviews.llvm.org/D77062