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

Updated. Unlinked parent revision D95799 <https://reviews.llvm.org/D95799> from 
the stack. Made corresponding changes in this patch.


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

https://reviews.llvm.org/D96090

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -394,48 +394,6 @@
   return UnknownVal();
 }
 
-static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
-  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
-         ty2->getPointeeType().getCanonicalType().getTypePtr();
-}
-
-/// CastRetrievedVal - Used by subclasses of StoreManager to implement
-///  implicit casts that arise from loads from regions that are reinterpreted
-///  as another region.
-SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-                                    QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
-    return V;
-
-  // The dispatchCast() call below would convert the int into a float.
-  // What we want, however, is a bit-by-bit reinterpretation of the int
-  // as a float, which usually yields nothing garbage. For now skip casts
-  // from ints to floats.
-  // TODO: What other combinations of types are affected?
-  if (castTy->isFloatingType()) {
-    SymbolRef Sym = V.getAsSymbol();
-    if (Sym && !Sym->getType()->isFloatingType())
-      return UnknownVal();
-  }
-
-  // When retrieving symbolic pointer and expecting a non-void pointer,
-  // wrap them into element regions of the expected type if necessary.
-  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
-  // make sure that the retrieved value makes sense, because there's no other
-  // cast in the AST that would tell us to cast it to the correct pointer type.
-  // We might need to do that for non-void pointers as well.
-  // FIXME: We really need a single good function to perform casts for us
-  // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
-    if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion())) {
-      QualType sr = SR->getSymbol()->getType();
-      if (!hasSameUnqualifiedPointeeType(sr, castTy))
-          return loc::MemRegionVal(castRegion(SR, castTy));
-    }
-
-  return svalBuilder.dispatchCast(V, castTy);
-}
-
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   if (Base.isUnknownOrUndef())
     return Base;
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -66,9 +66,7 @@
 //===----------------------------------------------------------------------===//
 
 SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
-  assert(Val.getAs<Loc>() || Val.getAs<NonLoc>());
-  return Val.getAs<Loc>() ? evalCastFromLoc(Val.castAs<Loc>(), CastTy)
-                           : evalCastFromNonLoc(Val.castAs<NonLoc>(), CastTy);
+  return evalCast(Val, CastTy);
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -536,20 +536,29 @@
 // `evalCastKind` and `evalCastSubKind` are helpers
 //===----------------------------------------------------------------------===//
 
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  CastTy = Context.getCanonicalType(CastTy);
-  OriginalTy = Context.getCanonicalType(OriginalTy);
-  if (CastTy == OriginalTy)
+// In case when `OriginalTy.isNull() == true` we cast `V` a bit differently.
+SVal SValBuilder::evalCast(SVal V, QualType CastTy,
+                           QualType OriginalTy /*= QualType{}*/) {
+  if (CastTy.isNull())
     return V;
 
-  // FIXME: Move this check to the most appropriate evalCastKind/evalCastSubKind
-  // function.
-  // For const casts, casts to void, just propagate the value.
-  if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
-    if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
-                                Context.getPointerType(OriginalTy)))
+  CastTy = Context.getCanonicalType(CastTy);
+
+  if (!OriginalTy.isNull()) {
+    OriginalTy = Context.getCanonicalType(OriginalTy);
+
+    if (CastTy == OriginalTy)
       return V;
 
+    // FIXME: Move this check to the most appropriate
+    // evalCastKind/evalCastSubKind function. For const casts, casts to void,
+    // just propagate the value.
+    if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
+      if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
+                                  Context.getPointerType(OriginalTy)))
+        return V;
+  }
+
   // Cast SVal according to kinds.
   switch (V.getBaseKind()) {
   case SVal::UndefinedValKind:
@@ -644,10 +653,12 @@
     return makeLocAsInteger(V, BitWidth);
   }
 
