[PATCH] D48205: [analyzer] Assert that nonloc::SymbolVal always wraps a non-Loc-type symbol.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

`nonloc::SymbolVal` that contains a pointer-type or reference-type symbol is ill-formed; our code isn't prepared to work with such values. The canonical way of representing symbolic pointers is `loc::MemRegionVal` that wraps a `SymbolicRegion` for the respective symbol. For representing results of casting pointers into integers we have `nonloc::LocAsInteger`.

This is the one assertion that i regret accidentally omitting in https://reviews.llvm.org/D26837, because it's very fundamental.

The assertion indeed mostly holds on our tests; i found one violation (in my own code), but the ill-formed `SVal` was only used in intermediate computations and was never put into the program state.

https://bugs.llvm.org/show_bug.cgi?id=37802 contains another example of an ill-formed `SVal` of this kind, which causes a crash. This patch doesn't address that crash yet.


Repository:
  rC Clang

https://reviews.llvm.org/D48205

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp


Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1238,7 +1238,7 @@
 
     SVal VisitSymbolData(const SymbolData *S) {
       if (const llvm::APSInt *I =
-              SVB.getKnownValue(State, nonloc::SymbolVal(S)))
+              SVB.getKnownValue(State, SVB.makeSymbolVal(S)))
         return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
                                             : (SVal)SVB.makeIntVal(*I);
       return SVB.makeSymbolVal(S);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -343,11 +343,14 @@
 
 namespace nonloc {
 
-/// Represents symbolic expression.
+/// Represents symbolic expression that isn't a location.
 class SymbolVal : public NonLoc {
 public:
   SymbolVal() = delete;
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) { assert(sym); }
+  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {
+    assert(sym);
+    assert(!Loc::isLocType(sym->getType()));
+  }
 
   SymbolRef getSymbol() const {
     return (const SymExpr *) Data;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48205.151453.patch
Type: text/x-patch
Size: 1374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180615/6bbc9e03/attachment-0001.bin>