r334554 - [analyzer] Ensure that loop widening does not invalidate references

[Matthew Voss via cfe-commits]

Author: ormris
Date: Tue Jun 12 15:22:35 2018
New Revision: 334554

URL: http://llvm.org/viewvc/llvm-project?rev=334554&view=rev
Log:
[analyzer] Ensure that loop widening does not invalidate references

Loop widening can invalidate a reference. If the analyzer attempts to visit the
destructor to a non-existent reference, it will crash. This patch ensures that
the reference is preserved.

https://reviews.llvm.org/D47044


Added:
    cfe/trunk/test/Analysis/loop-widening-preserve-reference-type.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp?rev=334554&r1=334553&r2=334554&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp Tue Jun 12 15:22:35 2018
@@ -14,10 +14,16 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
+
+const auto MatchRef = "matchref";
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -49,6 +55,7 @@ ProgramStateRef getWidenedLoopState(Prog
   // TODO Nested loops are currently widened as a result of the invalidation
   //      being so inprecise. When the invalidation is improved, the handling
   //      of nested loops will also need to be improved.
+  ASTContext &ASTCtx = LCtx->getAnalysisDeclContext()->getASTContext();
   const StackFrameContext *STC = LCtx->getCurrentStackFrame();
   MemRegionManager &MRMgr = PrevState->getStateManager().getRegionManager();
   const MemRegion *Regions[] = {MRMgr.getStackLocalsRegion(STC),
@@ -60,6 +67,18 @@ ProgramStateRef getWidenedLoopState(Prog
                      RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
 
+  // References should not be invalidated.
+  auto Matches = match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef)))),
+                       *LCtx->getDecl()->getBody(), ASTCtx);
+  for (BoundNodes Match : Matches) {
+    const VarDecl *VD = Match.getNodeAs<VarDecl>(MatchRef);
+    assert(VD);
+    const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
+    ITraits.setTrait(VarMem,
+                     RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+
+
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
   // pointer shoule remain unchanged.

Added: cfe/trunk/test/Analysis/loop-widening-preserve-reference-type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-widening-preserve-reference-type.cpp?rev=334554&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/loop-widening-preserve-reference-type.cpp (added)
+++ cfe/trunk/test/Analysis/loop-widening-preserve-reference-type.cpp Tue Jun 12 15:22:35 2018
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A &x = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+}                               // expected-warning at -1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}