[clang] df64f47 - [analyzer] DynamicSize: Store the dynamic size

via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 5 10:05:33 PDT 2021


Author: Charusso
Date: 2021-04-05T19:04:53+02:00
New Revision: df64f471d1e26fc1e9e2f9cdcfc77c063fe55b56

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

LOG: [analyzer] DynamicSize: Store the dynamic size

This patch introduces a way to store the size.

Reviewed By: NoQ

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
    clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
    clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    clang/lib/StaticAnalyzer/Core/MemRegion.cpp
    clang/test/Analysis/explain-svals.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
index 398f9b6ac33a..bad6a19cb9ff 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
@@ -22,15 +22,21 @@
 namespace clang {
 namespace ento {
 
-/// Get the stored dynamic size for the region \p MR.
+/// \returns The stored dynamic size for the region \p MR.
 DefinedOrUnknownSVal getDynamicSize(ProgramStateRef State, const MemRegion *MR,
                                     SValBuilder &SVB);
 
-/// Get the stored element count of the region \p MR.
+/// \returns The element size of the type \p Ty.
+DefinedOrUnknownSVal getElementSize(QualType Ty, SValBuilder &SVB);
+
+/// \returns The stored element count of the region \p MR.
 DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
                                             const MemRegion *MR,
-                                            SValBuilder &SVB,
-                                            QualType ElementTy);
+                                            SValBuilder &SVB, QualType Ty);
+
+/// Set the dynamic size \p Size of the region \p MR.
+ProgramStateRef setDynamicSize(ProgramStateRef State, const MemRegion *MR,
+                               DefinedOrUnknownSVal Size, SValBuilder &SVB);
 
 /// Get the dynamic size for a symbolic value that represents a buffer. If
 /// there is an offsetting to the underlying buffer we consider that too.
@@ -45,7 +51,7 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
 ///
 ///   char *bufptr;
 ///   (bufptr) // size is unknown
-SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV);
+SVal getDynamicSizeWithOffset(ProgramStateRef State, SVal BufV);
 
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 233ce57c3ac9..31f9b14f7eeb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -92,12 +92,8 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
     if (Size.isUndef())
       return true; // Return true to model purity.
 
-    SValBuilder& svalBuilder = C.getSValBuilder();
-    DefinedOrUnknownSVal DynSize = getDynamicSize(state, R, svalBuilder);
-    DefinedOrUnknownSVal DynSizeMatchesSizeArg =
-        svalBuilder.evalEQ(state, DynSize, Size.castAs<DefinedOrUnknownSVal>());
-    state = state->assume(DynSizeMatchesSizeArg, true);
-    assert(state && "The region should not have any previous constraints");
+    state = setDynamicSize(state, R, Size.castAs<DefinedOrUnknownSVal>(),
+                           C.getSValBuilder());
 
     C.addTransition(state->BindExpr(CE, LCtx, loc::MemRegionVal(R)));
     return true;

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f117d5505ecb..7eafdaa97bb4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -509,10 +509,6 @@ class MallocChecker
                                       ProgramStateRef State,
                                       AllocationFamily Family);
 
-  LLVM_NODISCARD
-  static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE,
-                                       ProgramStateRef State, SVal Target);
-
   // Check if this malloc() for special flags. At present that means M_ZERO or
   // __GFP_ZERO (in which case, treat it like calloc).
   LLVM_NODISCARD
@@ -1424,7 +1420,6 @@ MallocChecker::processNewAllocation(const CXXAllocatorCall &Call,
   // existing binding.
   SVal Target = Call.getObjectUnderConstruction();
   State = MallocUpdateRefState(C, NE, State, Family, Target);
-  State = addExtentSize(C, NE, State, Target);
   State = ProcessZeroAllocCheck(Call, 0, State, Target);
   return State;
 }
@@ -1439,52 +1434,6 @@ void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call,
   }
 }
 
