[analyzer] Ensure BugReporterTracking works on regions with pointer arithmetic

Introduce a new helper function, which computes the first symbolic region in
the base region chain. The corresponding symbol has been used for assuming that
a pointer is null. Now, it will also be used for checking if it is null.

This ensures that we are tracking a null pointer correctly in the BugReporter.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179916 91177308-0d34-0410-b5e6-96231b3b80d8

8 files changed, 67 insertions, 27 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index e4d67cab0d..9b4f77dd67 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -37,11 +37,12 @@ class StackFrameContext;
 
 namespace ento {
 
+class CodeTextRegion;
 class MemRegionManager;
 class MemSpaceRegion;
 class SValBuilder;
+class SymbolicRegion;
 class VarRegion;
-class CodeTextRegion;
 
 /// Represent a region's offset within the top level base region.
 class RegionOffset {
@@ -145,6 +146,10 @@ public:
 
   const MemRegion *StripCasts(bool StripBaseCasts = true) const;
 
+  /// \brief If this is a symbolic region, returns the region. Otherwise,
+  /// goes up the base chain looking for the first symbolic base region.
+  const SymbolicRegion *getSymbolicBase() const;
+
   bool hasGlobalsOrParametersStorage() const;
 
   bool hasStackStorage() const;
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 1c5519e9e7..326e784e83 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -144,16 +144,24 @@ public:
   /// Otherwise return 0.
   const FunctionDecl *getAsFunctionDecl() const;
 
-  /// If this SVal is a location (subclasses Loc) and
-  /// wraps a symbol, return that SymbolRef.  Otherwise return 0.
-  SymbolRef getAsLocSymbol() const;
+  /// \brief If this SVal is a location and wraps a symbol, return that
+  ///  SymbolRef. Otherwise return 0.
+  ///
+  /// Casts are ignored during lookup.
+  /// \param IncludeBaseRegions The boolean that controls whether the search
+  /// should continue to the base regions if the region is not symbolic.
+  SymbolRef getAsLocSymbol(bool IncludeBaseRegions = false) const;
 
   /// Get the symbol in the SVal or its base region.
   SymbolRef getLocSymbolInBase() const;
 
-  /// If this SVal wraps a symbol return that SymbolRef.
+  /// \brief If this SVal wraps a symbol return that SymbolRef.
   /// Otherwise, return 0.
-  SymbolRef getAsSymbol() const;
+  ///
+  /// Casts are ignored during lookup.
+  /// \param IncludeBaseRegions The boolean that controls whether the search
+  /// should continue to the base regions if the region is not symbolic.
+  SymbolRef getAsSymbol(bool IncludeBaseRegions = false) const;
 
   /// getAsSymbolicExpression - If this Sval wraps a symbolic expression then
   ///  return that expression.  Otherwise return NULL.
diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 8f78757ee7..7224c667c9 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -964,7 +964,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
       report.addVisitor(new UndefOrNullArgVisitor(R));
 
       // If the contents are symbolic, find out when they became null.
-      if (V.getAsLocSymbol()) {
+      if (V.getAsLocSymbol(/*IncludeBaseRegions*/ true)) {
         BugReporterVisitor *ConstraintTracker =
           new TrackConstraintBRVisitor(V.castAs<DefinedSVal>(), false);
         report.addVisitor(ConstraintTracker);
diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index cad42044b9..42073d4841 100644
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1080,6 +1080,17 @@ const MemRegion *MemRegion::StripCasts(bool StripBaseCasts) const {
   }
 }
 
+const SymbolicRegion *MemRegion::getSymbolicBase() const {
+  const SubRegion *SubR = dyn_cast<SubRegion>(this);
+
+  while (SubR) {
+    if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SubR))
+      return SymR;
+    SubR = dyn_cast<SubRegion>(SubR->getSuperRegion());
+  }
+  return 0;
+}
+
 // FIXME: Merge with the implementation of the same method in Store.cpp
 static bool IsCompleteType(ASTContext &Ctx, QualType Ty) {
   if (const RecordType *RT = Ty->getAs<RecordType>()) {
diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index 4b857b0ac2..5c54e2f112 100644
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -383,7 +383,7 @@ ConditionTruthVal ProgramState::isNull(SVal V) const {
   if (V.isConstant())
     return false;
   
-  SymbolRef Sym = V.getAsSymbol();
+  SymbolRef Sym = V.getAsSymbol(/* IncludeBaseRegion */ true);
   if (!Sym)
     return ConditionTruthVal();
   
diff --git a/lib/StaticAnalyzer/Core/SVals.cpp b/lib/StaticAnalyzer/Core/SVals.cpp
index 38e216f28c..650691535f 100644
--- a/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/lib/StaticAnalyzer/Core/SVals.cpp
@@ -64,14 +64,18 @@ const FunctionDecl *SVal::getAsFunctionDecl() const {
 ///
 /// Implicit casts (ex: void* -> char*) can turn Symbolic region into Element
 /// region. If that is the case, gets the underlining region.
-SymbolRef SVal::getAsLocSymbol() const {
+/// When IncludeBaseRegions is set to true and the SubRegion is non-symbolic,
+/// the first symbolic parent region is returned.
+SymbolRef SVal::getAsLocSymbol(bool IncludeBaseRegions) const {
   // FIXME: should we consider SymbolRef wrapped in CodeTextRegion?
   if (Optional<nonloc::LocAsInteger> X = getAs<nonloc::LocAsInteger>())
     return X->getLoc().getAsLocSymbol();
 
   if (Optional<loc::MemRegionVal> X = getAs<loc::MemRegionVal>()) {
-    const MemRegion *R = X->stripCasts();
-    if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(R))
+    const MemRegion *R = X->getRegion();
+    if (const SymbolicRegion *SymR = IncludeBaseRegions ?
+                                      R->getSymbolicBase() :
+                                      dyn_cast<SymbolicRegion>(R->StripCasts()))
       return SymR->getSymbol();
   }
   return 0;
@@ -99,13 +103,17 @@ SymbolRef SVal::getLocSymbolInBase() const {
 // TODO: The next 3 functions have to be simplified.
 
 /// \brief If this SVal wraps a symbol return that SymbolRef.
-///  Otherwise return 0.
-SymbolRef SVal::getAsSymbol() const {
+/// Otherwise, return 0.
+///
+/// Casts are ignored during lookup.
+/// \param IncludeBaseRegions The boolean that controls whether the search
+/// should continue to the base regions if the region is not symbolic.
+SymbolRef SVal::getAsSymbol(bool IncludeBaseRegion) const {
   // FIXME: should we consider SymbolRef wrapped in CodeTextRegion?
   if (Optional<nonloc::SymbolVal> X = getAs<nonloc::SymbolVal>())
     return X->getSymbol();
 
-  return getAsLocSymbol();
+  return getAsLocSymbol(IncludeBaseRegion);
 }
 
 /// getAsSymbolicExpression - If this Sval wraps a symbolic expression then
diff --git a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index 34de3453ca..a06268dd33 100644
--- a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -90,20 +90,15 @@ ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef state,
 
   case loc::MemRegionKind: {
     // FIXME: Should this go into the storemanager?
-
     const MemRegion *R = Cond.castAs<loc::MemRegionVal>().getRegion();
-    const SubRegion *SubR = dyn_cast<SubRegion>(R);
-
-    while (SubR) {
-      // FIXME: now we only find the first symbolic region.
-      if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SubR)) {
-        const llvm::APSInt &zero = getBasicVals().getZeroWithPtrWidth();
-        if (Assumption)
-          return assumeSymNE(state, SymR->getSymbol(), zero, zero);
-        else
-          return assumeSymEQ(state, SymR->getSymbol(), zero, zero);
-      }
-      SubR = dyn_cast<SubRegion>(SubR->getSuperRegion());
+
+    // FIXME: now we only find the first symbolic region.
+    if (const SymbolicRegion *SymR = R->getSymbolicBase()) {
+      const llvm::APSInt &zero = getBasicVals().getZeroWithPtrWidth();
+      if (Assumption)
+        return assumeSymNE(state, SymR->getSymbol(), zero, zero);
+      else
+        return assumeSymEQ(state, SymR->getSymbol(), zero, zero);
     }
 
     // FALL-THROUGH.
diff --git a/test/Analysis/inlining/inline-defensive-checks.c b/test/Analysis/inlining/inline-defensive-checks.c
index df3a8f2281..aa7f70030c 100644
--- a/test/Analysis/inlining/inline-defensive-checks.c
+++ b/test/Analysis/inlining/inline-defensive-checks.c
@@ -97,3 +97,16 @@ void test24(char *buffer) {
   use(buffer);
   buffer[1] = 'b';
 }
+
+// Ensure idc works on pointers with constant offset.
+void idcchar(const char *s2) {
+  if(s2)
+    ;
+}
+void testConstantOffset(char *value) {
+  char *cursor = value + 5;
+  idcchar(cursor);
+  if (*cursor) {
+    cursor++;
+  }
+}