-  // Array to pointer.
-  if (isa<ArrayType>(OriginalTy))
-    if (CastTy->isPointerType() || CastTy->isReferenceType())
-      return UnknownVal();
+  if (!OriginalTy.isNull()) {
+    // Array to pointer.
+    if (isa<ArrayType>(OriginalTy))
+      if (CastTy->isPointerType() || CastTy->isReferenceType())
+        return UnknownVal();
+  }
 
   // Pointer to any pointer.
   if (Loc::isLocType(CastTy))
@@ -656,6 +667,10 @@
   // Pointer to whatever else.
   return UnknownVal();
 }
+static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
+  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
+         ty2->getPointeeType().getCanonicalType().getTypePtr();
+}
 
 SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
                                   QualType OriginalTy) {
@@ -678,7 +693,9 @@
   }
 
   // Try to cast to array
-  const auto *ArrayTy = dyn_cast<ArrayType>(OriginalTy.getCanonicalType());
+  const auto *ArrayTy =
+      OriginalTy.isNull() ? nullptr
+                          : dyn_cast<ArrayType>(OriginalTy.getCanonicalType());
 
   // Pointer to integer.
   if (CastTy->isIntegralOrEnumerationType()) {
@@ -699,6 +716,29 @@
 
   // Pointer to pointer.
   if (Loc::isLocType(CastTy)) {
+
+    if (OriginalTy.isNull()) {
+      // When retrieving symbolic pointer and expecting a non-void pointer,
+      // wrap them into element regions of the expected type if necessary.
+      // It is necessary to make sure that the retrieved value makes sense,
+      // because there's no other cast in the AST that would tell us to cast
+      // it to the correct pointer type. We might need to do that for non-void
+      // pointers as well.
+      // FIXME: We really need a single good function to perform casts for us
+      // correctly every time we need it.
+      if (CastTy->isPointerType() && !CastTy->isVoidPointerType()) {
+        const MemRegion *R = V.getRegion();
+        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);
+          }
+        }
+      }
+      return V;
+    }
+
     if (OriginalTy->isIntegralOrEnumerationType() ||
         OriginalTy->isBlockPointerType() || OriginalTy->isFunctionPointerType())
       return V;
@@ -799,7 +839,8 @@
     // Pass to Loc function.
     return evalCastKind(L, CastTy, OriginalTy);
 
