[clang] fae3534 - [analyzer] Use Optional as a return type of StoreManager::castRegion

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-05-29T15:16:56+03:00
New Revision: fae3534b3056bb96d26a6d1b6e7d6a2ccaf4fab1

URL: https://github.com/llvm/llvm-project/commit/fae3534b3056bb96d26a6d1b6e7d6a2ccaf4fab1
DIFF: https://github.com/llvm/llvm-project/commit/fae3534b3056bb96d26a6d1b6e7d6a2ccaf4fab1.diff

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

Summary: Make StoreManager::castRegion function usage safier. Replace `const MemRegion *` with `Optional<const MemRegion *>`. Simplified one of related test cases due to suggestions in D101635.

Differential Revision: https://reviews.llvm.org/D103319

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index 947913ae4eee9..d2461705d1282 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -181,7 +181,8 @@ class StoreManager {
   /// 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;

diff  --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 0003c27513994..39787886cd7a5 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -753,16 +753,16 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
         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 
diff erent 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 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
 
     // Get the result of casting a region to a 
diff erent 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 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
   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 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
       // Delegate to store manager to get the result of casting a region to a
       // 
diff erent 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)) {

diff  --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index c563b44efc13e..b867b0746f90f 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -71,7 +71,8 @@ const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R,
   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 @@ const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy)
 
     // 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 @@ const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy)
       // 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();
 

diff  --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c
index 1de7ef54b57fb..6b9108ac6bb03 100644
--- a/clang/test/Analysis/casts.c
+++ b/clang/test/Analysis/casts.c
@@ -251,18 +251,9 @@ void no_crash_reinterpret_char_as_uchar(char ***a, int *b) {
     ;
 }
 
-// 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(struct S *ptr, int i) {
+  const struct S *pS = ptr + i;
+  struct S s = *pS; // no-crash
 }