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

llvm-svn: 179916

clang/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;

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

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

clang/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>()) {

clang/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();
   

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

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

clang/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++;
+  }
+}