-// Sets the extent value of the MemRegion allocated by
-// new expression NE to its size in Bytes.
-//
-ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C,
-                                             const CXXNewExpr *NE,
-                                             ProgramStateRef State,
-                                             SVal Target) {
-  if (!State)
-    return nullptr;
-  SValBuilder &svalBuilder = C.getSValBuilder();
-  SVal ElementCount;
-  const SubRegion *Region;
-  if (NE->isArray()) {
-    const Expr *SizeExpr = *NE->getArraySize();
-    ElementCount = C.getSVal(SizeExpr);
-    // Store the extent size for the (symbolic)region
-    // containing the elements.
-    Region = Target.getAsRegion()
-                 ->castAs<SubRegion>()
-                 ->StripCasts()
-                 ->castAs<SubRegion>();
-  } else {
-    ElementCount = svalBuilder.makeIntVal(1, true);
-    Region = Target.getAsRegion()->castAs<SubRegion>();
-  }
-
-  // Set the region's extent equal to the Size in Bytes.
-  QualType ElementType = NE->getAllocatedType();
-  ASTContext &AstContext = C.getASTContext();
-  CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
-
-  if (ElementCount.getAs<NonLoc>()) {
-    DefinedOrUnknownSVal DynSize = getDynamicSize(State, Region, svalBuilder);
-
-    // size in Bytes = ElementCount*TypeSize
-    SVal SizeInBytes = svalBuilder.evalBinOpNN(
-        State, BO_Mul, ElementCount.castAs<NonLoc>(),
-        svalBuilder.makeArrayIndex(TypeSize.getQuantity()),
-        svalBuilder.getArrayIndexType());
-    DefinedOrUnknownSVal DynSizeMatchesSize = svalBuilder.evalEQ(
-        State, DynSize, SizeInBytes.castAs<DefinedOrUnknownSVal>());
-    State = State->assume(DynSizeMatchesSize, true);
-  }
-  return State;
-}
-
 static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) {
   // If the first selector piece is one of the names below, assume that the
   // object takes ownership of the memory, promising to eventually deallocate it
@@ -1588,21 +1537,9 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   // Fill the region with the initialization value.
   State = State->bindDefaultInitial(RetVal, Init, LCtx);
 
-  // Set the region's extent equal to the Size parameter.
-  const SymbolicRegion *R =
-      dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion());
-  if (!R)
-    return nullptr;
-  if (Optional<DefinedOrUnknownSVal> DefinedSize =
-          Size.getAs<DefinedOrUnknownSVal>()) {
-    DefinedOrUnknownSVal DynSize = getDynamicSize(State, R, svalBuilder);
-
-    DefinedOrUnknownSVal DynSizeMatchesSize =
-        svalBuilder.evalEQ(State, DynSize, *DefinedSize);
-
-    State = State->assume(DynSizeMatchesSize, true);
-    assert(State);
-  }
+  // Set the region's extent.
+  State = setDynamicSize(State, RetVal.getAsRegion(),
+                         Size.castAs<DefinedOrUnknownSVal>(), svalBuilder);
 
   return MallocUpdateRefState(C, CE, State, Family);
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index d76b2a06aba5..78cee9cba97f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -285,21 +285,10 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
     return;
   }
 
-  // VLASizeChecker is responsible for defining the extent of the array being
-  // declared. We do this by multiplying the array length by the element size,
-  // then matching that with the array region's extent symbol.
-
+  // VLASizeChecker is responsible for defining the extent of the array.
   if (VD) {
-    // Assume that the array's size matches the region size.
-    const LocationContext *LC = C.getLocationContext();
-    DefinedOrUnknownSVal DynSize =
-        getDynamicSize(State, State->getRegion(VD, LC), SVB);
-
-    DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, *ArraySizeNL);
-    State = State->assume(SizeIsKnown, true);
-
-    // Assume should not fail at this point.
-    assert(State);
+    State = setDynamicSize(State, State->getRegion(VD, C.getLocationContext()),
+                           ArraySize.castAs<DefinedOrUnknownSVal>(), SVB);
   }
 
   // Remember our assumptions!

diff  --git a/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp b/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
index 8b2172db445c..0f9c8121ba26 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
+++ b/clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
@@ -19,32 +19,43 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicSizeMap, const clang::ento::MemRegion *,
+                               clang::ento::DefinedOrUnknownSVal)
+
 namespace clang {
 namespace ento {
 
 DefinedOrUnknownSVal getDynamicSize(ProgramStateRef State, const MemRegion *MR,
                                     SValBuilder &SVB) {
+  MR = MR->StripCasts();
+
+  if (const DefinedOrUnknownSVal *Size = State->get<DynamicSizeMap>(MR))
+    return *Size;
+
   return MR->getMemRegionManager().getStaticSize(MR, SVB);
 }
 
+DefinedOrUnknownSVal getElementSize(QualType Ty, SValBuilder &SVB) {
+  return SVB.makeIntVal(SVB.getContext().getTypeSizeInChars(Ty).getQuantity(),
+                        SVB.getArrayIndexType());
+}
+
 DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
                                             const MemRegion *MR,
                                             SValBuilder &SVB,
                                             QualType ElementTy) {
-  MemRegionManager &MemMgr = MR->getMemRegionManager();
-  ASTContext &Ctx = MemMgr.getContext();
+  MR = MR->StripCasts();
 
   DefinedOrUnknownSVal Size = getDynamicSize(State, MR, SVB);
-  SVal ElementSizeV = SVB.makeIntVal(
-      Ctx.getTypeSizeInChars(ElementTy).getQuantity(), SVB.getArrayIndexType());
+  SVal ElementSize = getElementSize(ElementTy, SVB);
 
-  SVal DivisionV =
-      SVB.evalBinOp(State, BO_Div, Size, ElementSizeV, SVB.getArrayIndexType());
+  SVal ElementCount =
+      SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType());
 
-  return DivisionV.castAs<DefinedOrUnknownSVal>();
+  return ElementCount.castAs<DefinedOrUnknownSVal>();
 }
 
-SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV) {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, SVal BufV) {
   SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
   const MemRegion *MRegion = BufV.getAsRegion();
   if (!MRegion)
@@ -67,5 +78,15 @@ SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV) {
                                SvalBuilder.getArrayIndexType());
 }
 
+ProgramStateRef setDynamicSize(ProgramStateRef State, const MemRegion *MR,
+                               DefinedOrUnknownSVal Size, SValBuilder &SVB) {
+  MR = MR->StripCasts();
+
+  if (Size.isUnknown())
+    return State;
+
+  return State->set<DynamicSizeMap>(MR->StripCasts(), Size);
+}
+
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 996d3644e018..cbe6b5338348 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -18,6 +18,7 @@
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
@@ -689,16 +690,30 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
 
     // See if we need to conjure a heap pointer instead of
     // a regular unknown pointer.
-    bool IsHeapPointer = false;
-    if (const auto *CNE = dyn_cast<CXXNewExpr>(E))
-      if (CNE->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
-        // FIXME: Delegate this to evalCall in MallocChecker?
-        IsHeapPointer = true;
+    const auto *CNE = dyn_cast<CXXNewExpr>(E);
+    if (CNE && CNE->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+      R = svalBuilder.getConjuredHeapSymbolVal(E, LCtx, Count);
+      const MemRegion *MR = R.getAsRegion()->StripCasts();
+
+      // Store the extent of the allocated object(s).
+      SVal ElementCount;
+      if (const Expr *SizeExpr = CNE->getArraySize().getValueOr(nullptr)) {
+        ElementCount = State->getSVal(SizeExpr, LCtx);
+      } else {
+        ElementCount = svalBuilder.makeIntVal(1, /*IsUnsigned=*/true);
       }
 
-    R = IsHeapPointer ? svalBuilder.getConjuredHeapSymbolVal(E, LCtx, Count)
-                      : svalBuilder.conjureSymbolVal(nullptr, E, LCtx, ResultTy,
-                                                     Count);
+      SVal ElementSize = getElementSize(CNE->getAllocatedType(), svalBuilder);
+
+      SVal Size =
+          svalBuilder.evalBinOp(State, BO_Mul, ElementCount, ElementSize,
+                                svalBuilder.getArrayIndexType());
+
+      State = setDynamicSize(State, MR, Size.castAs<DefinedOrUnknownSVal>(),
+                             svalBuilder);
+    } else {
+      R = svalBuilder.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
+    }
   }
   return State->BindExpr(E, LCtx, R);
 }

diff  --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 455adf53ac99..bb0038d08a4f 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -729,13 +730,6 @@ SourceRange MemRegion::sourceRange() const {
 // MemRegionManager methods.
 //===----------------------------------------------------------------------===//
 
-static DefinedOrUnknownSVal getTypeSize(QualType Ty, ASTContext &Ctx,
-                                          SValBuilder &SVB) {
-  CharUnits Size = Ctx.getTypeSizeInChars(Ty);
-  QualType SizeTy = SVB.getArrayIndexType();
-  return SVB.makeIntVal(Size.getQuantity(), SizeTy);
-}
-
 DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
                                                      SValBuilder &SVB) const {
   const auto *SR = cast<SubRegion>(MR);
@@ -766,7 +760,7 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
     if (Ty->isIncompleteType())
       return UnknownVal();
 
-    return getTypeSize(Ty, Ctx, SVB);
+    return getElementSize(Ty, SVB);
   }
   case MemRegion::FieldRegionKind: {
     // Force callers to deal with bitfields explicitly.
@@ -774,7 +768,7 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
       return UnknownVal();
 
     QualType Ty = cast<TypedValueRegion>(SR)->getDesugaredValueType(Ctx);
-    DefinedOrUnknownSVal Size = getTypeSize(Ty, Ctx, SVB);
+    DefinedOrUnknownSVal Size = getElementSize(Ty, SVB);
 
     // A zero-length array at the end of a struct often stands for dynamically
     // allocated extra memory.

diff  --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp
index c33373f00de2..387530c17f6c 100644
--- a/clang/test/Analysis/explain-svals.cpp
+++ b/clang/test/Analysis/explain-svals.cpp
@@ -54,7 +54,7 @@ void test_2(char *ptr, int ext) {
   int *x = new int[ext];
   clang_analyzer_explain(x); // expected-warning-re{{{{^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$}}}}
   // Sic! What gets computed is the extent of the element-region.
-  clang_analyzer_explain(clang_analyzer_getExtent(x)); // expected-warning-re{{{{^signed 32-bit integer '4'$}}}}
+  clang_analyzer_explain(clang_analyzer_getExtent(x)); // expected-warning-re{{{{^\(argument 'ext'\) \* 4$}}}}
   delete[] x;
 }
 


        


More information about the cfe-commits mailing list