-  if (Loc::isLocType(CastTy) && OriginalTy->isIntegralOrEnumerationType()) {
+  if (Loc::isLocType(CastTy) && !OriginalTy.isNull() &&
+      OriginalTy->isIntegralOrEnumerationType()) {
     if (const MemRegion *R = L.getAsRegion())
       if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
         return loc::MemRegionVal(R);
@@ -807,7 +848,8 @@
   }
 
   // Pointer as integer with region to integer/pointer.
-  if (const MemRegion *R = L.getAsRegion()) {
+  const MemRegion *R = L.getAsRegion();
+  if (!OriginalTy.isNull() && R) {
     if (CastTy->isIntegralOrEnumerationType())
       // Pass to MemRegion function.
       return evalCastSubKind(loc::MemRegionVal(R), CastTy, OriginalTy);
@@ -822,15 +864,29 @@
         return loc::MemRegionVal(R);
     }
   } else {
-    if (Loc::isLocType(CastTy))
+    if (Loc::isLocType(CastTy)) {
+      if (OriginalTy.isNull())
+        // Pass to MemRegion function.
+        return evalCastSubKind(loc::MemRegionVal(R), CastTy, OriginalTy);
       return L;
+    }
 
-    // FIXME: Correctly support promotions/truncations.
-    const unsigned CastSize = Context.getIntWidth(CastTy);
-    if (CastSize == V.getNumBits())
-      return V;
+    SymbolRef SE = nullptr;
+    if (R) {
+      if (const SymbolicRegion *SR =
+              dyn_cast<SymbolicRegion>(R->StripCasts())) {
+        SE = SR->getSymbol();
+      }
+    }
+
+    if (!CastTy->isFloatingType() || !SE || SE->getType()->isFloatingType()) {
+      // FIXME: Correctly support promotions/truncations.
+      const unsigned CastSize = Context.getIntWidth(CastTy);
+      if (CastSize == V.getNumBits())
+        return V;
 
-    return makeLocAsInteger(L, CastSize);
+      return makeLocAsInteger(L, CastSize);
+    }
   }
 
   // Pointer as integer to whatever else.
@@ -842,12 +898,11 @@
   SymbolRef SE = V.getSymbol();
 
   // Symbol to bool.
-  if (CastTy->isBooleanType()) {
+  if (!OriginalTy.isNull() && CastTy->isBooleanType()) {
     // Non-float to bool.
     if (Loc::isLocType(OriginalTy) ||
         OriginalTy->isIntegralOrEnumerationType() ||
         OriginalTy->isMemberPointerType()) {
-      SymbolRef SE = V.getSymbol();
       BasicValueFactory &BVF = getBasicValueFactory();
       return makeNonLoc(SE, BO_NE, BVF.getValue(0, SE->getType()), CastTy);
     }
@@ -864,7 +919,9 @@
     if (haveSameType(T, CastTy))
       return V;
     if (!Loc::isLocType(CastTy))
-      return makeNonLoc(SE, T, CastTy);
+      if (!OriginalTy.isNull() || !CastTy->isFloatingType() ||
+          T->isFloatingType())
+        return makeNonLoc(SE, T, CastTy);
   }
 
   // Symbol to pointer and whatever else.
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1479,7 +1479,7 @@
     return UnknownVal();
 
   if (const FieldRegion* FR = dyn_cast<FieldRegion>(R))
-    return CastRetrievedVal(getBindingForField(B, FR), FR, T);
+    return svalBuilder.evalCast(getBindingForField(B, FR), T);
 
   if (const ElementRegion* ER = dyn_cast<ElementRegion>(R)) {
     // FIXME: Here we actually perform an implicit conversion from the loaded
@@ -1487,7 +1487,7 @@
     // more intelligently.  For example, an 'element' can encompass multiple
     // bound regions (e.g., several bound bytes), or could be a subset of
     // a larger value.
-    return CastRetrievedVal(getBindingForElement(B, ER), ER, T);
+    return svalBuilder.evalCast(getBindingForElement(B, ER), T);
   }
 
   if (const ObjCIvarRegion *IVR = dyn_cast<ObjCIvarRegion>(R)) {
@@ -1497,7 +1497,7 @@
     // reinterpretted, it is possible we stored a different value that could
     // fit within the ivar.  Either we need to cast these when storing them
     // or reinterpret them lazily (as we do here).
-    return CastRetrievedVal(getBindingForObjCIvar(B, IVR), IVR, T);
+    return svalBuilder.evalCast(getBindingForObjCIvar(B, IVR), T);
   }
 
   if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
@@ -1507,7 +1507,7 @@
     // variable is reinterpretted, it is possible we stored a different value
     // that could fit within the variable.  Either we need to cast these when
     // storing them or reinterpret them lazily (as we do here).
-    return CastRetrievedVal(getBindingForVar(B, VR), VR, T);
+    return svalBuilder.evalCast(getBindingForVar(B, VR), T);
   }
 
   const SVal *V = B.lookup(R, BindingKey::Direct);
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
@@ -280,12 +280,6 @@
                                          QualType pointeeTy,
                                          uint64_t index = 0);
 
-  /// CastRetrievedVal - Used by subclasses of StoreManager to implement
-  ///  implicit casts that arise from loads from regions that are reinterpreted
-  ///  as another region.
-  SVal CastRetrievedVal(SVal val, const TypedValueRegion *region,
-                        QualType castTy);
-
 private:
   SVal getLValueFieldOrIvar(const Decl *decl, SVal base);
 };
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -124,7 +124,7 @@
              Ty2->isIntegralOrEnumerationType()));
   }
 
-  SVal evalCast(SVal V, QualType CastTy, QualType OriginalTy);
+  SVal evalCast(SVal V, QualType CastTy, QualType OriginalTy = QualType{});
 
   // Handles casts of type CK_IntegralCast.
   SVal evalIntegralCast(ProgramStateRef state, SVal val, QualType castTy,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to