Skip to content

[analyzer][NFC] Migrate nonloc::ConcreteInt to use APSIntPtr (2/4) #120436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

steakhal
Copy link
Contributor

No description provided.

Copy link
Contributor Author

steakhal commented Dec 18, 2024

@steakhal steakhal marked this pull request as ready for review December 18, 2024 15:25
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Dec 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

Changes

Patch is 22.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120436.diff

18 Files Affected:

  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (+1-1)
  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (+6-2)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp (+7-7)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp (+4-4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp (+5-3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/Iterator.cpp (+7-7)
  • (modified) clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp (+2-2)
  • (modified) clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+3-5)
  • (modified) clang/lib/StaticAnalyzer/Core/ProgramState.cpp (+2-4)
  • (modified) clang/lib/StaticAnalyzer/Core/SValBuilder.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/SVals.cpp (+5-5)
  • (modified) clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (+16-14)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 0d9566285f5d4e..f88bf70d72398c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -1206,7 +1206,7 @@ class ElementRegion : public TypedValueRegion {
       : TypedValueRegion(sReg, ElementRegionKind), ElementType(elementType),
         Index(Idx) {
     assert((!isa<nonloc::ConcreteInt>(Idx) ||
-            Idx.castAs<nonloc::ConcreteInt>().getValue().isSigned()) &&
+            Idx.castAs<nonloc::ConcreteInt>().getValue()->isSigned()) &&
            "The index must be signed");
     assert(!elementType.isNull() && !elementType->isVoidType() &&
            "Invalid region type!");
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index a054a819a15a85..57d7514280f10f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -17,6 +17,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -298,9 +299,12 @@ class SymbolVal : public NonLoc {
 /// Value representing integer constant.
 class ConcreteInt : public NonLoc {
 public:
-  explicit ConcreteInt(const llvm::APSInt &V) : NonLoc(ConcreteIntKind, &V) {}
+  explicit ConcreteInt(APSIntPtr V) : NonLoc(ConcreteIntKind, V.get()) {}
 
-  const llvm::APSInt &getValue() const { return *castDataAs<llvm::APSInt>(); }
+  APSIntPtr getValue() const {
+    // This is safe because in the ctor we take a safe APSIntPtr.
+    return APSIntPtr::unsafeConstructor(castDataAs<llvm::APSInt>());
+  }
 
   static bool classof(SVal V) { return V.getKind() == ConcreteIntKind; }
 };
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 3f837564cf47c4..6422933c8828a9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -22,6 +22,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
@@ -241,26 +242,25 @@ computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) {
 static std::pair<NonLoc, nonloc::ConcreteInt>
 getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
                      SValBuilder &svalBuilder) {
+  const llvm::APSInt &extentVal = extent.getValue();
   std::optional<nonloc::SymbolVal> SymVal = offset.getAs<nonloc::SymbolVal>();
   if (SymVal && SymVal->isExpression()) {
     if (const SymIntExpr *SIE = dyn_cast<SymIntExpr>(SymVal->getSymbol())) {
-      llvm::APSInt constant =
-          APSIntType(extent.getValue()).convert(SIE->getRHS());
+      llvm::APSInt constant = APSIntType(extentVal).convert(SIE->getRHS());
       switch (SIE->getOpcode()) {
       case BO_Mul:
         // The constant should never be 0 here, becasue multiplication by zero
         // is simplified by the engine.
-        if ((extent.getValue() % constant) != 0)
+        if ((extentVal % constant) != 0)
           return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
         else
           return getSimplifiedOffsets(
               nonloc::SymbolVal(SIE->getLHS()),
-              svalBuilder.makeIntVal(extent.getValue() / constant),
-              svalBuilder);
+              svalBuilder.makeIntVal(extentVal / constant), svalBuilder);
       case BO_Add:
         return getSimplifiedOffsets(
             nonloc::SymbolVal(SIE->getLHS()),
-            svalBuilder.makeIntVal(extent.getValue() - constant), svalBuilder);
+            svalBuilder.makeIntVal(extentVal - constant), svalBuilder);
       default:
         break;
       }
@@ -363,7 +363,7 @@ static std::string getRegionName(const SubRegion *Region) {
 
 static std::optional<int64_t> getConcreteValue(NonLoc SV) {
   if (auto ConcreteVal = SV.getAs<nonloc::ConcreteInt>()) {
-    return ConcreteVal->getValue().tryExtValue();
+    return ConcreteVal->getValue()->tryExtValue();
   }
   return std::nullopt;
 }
diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 80f128b917b200..cc089767adfeec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -457,7 +457,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE,
   if (!V)
     return;
 
-  uint64_t NumberKind = V->getValue().getLimitedValue();
+  uint64_t NumberKind = V->getValue()->getLimitedValue();
   std::optional<uint64_t> OptCFNumberSize = GetCFNumberSize(Ctx, NumberKind);
 
   // FIXME: In some cases we can emit an error.
diff --git a/clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
index 17f1214195b3ee..ed26ddea93a262 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
@@ -252,11 +252,11 @@ BugReportPtr BitwiseShiftValidator::checkLeftShiftOverflow() {
 
   // We should have already reported a bug if the left operand of the shift was
   // negative, so it cannot be negative here.
-  assert(Left->getValue().isNonNegative());
+  assert(Left->getValue()->isNonNegative());
 
   const unsigned LeftAvailableBitWidth =
       LeftBitWidth - static_cast<unsigned>(ShouldPreserveSignBit);
-  const unsigned UsedBitsInLeftOperand = Left->getValue().getActiveBits();
+  const unsigned UsedBitsInLeftOperand = Left->getValue()->getActiveBits();
   assert(LeftBitWidth >= UsedBitsInLeftOperand);
   const unsigned MaximalAllowedShift =
       LeftAvailableBitWidth - UsedBitsInLeftOperand;
@@ -275,9 +275,9 @@ BugReportPtr BitwiseShiftValidator::checkLeftShiftOverflow() {
   if (const auto ConcreteRight = Right.getAs<nonloc::ConcreteInt>()) {
     // Here ConcreteRight must contain a small non-negative integer, because
     // otherwise one of the earlier checks should've reported a bug.
-    const unsigned RHS = ConcreteRight->getValue().getExtValue();
+    const int64_t RHS = ConcreteRight->getValue()->getExtValue();
     assert(RHS > MaximalAllowedShift);
-    const unsigned OverflownBits = RHS - MaximalAllowedShift;
+    const int64_t OverflownBits = RHS - MaximalAllowedShift;
     ShortMsg = formatv(
         "The shift '{0} << {1}' overflows the capacity of '{2}'",
         Left->getValue(), ConcreteRight->getValue(), LHSTy.getAsString());
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 4ab0c4c9ae7b70..cfdd3c9faa360a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -155,12 +155,14 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
   unsigned BitWidth = C.getASTContext().getIntWidth(Res);
   bool IsUnsigned = Res->isUnsignedIntegerType();
 
+  SValBuilder &SVB = C.getSValBuilder();
+  BasicValueFactory &VF = SVB.getBasicValueFactory();
+
   auto MinValType = llvm::APSInt::getMinValue(BitWidth, IsUnsigned);
   auto MaxValType = llvm::APSInt::getMaxValue(BitWidth, IsUnsigned);
-  nonloc::ConcreteInt MinVal{MinValType};
-  nonloc::ConcreteInt MaxVal{MaxValType};
+  nonloc::ConcreteInt MinVal{VF.getValue(MinValType)};
+  nonloc::ConcreteInt MaxVal{VF.getValue(MaxValType)};
 
-  SValBuilder &SVB = C.getSValBuilder();
   ProgramStateRef State = C.getState();
   SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, MaxVal, Res);
   SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, MinVal, Res);
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index 1b89951397cfb1..3a66b0f11eb2eb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -124,7 +124,7 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
             "requires {1} bytes. Current overhead requires the size of {2} "
             "bytes",
             SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue(),
-            SizeOfPlaceCI->getValue() - SizeOfTargetCI->getValue()));
+            *SizeOfPlaceCI->getValue().get() - SizeOfTargetCI->getValue()));
       else if (IsArrayTypeAllocated &&
                SizeOfPlaceCI->getValue() == SizeOfTargetCI->getValue())
         Msg = std::string(llvm::formatv(
diff --git a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
index e8d35aac2efd9e..ba561ddebdb697 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
@@ -241,7 +241,7 @@ ProgramStateRef advancePosition(ProgramStateRef State, SVal Iter,
   // For concrete integers we can calculate the new position
   nonloc::ConcreteInt IntDist = *IntDistOp;
 
-  if (IntDist.getValue().isNegative()) {
+  if (IntDist.getValue()->isNegative()) {
     IntDist = nonloc::ConcreteInt(BVF.getValue(-IntDist.getValue()));
     BinOp = (BinOp == BO_Add) ? BO_Sub : BO_Add;
   }
@@ -272,9 +272,9 @@ ProgramStateRef assumeNoOverflow(ProgramStateRef State, SymbolRef Sym,
   ProgramStateRef NewState = State;
 
   llvm::APSInt Max = AT.getMaxValue() / AT.getValue(Scale);
-  SVal IsCappedFromAbove =
-      SVB.evalBinOpNN(State, BO_LE, nonloc::SymbolVal(Sym),
-                      nonloc::ConcreteInt(Max), SVB.getConditionType());
+  SVal IsCappedFromAbove = SVB.evalBinOpNN(
+      State, BO_LE, nonloc::SymbolVal(Sym),
+      nonloc::ConcreteInt(BV.getValue(Max)), SVB.getConditionType());
   if (auto DV = IsCappedFromAbove.getAs<DefinedSVal>()) {
     NewState = NewState->assume(*DV, true);
     if (!NewState)
@@ -282,9 +282,9 @@ ProgramStateRef assumeNoOverflow(ProgramStateRef State, SymbolRef Sym,
   }
 
   llvm::APSInt Min = -Max;
-  SVal IsCappedFromBelow =
-      SVB.evalBinOpNN(State, BO_GE, nonloc::SymbolVal(Sym),
-                      nonloc::ConcreteInt(Min), SVB.getConditionType());
+  SVal IsCappedFromBelow = SVB.evalBinOpNN(
+      State, BO_GE, nonloc::SymbolVal(Sym),
+      nonloc::ConcreteInt(BV.getValue(Min)), SVB.getConditionType());
   if (auto DV = IsCappedFromBelow.getAs<DefinedSVal>()) {
     NewState = NewState->assume(*DV, true);
     if (!NewState)
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
index 5649454b4cd47e..d4ce73b03acb82 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -507,8 +507,8 @@ void IteratorModeling::processComparison(CheckerContext &C,
                                          OverloadedOperatorKind Op) const {
   if (const auto TruthVal = RetVal.getAs<nonloc::ConcreteInt>()) {
     if ((State = relateSymbols(State, Sym1, Sym2,
-                              (Op == OO_EqualEqual) ==
-                               (TruthVal->getValue() != 0)))) {
+                               (Op == OO_EqualEqual) ==
+                                   (TruthVal->getValue()->getBoolValue())))) {
       C.addTransition(State);
     } else {
       C.generateSink(State, C.getPredecessor());
diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
index 4b8e5216550d93..9a8c128edc2331 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -68,7 +68,7 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call,
     auto ProtLoc = ProtVal.getAs<nonloc::ConcreteInt>();
     if (!ProtLoc)
       return;
-    int64_t Prot = ProtLoc->getValue().getSExtValue();
+    int64_t Prot = ProtLoc->getValue()->getSExtValue();
 
     if ((Prot & ProtWrite) && (Prot & ProtExec)) {
       ExplodedNode *N = C.generateNonFatalErrorNode();
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 0a823a1126ce3f..80969ce6645306 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1977,7 +1977,7 @@ StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
   if (!CI)
     return State;
 
-  int64_t X = CI->getValue().getSExtValue();
+  int64_t X = CI->getValue()->getSExtValue();
   if (X == SeekSetVal || X == SeekCurVal || X == SeekEndVal)
     return State;
 
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index c4479db14b791d..a9b4dbb39b5bd6 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -264,7 +264,7 @@ getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
 
   if (std::optional<SVal> V = getSValForVar(CondVarExpr, N))
     if (auto CI = V->getAs<nonloc::ConcreteInt>())
-      return &CI->getValue();
+      return CI->getValue().get();
   return std::nullopt;
 }
 
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index bbf2303b9f6ef3..559c80634c12e5 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -743,7 +743,7 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes) const {
     // Index is a ConcreteInt.
     if (auto CI = ER->getIndex().getAs<nonloc::ConcreteInt>()) {
       llvm::SmallString<2> Idx;
-      CI->getValue().toString(Idx);
+      CI->getValue()->toString(Idx);
       ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str();
     }
     // Index is symbolic, but may have a descriptive name.
@@ -1458,9 +1458,7 @@ RegionRawOffset ElementRegion::getAsArrayOffset() const {
     SVal index = ER->getIndex();
     if (auto CI = index.getAs<nonloc::ConcreteInt>()) {
       // Update the offset.
-      int64_t i = CI->getValue().getSExtValue();
-
-      if (i != 0) {
+      if (int64_t i = CI->getValue()->getSExtValue(); i != 0) {
         QualType elemType = ER->getElementType();
 
         // If we are pointing to an incomplete type, go no further.
@@ -1632,7 +1630,7 @@ static RegionOffset calculateOffset(const MemRegion *R) {
         if (SymbolicOffsetBase)
           continue;
 
-        int64_t i = CI->getValue().getSExtValue();
+        int64_t i = CI->getValue()->getSExtValue();
         // This type size is in bits.
         Offset += i * R->getContext().getTypeSize(EleTy);
       } else {
diff --git a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
index d4f56342d934c9..34ab2388cbd2f0 100644
--- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -288,12 +288,10 @@ SVal ProgramState::getSVal(Loc location, QualType T) const {
         //  The symbolic value stored to 'x' is actually the conjured
         //  symbol for the call to foo(); the type of that symbol is 'char',
         //  not unsigned.
-        const llvm::APSInt &NewV = getBasicVals().Convert(T, *Int);
-
+        APSIntPtr NewV = getBasicVals().Convert(T, *Int);
         if (V.getAs<Loc>())
           return loc::ConcreteInt(NewV);
-        else
-          return nonloc::ConcreteInt(NewV);
+        return nonloc::ConcreteInt(NewV);
       }
     }
   }
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 92e9d245520345..5741fff0cc12f7 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -875,7 +875,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
 
     // Integer to bool.
     if (CastTy->isBooleanType())
-      return VB.makeTruthVal(V.getValue().getBoolValue(), CastTy);
+      return VB.makeTruthVal(V.getValue()->getBoolValue(), CastTy);
 
     // Integer to pointer.
     if (CastTy->isIntegralOrEnumerationType())
diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp
index d009552965eca8..ec88f52a2b3c58 100644
--- a/clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -111,7 +111,7 @@ SymbolRef SVal::getAsSymbol(bool IncludeBaseRegions) const {
 
 const llvm::APSInt *SVal::getAsInteger() const {
   if (auto CI = getAs<nonloc::ConcreteInt>())
-    return &CI->getValue();
+    return CI->getValue().get();
   if (auto CI = getAs<loc::ConcreteInt>())
     return &CI->getValue();
   return nullptr;
@@ -251,7 +251,7 @@ bool SVal::isConstant(int I) const {
   if (std::optional<loc::ConcreteInt> LV = getAs<loc::ConcreteInt>())
     return LV->getValue() == I;
   if (std::optional<nonloc::ConcreteInt> NV = getAs<nonloc::ConcreteInt>())
-    return NV->getValue() == I;
+    return *NV->getValue().get() == I;
   return false;
 }
 
@@ -314,9 +314,9 @@ void SVal::dumpToStream(raw_ostream &os) const {
 void NonLoc::dumpToStream(raw_ostream &os) const {
   switch (getKind()) {
   case nonloc::ConcreteIntKind: {
-    const auto &Value = castAs<nonloc::ConcreteInt>().getValue();
-    os << Value << ' ' << (Value.isSigned() ? 'S' : 'U') << Value.getBitWidth()
-       << 'b';
+    APSIntPtr Value = castAs<nonloc::ConcreteInt>().getValue();
+    os << Value << ' ' << (Value->isSigned() ? 'S' : 'U')
+       << Value->getBitWidth() << 'b';
     break;
   }
     case nonloc::SymbolValKind:
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index 8ca2cdb9d3ab7a..3c5c992fa8dbc7 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -75,7 +75,7 @@ ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
   }
 
   case nonloc::ConcreteIntKind: {
-    bool b = Cond.castAs<nonloc::ConcreteInt>().getValue() != 0;
+    bool b = *Cond.castAs<nonloc::ConcreteInt>().getValue().get() != 0;
     bool isFeasible = b ? Assumption : !Assumption;
     return isFeasible ? State : nullptr;
   }
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 7b7fc801ec7f4a..d2e6870ad17079 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -10,10 +10,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include <optional>
 
@@ -179,8 +180,7 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
     if (RHS == 0)
       isIdempotent = true;
     else if (RHS.isAllOnes()) {
-      const llvm::APSInt &Result = BasicVals.Convert(resultTy, RHS);
-      return nonloc::ConcreteInt(Result);
+      return nonloc::ConcreteInt(BasicVals.Convert(resultTy, RHS));
     }
     break;
   }
@@ -234,9 +234,10 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
 static bool isInRelation(BinaryOperator::Opcode Rel, SymbolRef Sym,
                          llvm::APSInt Bound, ProgramStateRef State) {
   SValBuilder &SVB = State->getStateManager().getSValBuilder();...
[truncated]

@steakhal steakhal force-pushed the users/steakhal/bb/fix-dangling-aps-ints-1 branch from eddf348 to 587368a Compare December 19, 2024 10:11
@steakhal steakhal force-pushed the users/steakhal/bb/fix-dangling-aps-ints-2 branch 2 times, most recently from d9ce18f to 09aa29d Compare December 19, 2024 10:19
Comment on lines 243 to 265
getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
SValBuilder &svalBuilder) {
const llvm::APSInt &extentVal = extent.getValue();
std::optional<nonloc::SymbolVal> SymVal = offset.getAs<nonloc::SymbolVal>();
if (SymVal && SymVal->isExpression()) {
if (const SymIntExpr *SIE = dyn_cast<SymIntExpr>(SymVal->getSymbol())) {
llvm::APSInt constant =
APSIntType(extent.getValue()).convert(SIE->getRHS());
llvm::APSInt constant = APSIntType(extentVal).convert(SIE->getRHS());
switch (SIE->getOpcode()) {
case BO_Mul:
// The constant should never be 0 here, becasue multiplication by zero
// is simplified by the engine.
if ((extent.getValue() % constant) != 0)
if ((extentVal % constant) != 0)
return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
else
return getSimplifiedOffsets(
nonloc::SymbolVal(SIE->getLHS()),
svalBuilder.makeIntVal(extent.getValue() / constant),
svalBuilder);
svalBuilder.makeIntVal(extentVal / constant), svalBuilder);
case BO_Add:
return getSimplifiedOffsets(
nonloc::SymbolVal(SIE->getLHS()),
svalBuilder.makeIntVal(extent.getValue() - constant), svalBuilder);
svalBuilder.makeIntVal(extentVal - constant), svalBuilder);
default:
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly offtopic request: as you're already editing many lines of this function, please convert it to UpperCamelCase variable names (which is the standard suggested by the LLVM style guide).

When I started to work on this checker in April 2023, it was an ugly mixture of UpperCamelCase and lowerCamelCase variable names; but then, as I refactored various parts of it, I gradually renamed (or removed) most old lowerCamelCase variables, so (if I recall correctly) this is the last function that heavily uses them.

Base automatically changed from users/steakhal/bb/fix-dangling-aps-ints-1 to main December 19, 2024 11:04
@steakhal steakhal force-pushed the users/steakhal/bb/fix-dangling-aps-ints-2 branch from 09aa29d to 75579cb Compare December 19, 2024 11:05
Copy link
Contributor Author

steakhal commented Dec 19, 2024

Merge activity

  • Dec 19, 6:49 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Dec 19, 6:51 AM EST: A user merged this pull request with Graphite.

@steakhal steakhal merged commit d0d5101 into main Dec 19, 2024
6 of 8 checks passed
@steakhal steakhal deleted the users/steakhal/bb/fix-dangling-aps-ints-2 branch December 19, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants