[analyzer] Add LocationContext to SymbolMetadata

[a.sidorin]

Summary

Currently, SymbolMetadata are distinguished by statement and block count. However, in case of inlining, block counts may be the same every time function is called because the counter restarts from zero (it is related to StackFrameContext which is always different). This leads to unexpected resurrection of SymbolMetadata.

This patch solves this problem by adding a LocationContext to SymbolMetadata. A simple test is provided as well.

[NoQ]

I also never noticed that the block count gets reset on every stack frame, that's pretty unobvious. Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count itself seems to be pretty complicated.

Also, maybe once the ScopeContext thing is ready, we'd be able to omit the block count completely.

[a.sidorin]

In D21978#476200, @NoQ wrote:

I also never noticed that the block count gets reset on every stack frame, that's pretty unobvious.

Yes, that was surprising for me too.

Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count itself seems to be pretty complicated.

Also, maybe once the ScopeContext thing is ready, we'd be able to omit the block count completely.

Not sure. According to review comments, we may want for ScopeContext to be created for scopes with variables declared only (it seems reasonable). So ScopeContext may cover multiple blocks.

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

public:
   /// Note, the expression should represent a location.
   DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
                                                 const LocationContext *LCtx,
                                                 unsigned Count);
 
   DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
       SymbolRef parentSymbol, const TypedValueRegion *region);
 
-  DefinedSVal getMetadataSymbolVal(
-      const void *symbolTag, const MemRegion *region,
-      const Expr *expr, QualType type, unsigned count);
+  DefinedSVal getMetadataSymbolVal(const void *symbolTag,
+                                   const MemRegion *region,
+                                   const Expr *expr, QualType type,
+                                   const LocationContext *LCtx,
+                                   unsigned count);
 
   DefinedSVal getFunctionPointer(const FunctionDecl *func);
   
   DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
                               const LocationContext *locContext,
                               unsigned blockCount);
 
   /// Returns the value of \p E, if it can be determined in a non-path-sensitive

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h

 /// SymbolMetadata - Represents path-dependent metadata about a specific region.
 ///  Metadata symbols remain live as long as they are marked as in use before
 ///  dead-symbol sweeping AND their associated regions are still alive.
 ///  Intended for use by checkers.
 class SymbolMetadata : public SymbolData {
   const MemRegion* R;
   const Stmt *S;
   QualType T;
+  const LocationContext *LCtx;
   unsigned Count;
   const void *Tag;
 public:
   SymbolMetadata(SymbolID sym, const MemRegion* r, const Stmt *s, QualType t,
-                 unsigned count, const void *tag)
-  : SymbolData(SymbolMetadataKind, sym), R(r), S(s), T(t), Count(count), Tag(tag) {}
+                 const LocationContext *LCtx, unsigned count, const void *tag)
+  : SymbolData(SymbolMetadataKind, sym), R(r), S(s), T(t), LCtx(LCtx),
+    Count(count), Tag(tag) {}
 
   const MemRegion *getRegion() const { return R; }
   const Stmt *getStmt() const { return S; }
+  const LocationContext *getLocationContext() const { return LCtx; }
   unsigned getCount() const { return Count; }
   const void *getTag() const { return Tag; }
 
   QualType getType() const override;
 
   void dumpToStream(raw_ostream &os) const override;
 
   static void Profile(llvm::FoldingSetNodeID& profile, const MemRegion *R,
-                      const Stmt *S, QualType T, unsigned Count,
-                      const void *Tag) {
+                      const Stmt *S, QualType T, const LocationContext *LCtx,
+                      unsigned Count, const void *Tag) {
     profile.AddInteger((unsigned) SymbolMetadataKind);
     profile.AddPointer(R);
     profile.AddPointer(S);
     profile.Add(T);
+    profile.AddPointer(LCtx);
     profile.AddInteger(Count);
     profile.AddPointer(Tag);
   }
 
   void Profile(llvm::FoldingSetNodeID& profile) override {
-    Profile(profile, R, S, T, Count, Tag);
+    Profile(profile, R, S, T, LCtx, Count, Tag);
   }
 
   // Implement isa<T> support.
   static inline bool classof(const SymExpr *SE) {
     return SE->getKind() == SymbolMetadataKind;
   }
 };
 
public:
 
   const SymbolExtent *getExtentSymbol(const SubRegion *R);
 
   /// \brief Creates a metadata symbol associated with a specific region.
   ///
   /// VisitCount can be used to differentiate regions corresponding to
   /// different loop iterations, thus, making the symbol path-dependent.
   const SymbolMetadata *getMetadataSymbol(const MemRegion *R, const Stmt *S,
-                                          QualType T, unsigned VisitCount,
+                                          QualType T,
+                                          const LocationContext *LCtx,
+                                          unsigned VisitCount,
                                           const void *SymbolTag = nullptr);
 
   const SymbolCast* getCastSymbol(const SymExpr *Operand,
                                   QualType From, QualType To);
 
   const SymIntExpr *getSymIntExpr(const SymExpr *lhs, BinaryOperator::Opcode op,
                                   const llvm::APSInt& rhs, QualType t);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

if (Recorded)
       return *Recorded;
   }
 
   // Otherwise, get a new symbol and update the state.
   SValBuilder &svalBuilder = C.getSValBuilder();
   QualType sizeTy = svalBuilder.getContext().getSizeType();
   SVal strLength = svalBuilder.getMetadataSymbolVal(CStringChecker::getTag(),
                                                     MR, Ex, sizeTy,
+                                                    C.getLocationContext(),
                                                     C.blockCount());
 
   if (!hypothetical) {
     if (Optional<NonLoc> strLn = strLength.getAs<NonLoc>()) {
       // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4
       BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
       const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
       llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4);

cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp

SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
 
   SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount);
   return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));
 }
 
 DefinedSVal SValBuilder::getMetadataSymbolVal(const void *symbolTag,
                                               const MemRegion *region,
                                               const Expr *expr, QualType type,
+                                              const LocationContext *LCtx,
                                               unsigned count) {
   assert(SymbolManager::canSymbolicate(type) && "Invalid metadata symbol type");
 
   SymbolRef sym =
-      SymMgr.getMetadataSymbol(region, expr, type, count, symbolTag);
+      SymMgr.getMetadataSymbol(region, expr, type, LCtx, count, symbolTag);
 
   if (Loc::isLocType(type))
     return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
   return nonloc::SymbolVal(sym);
 }
 
 DefinedOrUnknownSVal

cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp

if (!SD) {
     new (SD) SymbolExtent(SymbolCounter, R);
     DataSet.InsertNode(SD, InsertPos);
     ++SymbolCounter;
   }
 
   return cast<SymbolExtent>(SD);
 }
 
-const SymbolMetadata*
+const SymbolMetadata *
 SymbolManager::getMetadataSymbol(const MemRegion* R, const Stmt *S, QualType T,
+                                 const LocationContext *LCtx,
                                  unsigned Count, const void *SymbolTag) {
 
   llvm::FoldingSetNodeID profile;
-  SymbolMetadata::Profile(profile, R, S, T, Count, SymbolTag);
+  SymbolMetadata::Profile(profile, R, S, T, LCtx, Count, SymbolTag);
   void *InsertPos;
   SymExpr *SD = DataSet.FindNodeOrInsertPos(profile, InsertPos);
   if (!SD) {
     SD = (SymExpr*) BPAlloc.Allocate<SymbolMetadata>();
-    new (SD) SymbolMetadata(SymbolCounter, R, S, T, Count, SymbolTag);
+    new (SD) SymbolMetadata(SymbolCounter, R, S, T, LCtx, Count, SymbolTag);
     DataSet.InsertNode(SD, InsertPos);
     ++SymbolCounter;
   }
 
   return cast<SymbolMetadata>(SD);
 }
 
 const SymbolCast*

cfe/trunk/test/Analysis/string.c

 }
 
 void strlen_liveness(const char *x) {
   if (strlen(x) < 5)
     return;
   clang_analyzer_eval(strlen(x) < 5); // expected-warning{{FALSE}}
 }
 
+
+size_t strlenWrapper(const char *str) {
+  return strlen(str);
+}
+
+extern void invalidate(char *s);
+
+void testStrlenCallee() {
+  char str[42];
+  invalidate(str);
+  size_t lenBefore = strlenWrapper(str);
+  invalidate(str);
+  size_t lenAfter = strlenWrapper(str);
+  clang_analyzer_eval(lenBefore == lenAfter); // expected-warning{{UNKNOWN}}
+}
+
+
 //===----------------------------------------------------------------------===
 // strnlen()
 //===----------------------------------------------------------------------===
 
 size_t strnlen(const char *s, size_t maxlen);
 
 void strnlen_constant0() {
   clang_analyzer_eval(strnlen("123", 10) == 3); // expected-warning{{TRUE}}