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

[NoQ]

Summary

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 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.

[george.karpenkov]

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

1241

So what is the difference here? That SVB.makeSymbolVal is not always a nonloc?

[NoQ]

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

1241

That's right; it will be a symbolic region instead.

It won't affect the result of the calculation though (hopefully) (in the current implementation).

1241

I mean, it will *sometimes* be a symbolic region (when the symbol is of pointer type).

include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

 };
 
 //==------------------------------------------------------------------------==//
 //  Subclasses of NonLoc.
 //==------------------------------------------------------------------------==//
 
 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;
   }
 
   bool isExpression() const {
     return !isa<SymbolData>(getSymbol());
   }

lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

class Simplifier : public FullSValVisitor<Simplifier, SVal> {
     }
 
   public:
     Simplifier(ProgramStateRef State)
         : State(State), SVB(State->getStateManager().getSValBuilder()) {}
 
     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);
     }
 
     // TODO: Support SymbolCast. Support IntSymExpr when/if we actually
     // start producing them.