[analyzer] Replace StoreManager::evalIntegralCast with SValBuilder::evalCast

[ASDenysPetrov]

Summary

Simplify and move functionality from evalIntegralCast to evalCast. Replace evalIntegralCast with evalCast.

This patch shall not change any behavior.

[ASDenysPetrov]

A bit improved.

[steakhal]

Please make sure that you specify the parent revision or the patch can be applied on top of the tree.
Your lines clearly not match the top of the tree state.
https://github.com/llvm/llvm-project/blob/13f4448ae7db1a477ec2d48776e46415a3401314/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h#L129-L131

I really want to help you by finding regressions and stuff. But these mistakes hinder me a lot in applying your patches.

[ASDenysPetrov]

@steakhal

Please make sure that you specify the parent revision or the patch can be applied on top of the tree.

Sorry, probably missed to add a parent revision. Done.

[NoQ]

That's definitely an improvement for our API surface. I think this is good but like @steakhal said I recommend running on a large codebase looking for potential regressions because this code is (still) very much spaghetti and hard to reason about.

[ASDenysPetrov]

Rebased on main.

[steakhal]

All reports and crashes are preserved at this point of the patch stack - with or without z3 crosscheck on multiple projects - even on llvm.

[steakhal]

If @NoQ doesn't have any objections I'm ok with this change.

[NoQ]

Ok then, let's try to land it! 🤞

[ASDenysPetrov]

In D97388#2615104, @NoQ wrote:

Ok then, let's try to land it! 🤞

Thanks for the approval. Before land this we should land the preparatory revision D97296. Please, take a look. It should be safe as well.

[ASDenysPetrov]

This comment was removed by ASDenysPetrov.

[ASDenysPetrov]

Mistakenly erased with another patch. Restored. But anyway this revision should be abandoned as irrelevant any more. D103096 is a new revision which makes this patch deprecated.

[ASDenysPetrov]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

   SVal evalCast(ProgramStateRef State, SVal V, QualType CastTy,
                 QualType OriginalTy = QualType{});
 
-  // Handles casts of type CK_IntegralCast.
-  SVal evalIntegralCast(ProgramStateRef state, SVal val, QualType castTy,
-                        QualType originalType);
-
   virtual SVal evalMinus(NonLoc val) = 0;
 
   virtual SVal evalComplement(NonLoc val) = 0;

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

       case CK_IntegralCast: {
         // Delegate to SValBuilder to process.
         SVal V = state->getSVal(Ex, LCtx);
-        V = svalBuilder.evalIntegralCast(state, V, T, ExTy);
+        V = svalBuilder.evalCast(state, V, T, ExTy);
         state = state->BindExpr(CastE, LCtx, V);
         Bldr.generateNode(CastE, Pred, state);
         continue;

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

   return true;
 }
 
-// Handles casts of type CK_IntegralCast.
-// At the moment, this function will redirect to evalCast, except when the range
-// of the original value is known to be greater than the max of the target type.
-SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
-                                   QualType castTy, QualType originalTy) {
-  // No truncations if target type is big enough.
-  if (getContext().getTypeSize(castTy) >= getContext().getTypeSize(originalTy))
-    return evalCast(state, val, castTy, originalTy);
-
-  SymbolRef se = val.getAsSymbol();
-  if (!se) // Let evalCast handle non symbolic expressions.
-    return evalCast(state, val, castTy, originalTy);
-
-  // Find the maximum value of the target type.
-  APSIntType ToType(getContext().getTypeSize(castTy),
-                    castTy->isUnsignedIntegerType());
-  llvm::APSInt ToTypeMax = ToType.getMaxValue();
-  NonLoc ToTypeMaxVal =
-      makeIntVal(ToTypeMax.isUnsigned() ? ToTypeMax.getZExtValue()
-                                        : ToTypeMax.getSExtValue(),
-                 castTy)
-          .castAs<NonLoc>();
-  // Check the range of the symbol being casted against the maximum value of the
-  // target type.
-  NonLoc FromVal = val.castAs<NonLoc>();
-  QualType CmpTy = getConditionType();
-  NonLoc CompVal =
-      evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>();
-  ProgramStateRef IsNotTruncated, IsTruncated;
-  std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
-  if (!IsNotTruncated && IsTruncated) {
-    // Symbol is truncated so we evaluate it as a cast.
-    NonLoc CastVal = makeNonLoc(se, originalTy, castTy);
-    return CastVal;
-  }
-  return evalCast(state, val, castTy, originalTy);
-}
-
 //===----------------------------------------------------------------------===//
 // Cast methods.
 // `evalCast` is the main method

       return makeNonLoc(SE, BO_NE, BVF.getValue(0, SE->getType()), CastTy);
     }
   } else {
+    // Symbol to integer.
+    if (!OriginalTy.isNull() && CastTy->isIntegralOrEnumerationType()) {
+      // Symbolic integer to integer.
+      if (OriginalTy->isIntegralOrEnumerationType()) {
+        // Truncation.
+        if (getContext().getTypeSize(CastTy) <
+            getContext().getTypeSize(OriginalTy)) {
+          // Find the maximum value of the target type.
+          llvm::APSInt ToTypeMax =
+              llvm::APSInt::getMaxValue(getContext().getTypeSize(CastTy),
+                                        CastTy->isUnsignedIntegerType());
+          NonLoc ToTypeMaxVal =
+              makeIntVal(ToTypeMax.getExtValue(), CastTy).castAs<NonLoc>();
+          // Check the range of the symbol being casted against the maximum
+          // value of the target type.
+          NonLoc FromVal = V.castAs<NonLoc>();
+          QualType CmpTy = getConditionType();
+          NonLoc CompVal =
+              evalBinOpNN(State, BO_LE, FromVal, ToTypeMaxVal, CmpTy)
+                  .castAs<NonLoc>();
+          ProgramStateRef IsNotTruncated, IsTruncated;
+          std::tie(IsNotTruncated, IsTruncated) = State->assume(CompVal);
+          if (!IsNotTruncated && IsTruncated) {
+            // Symbol is truncated so we evaluate it as a cast.
+            NonLoc CastVal = makeNonLoc(SE, OriginalTy, CastTy);
+            return CastVal;
+          }
+        }
+      }
+    }
+
     // Symbol to integer, float.
     QualType T = Context.getCanonicalType(SE->getType());
     // If types are the same or both are integers, ignore the cast.