[PATCH] D103319: [analyzer] Use Optional as a return type of StoreManager::castRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 348524.
ASDenysPetrov added a comment.

Added a simplified test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103319/new/

https://reviews.llvm.org/D103319

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/casts.c


Index: clang/test/Analysis/casts.c
===================================================================
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -251,18 +251,9 @@
     ;
 }
 
-// See PR50179.
-// Just don't crash.
-typedef struct taskS {
-  void *pJob;
-} taskS;
-
-typedef struct workS {
-  taskS *pTaskList;
-} workS;
-
-void *getTaskJob(unsigned jobId, workS *pWork, unsigned taskId) {
-  const taskS *pTask = pWork->pTaskList + taskId;
-  taskS task = *pTask;
-  return task.pJob;
+// PR50179.
+struct S {};
+void symbolic_offset(S *ptr, int i) {
+  const S *pS = ptr + i;
+  S s = *pS; // no-crash
 }
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -71,7 +71,8 @@
   return MRMgr.getElementRegion(T, idx, R, Ctx);
 }
 
-const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) {
+Optional<const MemRegion *> StoreManager::castRegion(const MemRegion *R,
+                                                     QualType CastToTy) {
   ASTContext &Ctx = StateMgr.getContext();
 
   // Handle casts to Objective-C objects.
@@ -88,7 +89,7 @@
 
     // We don't know what to make of it.  Return a NULL region, which
     // will be interpreted as UnknownVal.
-    return nullptr;
+    return None;
   }
 
   // Now assume we are casting from pointer to pointer. Other cases should
@@ -168,7 +169,7 @@
       // If we cannot compute a raw offset, throw up our hands and return
       // a NULL MemRegion*.
       if (!baseR)
-        return nullptr;
+        return None;
 
       CharUnits off = rawOff.getOffset();
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -753,16 +753,16 @@
         if (const auto *SR = dyn_cast<SymbolicRegion>(R)) {
           QualType SRTy = SR->getSymbol()->getType();
           if (!hasSameUnqualifiedPointeeType(SRTy, CastTy)) {
-            R = StateMgr.getStoreManager().castRegion(SR, CastTy);
-            return loc::MemRegionVal(R);
+            if (auto OptR = StateMgr.getStoreManager().castRegion(SR, CastTy))
+              return loc::MemRegionVal(*OptR);
           }
         }
       }
       // Next fixes pointer dereference using type different from its initial
       // one. See PR37503 and PR49007 for details.
       if (const auto *ER = dyn_cast<ElementRegion>(R)) {
-        if ((R = StateMgr.getStoreManager().castRegion(ER, CastTy)))
-          return loc::MemRegionVal(R);
+        if (auto OptR = StateMgr.getStoreManager().castRegion(ER, CastTy))
+          return loc::MemRegionVal(*OptR);
       }
 
       return V;
@@ -807,8 +807,8 @@
 
     // Get the result of casting a region to a different type.
     const MemRegion *R = V.getRegion();
-    if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-      return loc::MemRegionVal(R);
+    if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+      return loc::MemRegionVal(*OptR);
   }
 
   // Pointer to whatever else.
@@ -873,8 +873,8 @@
   if (!IsUnknownOriginalType && Loc::isLocType(CastTy) &&
       OriginalTy->isIntegralOrEnumerationType()) {
     if (const MemRegion *R = L.getAsRegion())
-      if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-        return loc::MemRegionVal(R);
+      if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+        return loc::MemRegionVal(*OptR);
     return L;
   }
 
@@ -890,8 +890,8 @@
       // Delegate to store manager to get the result of casting a region to a
       // different type. If the MemRegion* returned is NULL, this expression
       // Evaluates to UnknownVal.
-      if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-        return loc::MemRegionVal(R);
+      if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+        return loc::MemRegionVal(*OptR);
     }
   } else {
     if (Loc::isLocType(CastTy)) {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -181,7 +181,8 @@
   /// castRegion - Used by ExprEngine::VisitCast to handle casts from
   ///  a MemRegion* to a specific location type.  'R' is the region being
   ///  casted and 'CastToTy' the result type of the cast.
-  const MemRegion *castRegion(const MemRegion *region, QualType CastToTy);
+  Optional<const MemRegion *> castRegion(const MemRegion *region,
+                                         QualType CastToTy);
 
   virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx,
                                       SymbolReaper &SymReaper) = 0;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits