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

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

   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);
   

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

   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; }
 

   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.

   /// 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,

lib/StaticAnalyzer/Checkers/CStringChecker.cpp

   QualType sizeTy = svalBuilder.getContext().getSizeType();
   SVal strLength = svalBuilder.getMetadataSymbolVal(CStringChecker::getTag(),
                                                     MR, Ex, sizeTy,
+                                                    C.getLocationContext(),
                                                     C.blockCount());
 
   if (!hypothetical) {

lib/StaticAnalyzer/Core/SValBuilder.cpp

 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));

lib/StaticAnalyzer/Core/SymbolManager.cpp

   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;
   }

test/Analysis/string.c

   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()
 //===----------------------------------------------------------------------===