[clang] da8bd97 - [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional<SVal>

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-12-17T13:03:47+02:00
New Revision: da8bd972a33ad642d6237b5b95b31a2a20f46a35

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

LOG: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional<SVal>

Summary: Refactor return value of `StoreManager::attemptDownCast` function by removing the last parameter `bool &Failed` and replace the return value `SVal` with `Optional<SVal>`.  Make the function consistent with the family of `evalDerivedToBase` by renaming it to `evalBaseToDerived`. Aligned the code on the call side with these changes.

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
    clang/lib/StaticAnalyzer/Core/Store.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index d2461705d128..bdf9662d5d99 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -172,9 +172,9 @@ class StoreManager {
   ///    dynamic_cast.
   ///  - We don't know (base is a symbolic region and we don't have
   ///    enough info to determine if the cast will succeed at run time).
-  /// The function returns an SVal representing the derived class; it's
-  /// valid only if Failed flag is set to false.
-  SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+  /// The function returns an optional with SVal representing the derived class
+  /// in case of a successful cast and `None` otherwise.
+  Optional<SVal> evalBaseToDerived(SVal Base, QualType DerivedPtrType);
 
   const ElementRegion *GetElementZeroRegion(const SubRegion *R, QualType T);
 

diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 764dad3e7ab4..ae46709340d3 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -762,9 +762,9 @@ void CXXInstanceCall::getInitialStackFrameContents(
       QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
 
       // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
-      bool Failed;
-      ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
-      if (Failed) {
+      Optional<SVal> V =
+          StateMgr.getStoreManager().evalBaseToDerived(ThisVal, Ty);
+      if (!V.hasValue()) {
         // We might have suffered some sort of placement new earlier, so
         // we're constructing in a completely unexpected storage.
         // Fall back to a generic pointer cast for this-value.
@@ -772,7 +772,8 @@ void CXXInstanceCall::getInitialStackFrameContents(
         const CXXRecordDecl *StaticClass = StaticMD->getParent();
         QualType StaticTy = Ctx.getPointerType(Ctx.getRecordType(StaticClass));
         ThisVal = SVB.evalCast(ThisVal, Ty, StaticTy);
-      }
+      } else
+        ThisVal = *V;
     }
 
     if (!ThisVal.isUnknown())

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 69d67cf9b465..637e4edfd778 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -439,14 +439,15 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
         if (CastE->isGLValue())
           resultType = getContext().getPointerType(resultType);
 
-        bool Failed = false;
-
-        // Check if the value being cast evaluates to 0.
-        if (val.isZeroConstant())
-          Failed = true;
-        // Else, evaluate the cast.
-        else
-          val = getStoreManager().attemptDownCast(val, T, Failed);
+        bool Failed = true;
+
+        // Check if the value being cast does not evaluates to 0.
+        if (!val.isZeroConstant())
+          if (Optional<SVal> V =
+                  StateMgr.getStoreManager().evalBaseToDerived(val, T)) {
+            val = *V;
+            Failed = false;
+          }
 
         if (Failed) {
           if (T->isReferenceType()) {
@@ -478,14 +479,13 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
         if (CastE->isGLValue())
           resultType = getContext().getPointerType(resultType);
 
-        bool Failed = false;
-
         if (!val.isConstant()) {
-          val = getStoreManager().attemptDownCast(val, T, Failed);
+          Optional<SVal> V = getStoreManager().evalBaseToDerived(val, T);
+          val = V ? *V : UnknownVal();
         }
 
         // Failed to cast or the result is unknown, fall back to conservative.
-        if (Failed || val.isUnknown()) {
+        if (val.isUnknown()) {
           val =
             svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
                                          currBldrCtx->blockCount());

diff  --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 05feb1325c93..2bcdb0faf5da 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -314,10 +314,7 @@ static const CXXRecordDecl *getCXXRecordType(const MemRegion *MR) {
   return nullptr;
 }
 
-SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
-                                   bool &Failed) {
-  Failed = false;
-
+Optional<SVal> StoreManager::evalBaseToDerived(SVal Base, QualType TargetType) {
   const MemRegion *MR = Base.getAsRegion();
   if (!MR)
     return UnknownVal();
@@ -392,7 +389,9 @@ SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
   }
 
   // We failed if the region we ended up with has perfect type info.
-  Failed = isa<TypedValueRegion>(MR);
+  if (isa<TypedValueRegion>(MR))
+    return None;
+
   return UnknownVal();
 }