[PATCH] D37120: [analyzer] Fix modeling arithmetic

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:371-373
+            return makeSymExprValNN(
+                state, op, lhs.castAs<nonloc::LocAsInteger>(),
+                rhs.castAs<nonloc::ConcreteInt>(), resultTy);
----------------
NoQ wrote:
> For now this code would return `UnknownVal` in most cases (pointer is not tainted, or not symbolic, or contains an offset), and still construct an invalid symbol in the rest of the cases (`makeSymExprValNN` would add the number to the pointer symbol, instead of modelling an offset within the pointed-to region).
> 
> Once D35450 finally lands (sorry for the delays...), it'd return `UnknownVal` less often and crash more often.
@NoQ - many thanks for the code review!

i assume i might be missing smth, anyway, (replying to this comment and another one below)

  >This is probably the way to go: just take the Loc behind the LocAsInteger, cast it to char * (because 
  >pointer type shouldn't affect how much bytes of offset are added, anymore), add your integer to it, 
  >pack back into LocAsInteger

i'm not sure i understand how that would work here and somehow don't like it. 
For example, op can be MultiplicativeOp (i.e. long y = ((long)p) * 13;) extracting the Loc and doing smth with the Loc -  doesn't seem to be the right thing (if i understood your suggestion correctly)

  >For now this code would return UnknownVal in 
  >most cases (pointer is not tainted, or not symbolic, 
  >or contains an offset)

that was my understanding what this code should do (return UnknownVal in most cases unless we can reason about the actual (integer) values of LHS,RHS)




Repository:
  rL LLVM

https://reviews.llvm.org/D37120