[analyzer] Refactor MallocChecker to use BindExpr in evalCall (#106081)

PR refactors `MallocChecker` to not violate invariant of `BindExpr`,
which should be called only during `evalCall` to avoid conflicts.

To achieve this, most of `postCall` logic was moved to `evalCall` with
addition return value binding in case of processing of allocation
functions. Check functions prototypes was changed to use `State` with
bound return value.

`checkDelim` logic was left in `postCall` to avoid conflicts with
`StreamChecker` which also evaluates `getline` and friends.

PR also introduces breaking change in the unlikely case when the
definition of an allocation function (e.g. `malloc()`) is visible: now
checker does not try to inline allocation functions and assumes their
initial semantics.

Closes #73830

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

@@ -36,7 +36,7 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
 
 /// Set the dynamic extent \p Extent of the region \p MR.
 ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
-                                 DefinedOrUnknownSVal Extent, SValBuilder &SVB);
+                                 DefinedOrUnknownSVal Extent);
 
 /// Get the dynamic extent for a symbolic value that represents a buffer. If
 /// there is an offsetting to the underlying buffer we consider that too.

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

@@ -215,17 +215,17 @@ public:
   /// Conjure a symbol representing heap allocated memory region.
   ///
   /// Note, the expression should represent a location.
-  DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
-                                                const LocationContext *LCtx,
-                                                unsigned Count);
+  DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
+                                       const LocationContext *LCtx,
+                                       unsigned Count);
 
   /// Conjure a symbol representing heap allocated memory region.
   ///
   /// Note, now, the expression *doesn't* need to represent a location.
   /// But the type need to!
-  DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
-                                                const LocationContext *LCtx,
-                                                QualType type, unsigned Count);
+  DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
+                                       const LocationContext *LCtx,
+                                       QualType type, unsigned Count);
 
   /// Create an SVal representing the result of an alloca()-like call, that is,
   /// an AllocaRegion on the stack.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -315,13 +315,24 @@ struct ReallocPair {
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
 
-/// Tells if the callee is one of the builtin new/delete operators, including
-/// placement operators and other standard overloads.
-static bool isStandardNewDelete(const FunctionDecl *FD);
-static bool isStandardNewDelete(const CallEvent &Call) {
+static bool isStandardNew(const FunctionDecl *FD);
+static bool isStandardNew(const CallEvent &Call) {
   if (!Call.getDecl() || !isa<FunctionDecl>(Call.getDecl()))
     return false;
-  return isStandardNewDelete(cast<FunctionDecl>(Call.getDecl()));
+  return isStandardNew(cast<FunctionDecl>(Call.getDecl()));
+}
+
+static bool isStandardDelete(const FunctionDecl *FD);
+static bool isStandardDelete(const CallEvent &Call) {
+  if (!Call.getDecl() || !isa<FunctionDecl>(Call.getDecl()))
+    return false;
+  return isStandardDelete(cast<FunctionDecl>(Call.getDecl()));
+}
+
+/// Tells if the callee is one of the builtin new/delete operators, including
+/// placement operators and other standard overloads.
+template <typename T> static bool isStandardNewDelete(const T &FD) {
+  return isStandardDelete(FD) || isStandardNew(FD);
 }
 
 //===----------------------------------------------------------------------===//
@@ -334,8 +345,9 @@ class MallocChecker
     : public Checker<check::DeadSymbols, check::PointerEscape,
                      check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
                      check::EndFunction, check::PreCall, check::PostCall,
-                     check::NewAllocator, check::PostStmt<BlockExpr>,
-                     check::PostObjCMessage, check::Location, eval::Assume> {
+                     eval::Call, check::NewAllocator,
+                     check::PostStmt<BlockExpr>, check::PostObjCMessage,
+                     check::Location, eval::Assume> {
 public:
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -367,6 +379,7 @@ public:
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
@@ -403,7 +416,8 @@ private:
   mutable std::unique_ptr<BugType> BT_TaintedAlloc;
 
 #define CHECK_FN(NAME)                                                         \
-  void NAME(const CallEvent &Call, CheckerContext &C) const;
+  void NAME(ProgramStateRef State, const CallEvent &Call, CheckerContext &C)   \
+      const;
 
   CHECK_FN(checkFree)
   CHECK_FN(checkIfNameIndex)
@@ -423,11 +437,12 @@ private:
   CHECK_FN(checkReallocN)
   CHECK_FN(checkOwnershipAttr)
 
-  void checkRealloc(const CallEvent &Call, CheckerContext &C,
-                    bool ShouldFreeOnFail) const;
+  void checkRealloc(ProgramStateRef State, const CallEvent &Call,
+                    CheckerContext &C, bool ShouldFreeOnFail) const;
 
-  using CheckFn = std::function<void(const MallocChecker *,
-                                     const CallEvent &Call, CheckerContext &C)>;
+  using CheckFn =
+      std::function<void(const MallocChecker *, ProgramStateRef State,
+                         const CallEvent &Call, CheckerContext &C)>;
 
   const CallDescriptionMap<CheckFn> PreFnMap{
       // NOTE: the following CallDescription also matches the C++ standard
@@ -436,6 +451,13 @@ private:
       {{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::preGetdelim},
   };
 
+  const CallDescriptionMap<CheckFn> PostFnMap{
+      // NOTE: the following CallDescription also matches the C++ standard
+      // library function std::getline(); the callback will filter it out.
+      {{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::checkGetdelim},
+      {{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::checkGetdelim},
+  };
+
   const CallDescriptionMap<CheckFn> FreeingMemFnMap{
       {{CDM::CLibrary, {"free"}, 1}, &MallocChecker::checkFree},
       {{CDM::CLibrary, {"if_freenameindex"}, 1},
@@ -446,10 +468,13 @@ private:
 
   bool isFreeingCall(const CallEvent &Call) const;
   static bool isFreeingOwnershipAttrCall(const FunctionDecl *Func);
+  static bool isFreeingOwnershipAttrCall(const CallEvent &Call);
+  static bool isAllocatingOwnershipAttrCall(const FunctionDecl *Func);
+  static bool isAllocatingOwnershipAttrCall(const CallEvent &Call);
 
   friend class NoMemOwnershipChangeVisitor;
 
-  CallDescriptionMap<CheckFn> AllocatingMemFnMap{
+  CallDescriptionMap<CheckFn> AllocaMemFnMap{
       {{CDM::CLibrary, {"alloca"}, 1}, &MallocChecker::checkAlloca},
       {{CDM::CLibrary, {"_alloca"}, 1}, &MallocChecker::checkAlloca},
       // The line for "alloca" also covers "__builtin_alloca", but the
@@ -457,6 +482,9 @@ private:
       // extra argument:
       {{CDM::CLibrary, {"__builtin_alloca_with_align"}, 2},
        &MallocChecker::checkAlloca},
+  };
+
+  CallDescriptionMap<CheckFn> AllocatingMemFnMap{
       {{CDM::CLibrary, {"malloc"}, 1}, &MallocChecker::checkBasicAlloc},
       {{CDM::CLibrary, {"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
       {{CDM::CLibrary, {"calloc"}, 2}, &MallocChecker::checkCalloc},
@@ -481,23 +509,20 @@ private:
 
   CallDescriptionMap<CheckFn> ReallocatingMemFnMap{
       {{CDM::CLibrary, {"realloc"}, 2},
-       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
+       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, _4, false)},
       {{CDM::CLibrary, {"reallocf"}, 2},
-       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, true)},
+       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, _4, true)},
       {{CDM::CLibrary, {"g_realloc"}, 2},
-       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
+       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, _4, false)},
       {{CDM::CLibrary, {"g_try_realloc"}, 2},
-       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
+       std::bind(&MallocChecker::checkRealloc, _1, _2, _3, _4, false)},
       {{CDM::CLibrary, {"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
       {{CDM::CLibrary, {"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
-
-      // NOTE: the following CallDescription also matches the C++ standard
-      // library function std::getline(); the callback will filter it out.
-      {{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::checkGetdelim},
-      {{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::checkGetdelim},
   };
 
   bool isMemCall(const CallEvent &Call) const;
+  bool hasOwnershipReturns(const CallEvent &Call) const;
+  bool hasOwnershipTakesHolds(const CallEvent &Call) const;
   void reportTaintBug(StringRef Msg, ProgramStateRef State, CheckerContext &C,
                       llvm::ArrayRef<SymbolRef> TaintedSyms,
                       AllocationFamily Family) const;
@@ -531,8 +556,8 @@ private:
   /// \param [in] RetVal Specifies the newly allocated pointer value;
   ///   if unspecified, the value of expression \p E is used.
   [[nodiscard]] static ProgramStateRef
-  ProcessZeroAllocCheck(const CallEvent &Call, const unsigned IndexOfSizeArg,
-                        ProgramStateRef State,
+  ProcessZeroAllocCheck(CheckerContext &C, const CallEvent &Call,
+                        const unsigned IndexOfSizeArg, ProgramStateRef State,
                         std::optional<SVal> RetVal = std::nullopt);
 
   /// Model functions with the ownership_returns attribute.
@@ -554,6 +579,17 @@ private:
   [[nodiscard]] ProgramStateRef
   MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
                        const OwnershipAttr *Att, ProgramStateRef State) const;
+  /// Models memory allocation.
+  ///
+  /// \param [in] C Checker context.
+  /// \param [in] Call The expression that allocates memory.
+  /// \param [in] State The \c ProgramState right before allocation.
+  /// \param [in] isAlloca Is the allocation function alloca-like
+  /// \returns The ProgramState with returnValue bound
+  [[nodiscard]] ProgramStateRef MallocBindRetVal(CheckerContext &C,
+                                                 const CallEvent &Call,
+                                                 ProgramStateRef State,
+                                                 bool isAlloca) const;
 
   /// Models memory allocation.
   ///
@@ -1031,13 +1067,28 @@ public:
 };
 } // end anonymous namespace
 
-static bool isStandardNewDelete(const FunctionDecl *FD) {
+static bool isStandardNew(const FunctionDecl *FD) {
   if (!FD)
     return false;
 
   OverloadedOperatorKind Kind = FD->getOverloadedOperator();
-  if (Kind != OO_New && Kind != OO_Array_New && Kind != OO_Delete &&
-      Kind != OO_Array_Delete)
+  if (Kind != OO_New && Kind != OO_Array_New)
+    return false;
+
+  // This is standard if and only if it's not defined in a user file.
+  SourceLocation L = FD->getLocation();
+  // If the header for operator delete is not included, it's still defined
+  // in an invalid source location. Check to make sure we don't crash.
+  return !L.isValid() ||
+         FD->getASTContext().getSourceManager().isInSystemHeader(L);
+}
+
+static bool isStandardDelete(const FunctionDecl *FD) {
+  if (!FD)
+    return false;
+
+  OverloadedOperatorKind Kind = FD->getOverloadedOperator();
+  if (Kind != OO_Delete && Kind != OO_Array_Delete)
     return false;
 
   // This is standard if and only if it's not defined in a user file.
@@ -1052,6 +1103,12 @@ static bool isStandardNewDelete(const FunctionDecl *FD) {
 // Methods of MallocChecker and MallocBugVisitor.
 //===----------------------------------------------------------------------===//
 
+bool MallocChecker::isFreeingOwnershipAttrCall(const CallEvent &Call) {
+  const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+
+  return Func && isFreeingOwnershipAttrCall(Func);
+}
+
 bool MallocChecker::isFreeingOwnershipAttrCall(const FunctionDecl *Func) {
   if (Func->hasAttrs()) {
     for (const auto *I : Func->specific_attrs<OwnershipAttr>()) {
@@ -1067,15 +1124,27 @@ bool MallocChecker::isFreeingCall(const CallEvent &Call) const {
   if (FreeingMemFnMap.lookup(Call) || ReallocatingMemFnMap.lookup(Call))
     return true;
 
-  if (const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
-    return isFreeingOwnershipAttrCall(Func);
+  return isFreeingOwnershipAttrCall(Call);
+}
+
+bool MallocChecker::isAllocatingOwnershipAttrCall(const CallEvent &Call) {
+  const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+
+  return Func && isAllocatingOwnershipAttrCall(Func);
+}
+
+bool MallocChecker::isAllocatingOwnershipAttrCall(const FunctionDecl *Func) {
+  for (const auto *I : Func->specific_attrs<OwnershipAttr>()) {
+    if (I->getOwnKind() == OwnershipAttr::Returns)
+      return true;
+  }
 
   return false;
 }
 
 bool MallocChecker::isMemCall(const CallEvent &Call) const {
   if (FreeingMemFnMap.lookup(Call) || AllocatingMemFnMap.lookup(Call) ||
-      ReallocatingMemFnMap.lookup(Call))
+      AllocaMemFnMap.lookup(Call) || ReallocatingMemFnMap.lookup(Call))
     return true;
 
   if (!ShouldIncludeOwnershipAnnotatedFunctions)
@@ -1182,18 +1251,18 @@ SVal MallocChecker::evalMulForBufferSize(CheckerContext &C, const Expr *Blocks,
   return TotalSize;
 }
 
-void MallocChecker::checkBasicAlloc(const CallEvent &Call,
+void MallocChecker::checkBasicAlloc(ProgramStateRef State,
+                                    const CallEvent &Call,
                                     CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State,
                        AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkKernelMalloc(const CallEvent &Call,
+void MallocChecker::checkKernelMalloc(ProgramStateRef State,
+                                      const CallEvent &Call,
                                       CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   std::optional<ProgramStateRef> MaybeState =
       performKernelMalloc(Call, C, State);
   if (MaybeState)
@@ -1226,7 +1295,8 @@ static bool isGRealloc(const CallEvent &Call) {
              AC.UnsignedLongTy;
 }
 
-void MallocChecker::checkRealloc(const CallEvent &Call, CheckerContext &C,
+void MallocChecker::checkRealloc(ProgramStateRef State, const CallEvent &Call,
+                                 CheckerContext &C,
                                  bool ShouldFreeOnFail) const {
   // Ignore calls to functions whose type does not match the expected type of
   // either the standard realloc or g_realloc from GLib.
@@ -1236,24 +1306,22 @@ void MallocChecker::checkRealloc(const CallEvent &Call, CheckerContext &C,
   if (!isStandardRealloc(Call) && !isGRealloc(Call))
     return;
 
-  ProgramStateRef State = C.getState();
   State = ReallocMemAux(C, Call, ShouldFreeOnFail, State,
                         AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkCalloc(const CallEvent &Call,
+void MallocChecker::checkCalloc(ProgramStateRef State, const CallEvent &Call,
                                 CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   State = CallocMem(C, Call, State);
-  State = ProcessZeroAllocCheck(Call, 0, State);
-  State = ProcessZeroAllocCheck(Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkFree(const CallEvent &Call, CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
+void MallocChecker::checkFree(ProgramStateRef State, const CallEvent &Call,
+                              CheckerContext &C) const {
   bool IsKnownToBeAllocatedMemory = false;
   if (suppressDeallocationsInSuspiciousContexts(Call, C))
     return;
@@ -1262,29 +1330,28 @@ void MallocChecker::checkFree(const CallEvent &Call, CheckerContext &C) const {
   C.addTransition(State);
 }
 
-void MallocChecker::checkAlloca(const CallEvent &Call,
+void MallocChecker::checkAlloca(ProgramStateRef State, const CallEvent &Call,
                                 CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State,
                        AllocationFamily(AF_Alloca));
-  State = ProcessZeroAllocCheck(Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkStrdup(const CallEvent &Call,
+void MallocChecker::checkStrdup(ProgramStateRef State, const CallEvent &Call,
                                 CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
-  State = MallocUpdateRefState(C, CE, State, AllocationFamily(AF_Malloc));
+  State = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State,
+                       AllocationFamily(AF_Malloc));
 
   C.addTransition(State);
 }
 
-void MallocChecker::checkIfNameIndex(const CallEvent &Call,
+void MallocChecker::checkIfNameIndex(ProgramStateRef State,
+                                     const CallEvent &Call,
                                      CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   // Should we model this differently? We can allocate a fixed number of
   // elements with zeros in the last one.
   State = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State,
@@ -1293,18 +1360,18 @@ void MallocChecker::checkIfNameIndex(const CallEvent &Call,
   C.addTransition(State);
 }
 
-void MallocChecker::checkIfFreeNameIndex(const CallEvent &Call,
+void MallocChecker::checkIfFreeNameIndex(ProgramStateRef State,
+                                         const CallEvent &Call,
                                          CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   bool IsKnownToBeAllocatedMemory = false;
   State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory,
                      AllocationFamily(AF_IfNameIndex));
   C.addTransition(State);
 }
 
-void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call,
+void MallocChecker::checkCXXNewOrCXXDelete(ProgramStateRef State,
+                                           const CallEvent &Call,
                                            CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   bool IsKnownToBeAllocatedMemory = false;
   const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
@@ -1321,12 +1388,12 @@ void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call,
   case OO_New:
     State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State,
                          AllocationFamily(AF_CXXNew));
-    State = ProcessZeroAllocCheck(Call, 0, State);
+    State = ProcessZeroAllocCheck(C, Call, 0, State);
     break;
   case OO_Array_New:
     State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State,
                          AllocationFamily(AF_CXXNewArray));
-    State = ProcessZeroAllocCheck(Call, 0, State);
+    State = ProcessZeroAllocCheck(C, Call, 0, State);
     break;
   case OO_Delete:
     State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory,
@@ -1344,48 +1411,44 @@ void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call,
   C.addTransition(State);
 }
 
-void MallocChecker::checkGMalloc0(const CallEvent &Call,
+void MallocChecker::checkGMalloc0(ProgramStateRef State, const CallEvent &Call,
                                   CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   SValBuilder &svalBuilder = C.getSValBuilder();
   SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
   State = MallocMemAux(C, Call, Call.getArgExpr(0), zeroVal, State,
                        AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkGMemdup(const CallEvent &Call,
+void MallocChecker::checkGMemdup(ProgramStateRef State, const CallEvent &Call,
                                  CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   State = MallocMemAux(C, Call, Call.getArgExpr(1), UnknownVal(), State,
                        AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkGMallocN(const CallEvent &Call,
+void MallocChecker::checkGMallocN(ProgramStateRef State, const CallEvent &Call,
                                   CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   SVal Init = UndefinedVal();
   SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
   State = MallocMemAux(C, Call, TotalSize, Init, State,
                        AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 0, State);
-  State = ProcessZeroAllocCheck(Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkGMallocN0(const CallEvent &Call,
+void MallocChecker::checkGMallocN0(ProgramStateRef State, const CallEvent &Call,
                                    CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   SValBuilder &SB = C.getSValBuilder();
   SVal Init = SB.makeZeroVal(SB.getContext().CharTy);
   SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
   State = MallocMemAux(C, Call, TotalSize, Init, State,
                        AllocationFamily(AF_Malloc));
-  State = ProcessZeroAllocCheck(Call, 0, State);
-  State = ProcessZeroAllocCheck(Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 0, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
   C.addTransition(State);
 }
 
@@ -1395,14 +1458,13 @@ static bool isFromStdNamespace(const CallEvent &Call) {
   return FD->isInStdNamespace();
 }
 
-void MallocChecker::preGetdelim(const CallEvent &Call,
+void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
                                 CheckerContext &C) const {
   // Discard calls to the C++ standard library function std::getline(), which
   // is completely unrelated to the POSIX getline() that we're checking.
   if (isFromStdNamespace(Call))
     return;
 
-  ProgramStateRef State = C.getState();
   const auto LinePtr = getPointeeVal(Call.getArgSVal(0), State);
   if (!LinePtr)
     return;
@@ -1419,22 +1481,19 @@ void MallocChecker::preGetdelim(const CallEvent &Call,
     C.addTransition(State);
 }
 
-void MallocChecker::checkGetdelim(const CallEvent &Call,
+void MallocChecker::checkGetdelim(ProgramStateRef State, const CallEvent &Call,
                                   CheckerContext &C) const {
   // Discard calls to the C++ standard library function std::getline(), which
   // is completely unrelated to the POSIX getline() that we're checking.
   if (isFromStdNamespace(Call))
     return;
 
-  ProgramStateRef State = C.getState();
   // Handle the post-conditions of getline and getdelim:
   // Register the new conjured value as an allocated buffer.
   const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
 
-  SValBuilder &SVB = C.getSValBuilder();
-
   const auto LinePtr =
       getPointeeVal(Call.getArgSVal(0), State)->getAs<DefinedSVal>();
   const auto Size =
@@ -1442,25 +1501,24 @@ void MallocChecker::checkGetdelim(const CallEvent &Call,
   if (!LinePtr || !Size || !LinePtr->getAsRegion())
     return;
 
-  State = setDynamicExtent(State, LinePtr->getAsRegion(), *Size, SVB);
+  State = setDynamicExtent(State, LinePtr->getAsRegion(), *Size);
   C.addTransition(MallocUpdateRefState(C, CE, State,
                                        AllocationFamily(AF_Malloc), *LinePtr));
 }
 
-void MallocChecker::checkReallocN(const CallEvent &Call,
+void MallocChecker::checkReallocN(ProgramStateRef State, const CallEvent &Call,
                                   CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   State = ReallocMemAux(C, Call, /*ShouldFreeOnFail=*/false, State,
                         AllocationFamily(AF_Malloc),
                         /*SuffixWithN=*/true);
-  State = ProcessZeroAllocCheck(Call, 1, State);
-  State = ProcessZeroAllocCheck(Call, 2, State);
+  State = ProcessZeroAllocCheck(C, Call, 1, State);
+  State = ProcessZeroAllocCheck(C, Call, 2, State);
   C.addTransition(State);
 }
 
-void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
+void MallocChecker::checkOwnershipAttr(ProgramStateRef State,
+                                       const CallEvent &Call,
                                        CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
   const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
@@ -1487,48 +1545,67 @@ void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
   C.addTransition(State);
 }
 
-void MallocChecker::checkPostCall(const CallEvent &Call,
-                                  CheckerContext &C) const {
-  if (C.wasInlined)
-    return;
+bool MallocChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
   if (!Call.getOriginExpr())
-    return;
+    return false;
 
   ProgramStateRef State = C.getState();
 
   if (const CheckFn *Callback = FreeingMemFnMap.lookup(Call)) {
-    (*Callback)(this, Call, C);
-    return;
+    (*Callback)(this, State, Call, C);
+    return true;
   }
 
   if (const CheckFn *Callback = AllocatingMemFnMap.lookup(Call)) {
-    (*Callback)(this, Call, C);
-    return;
+    State = MallocBindRetVal(C, Call, State, false);
+    (*Callback)(this, State, Call, C);
+    return true;
   }
 
   if (const CheckFn *Callback = ReallocatingMemFnMap.lookup(Call)) {
-    (*Callback)(this, Call, C);
-    return;
+    State = MallocBindRetVal(C, Call, State, false);
+    (*Callback)(this, State, Call, C);
+    return true;
   }
 
-  if (isStandardNewDelete(Call)) {
-    checkCXXNewOrCXXDelete(Call, C);
-    return;
+  if (isStandardNew(Call)) {
+    State = MallocBindRetVal(C, Call, State, false);
+    checkCXXNewOrCXXDelete(State, Call, C);
+    return true;
   }
 
-  checkOwnershipAttr(Call, C);
+  if (isStandardDelete(Call)) {
+    checkCXXNewOrCXXDelete(State, Call, C);
+    return true;
+  }
+
+  if (const CheckFn *Callback = AllocaMemFnMap.lookup(Call)) {
+    State = MallocBindRetVal(C, Call, State, true);
+    (*Callback)(this, State, Call, C);
+    return true;
+  }
+
+  if (isFreeingOwnershipAttrCall(Call)) {
+    checkOwnershipAttr(State, Call, C);
+    return true;
+  }
+
+  if (isAllocatingOwnershipAttrCall(Call)) {
+    State = MallocBindRetVal(C, Call, State, false);
+    checkOwnershipAttr(State, Call, C);
+    return true;
+  }
+
+  return false;
 }
 
 // Performs a 0-sized allocations check.
 ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
-    const CallEvent &Call, const unsigned IndexOfSizeArg, ProgramStateRef State,
-    std::optional<SVal> RetVal) {
+    CheckerContext &C, const CallEvent &Call, const unsigned IndexOfSizeArg,
+    ProgramStateRef State, std::optional<SVal> RetVal) {
   if (!State)
     return nullptr;
 
-  if (!RetVal)
-    RetVal = Call.getReturnValue();
-
   const Expr *Arg = nullptr;
 
   if (const CallExpr *CE = dyn_cast<CallExpr>(Call.getOriginExpr())) {
@@ -1545,6 +1622,9 @@ ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
     return nullptr;
   }
 
+  if (!RetVal)
+    RetVal = State->getSVal(Call.getOriginExpr(), C.getLocationContext());
+
   assert(Arg);
 
   auto DefArgVal =
@@ -1656,7 +1736,7 @@ MallocChecker::processNewAllocation(const CXXAllocatorCall &Call,
   }
 
   State = MallocUpdateRefState(C, NE, State, Family, Target);
-  State = ProcessZeroAllocCheck(Call, 0, State, Target);
+  State = ProcessZeroAllocCheck(C, Call, 0, State, Target);
   return State;
 }
 
@@ -1736,6 +1816,24 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
   return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, Family);
 }
 
+ProgramStateRef MallocChecker::MallocBindRetVal(CheckerContext &C,
+                                                const CallEvent &Call,
+                                                ProgramStateRef State,
+                                                bool isAlloca) const {
+  const Expr *CE = Call.getOriginExpr();
+
+  // We expect the allocation functions to return a pointer.
+  if (!Loc::isLocType(CE->getType()))
+    return nullptr;
+
+  unsigned Count = C.blockCount();
+  SValBuilder &SVB = C.getSValBuilder();
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  DefinedSVal RetVal = isAlloca ? SVB.getAllocaRegionVal(CE, LCtx, Count)
+                                : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count);
+  return State->BindExpr(CE, C.getLocationContext(), RetVal);
+}
+
 ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
                                             const CallEvent &Call,
                                             const Expr *SizeEx, SVal Init,
@@ -1814,20 +1912,12 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   const Expr *CE = Call.getOriginExpr();
 
   // We expect the malloc functions to return a pointer.
-  if (!Loc::isLocType(CE->getType()))
-    return nullptr;
+  // Should have been already checked.
+  assert(Loc::isLocType(CE->getType()) &&
+         "Allocation functions must return a pointer");
 
-  // Bind the return value to the symbolic value from the heap region.
-  // TODO: move use of this functions to an EvalCall callback, becasue
-  // BindExpr() should'nt be used elsewhere.
-  unsigned Count = C.blockCount();
-  SValBuilder &SVB = C.getSValBuilder();
   const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
-  DefinedSVal RetVal = ((Family.Kind == AF_Alloca)
-                            ? SVB.getAllocaRegionVal(CE, LCtx, Count)
-                            : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count)
-                                  .castAs<DefinedSVal>());
-  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SVal RetVal = State->getSVal(CE, C.getLocationContext());
 
   // Fill the region with the initialization value.
   State = State->bindDefaultInitial(RetVal, Init, LCtx);
@@ -1840,7 +1930,7 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
 
   // Set the region's extent.
   State = setDynamicExtent(State, RetVal.getAsRegion(),
-                           Size.castAs<DefinedOrUnknownSVal>(), SVB);
+                           Size.castAs<DefinedOrUnknownSVal>());
 
   return MallocUpdateRefState(C, CE, State, Family);
 }
@@ -1854,7 +1944,7 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
 
   // Get the return value.
   if (!RetVal)
-    RetVal = C.getSVal(E);
+    RetVal = State->getSVal(E, C.getLocationContext());
 
   // We expect the malloc functions to return a pointer.
   if (!RetVal->getAs<Loc>())
@@ -1862,20 +1952,15 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
 
   SymbolRef Sym = RetVal->getAsLocSymbol();
 
-  // This is a return value of a function that was not inlined, such as malloc()
-  // or new(). We've checked that in the caller. Therefore, it must be a symbol.
-  assert(Sym);
-  // FIXME: In theory this assertion should fail for `alloca()` calls (because
-  // `AllocaRegion`s are not symbolic); but in practice this does not happen.
-  // As the current code appears to work correctly, I'm not touching this issue
-  // now, but it would be good to investigate and clarify this.
-  // Also note that perhaps the special `AllocaRegion` should be replaced by
-  // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to enable
-  // proper tracking of memory allocated by `alloca()` -- and after that change
-  // this assertion would become valid again.
+  // NOTE: If this was an `alloca()` call, then `RetVal` holds an
+  // `AllocaRegion`, so `Sym` will be a nullpointer because `AllocaRegion`s do
+  // not have an associated symbol. However, this distinct region type means
+  // that we don't need to store anything about them in `RegionState`.
 
-  // Set the symbol's state to Allocated.
-  return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+  if (Sym)
+    return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+
+  return State;
 }
 
 ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
@@ -2214,6 +2299,14 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
   // that.
   assert(!RsBase || (RsBase && RsBase->getAllocationFamily() == Family));
 
+  // Assume that after memory is freed, it contains unknown values. This
+  // conforts languages standards, since reading from freed memory is considered
+  // UB and may result in arbitrary value.
+  State = State->invalidateRegions({location}, Call.getOriginExpr(),
+                                   C.blockCount(), C.getLocationContext(),
+                                   /*CausesPointerEscape=*/false,
+                                   /*InvalidatedSymbols=*/nullptr);
+
   // Normal free.
   if (Hold)
     return State->set<RegionState>(SymBase,
@@ -2781,6 +2874,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
     return stateMalloc;
   }
 
+  // Proccess as allocation of 0 bytes.
   if (PrtIsNull && SizeIsZero)
     return State;
 
@@ -2815,7 +2909,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
 
     // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
     SymbolRef FromPtr = arg0Val.getLocSymbolInBase();
-    SVal RetVal = C.getSVal(CE);
+    SVal RetVal = stateRealloc->getSVal(CE, C.getLocationContext());
     SymbolRef ToPtr = RetVal.getAsSymbol();
     assert(FromPtr && ToPtr &&
            "By this point, FreeMemAux and MallocMemAux should have checked "
@@ -3014,6 +3108,14 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   C.addTransition(state->set<RegionState>(RS), N);
 }
 
+void MallocChecker::checkPostCall(const CallEvent &Call,
+                                  CheckerContext &C) const {
+  if (const auto *PostFN = PostFnMap.lookup(Call)) {
+    (*PostFN)(this, C.getState(), Call, C);
+    return;
+  }
+}
+
 void MallocChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
 
@@ -3047,7 +3149,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
   // We need to handle getline pre-conditions here before the pointed region
   // gets invalidated by StreamChecker
   if (const auto *PreFN = PreFnMap.lookup(Call)) {
-    (*PreFN)(this, Call, C);
+    (*PreFN)(this, C.getState(), Call, C);
     return;
   }
 

clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

@@ -266,7 +266,6 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
     return;
 
   ASTContext &Ctx = C.getASTContext();
-  SValBuilder &SVB = C.getSValBuilder();
   ProgramStateRef State = C.getState();
   QualType TypeToCheck;
 
@@ -301,7 +300,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   if (VD) {
     State =
         setDynamicExtent(State, State->getRegion(VD, C.getLocationContext()),
-                         ArraySize.castAs<NonLoc>(), SVB);
+                         ArraySize.castAs<NonLoc>());
   }
 
   // Remember our assumptions!

clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp

@@ -120,7 +120,7 @@ DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
 }
 
 ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
-                                 DefinedOrUnknownSVal Size, SValBuilder &SVB) {
+                                 DefinedOrUnknownSVal Size) {
   MR = MR->StripCasts();
 
   if (Size.isUnknown())

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -817,8 +817,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
       if (Size.isUndef())
         Size = UnknownVal();
 
-      State = setDynamicExtent(State, MR, Size.castAs<DefinedOrUnknownSVal>(),
-                               svalBuilder);
+      State = setDynamicExtent(State, MR, Size.castAs<DefinedOrUnknownSVal>());
     } else {
       R = svalBuilder.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
     }

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

@@ -210,22 +210,24 @@ DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const Stmt *stmt,
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal
-SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
-                                      const LocationContext *LCtx,
-                                      unsigned VisitCount) {
+DefinedSVal SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+                                                  const LocationContext *LCtx,
+                                                  unsigned VisitCount) {
   QualType T = E->getType();
   return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
 }
 
-DefinedOrUnknownSVal
-SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
-                                      const LocationContext *LCtx,
-                                      QualType type, unsigned VisitCount) {
+DefinedSVal SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+                                                  const LocationContext *LCtx,
+                                                  QualType type,
+                                                  unsigned VisitCount) {
   assert(Loc::isLocType(type));
   assert(SymbolManager::canSymbolicate(type));
-  if (type->isNullPtrType())
-    return makeZeroVal(type);
+  if (type->isNullPtrType()) {
+    // makeZeroVal() returns UnknownVal only in case of FP number, which
+    // is not the case.
+    return makeZeroVal(type).castAs<DefinedSVal>();
+  }
 
   SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount);
   return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));

clang/test/Analysis/NewDelete-checker-test.cpp

@@ -37,10 +37,6 @@ extern "C" void *malloc(size_t);
 extern "C" void free (void* ptr);
 int *global;
 
-//------------------
-// check for leaks
-//------------------
-
 //----- Standard non-placement operators
 void testGlobalOpNew() {
   void *p = operator new(0);
@@ -67,19 +63,6 @@ void testGlobalNoThrowPlacementExprNewBeforeOverload() {
   int *p = new(std::nothrow) int;
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
-//----- Standard pointer placement operators
-void testGlobalPointerPlacementNew() {
-  int i;
-
-  void *p1 = operator new(0, &i); // no warn
-
-  void *p2 = operator new[](0, &i); // no warn
-
-  int *p3 = new(&i) int; // no warn
-
-  int *p4 = new(&i) int[0]; // no warn
-}
-
 //----- Other cases
 void testNewMemoryIsInHeap() {
   int *p = new int;

clang/test/Analysis/NewDelete-intersections.mm

@@ -3,13 +3,13 @@
 // RUN:  -analyzer-checker=core \
 // RUN:  -analyzer-checker=cplusplus.NewDelete
 
+// leak-no-diagnostics
+
 // RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
 // RUN:   -verify=leak \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
 
-// leak-no-diagnostics
-
 // RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
 // RUN:   -verify=mismatch \
 // RUN:   -analyzer-checker=core \

clang/test/Analysis/malloc-interprocedural.c

@@ -98,38 +98,3 @@ int uafAndCallsFooWithEmptyReturn(void) {
   fooWithEmptyReturn(12);
   return *x; // expected-warning {{Use of memory after it is freed}}
 }
-
-
-// If we inline any of the malloc-family functions, the checker shouldn't also
-// try to do additional modeling.
-char *strndup(const char *str, size_t n) {
-  if (!str)
-    return 0;
-  
-  // DO NOT FIX. This is to test that we are actually using the inlined
-  // behavior!
-  if (n < 5)
-    return 0;
-  
-  size_t length = strlen(str);
-  if (length < n)
-    n = length;
-  
-  char *result = malloc(n + 1);
-  memcpy(result, str, n);
-  result[n] = '\0';
-  return result;
-}
-
-void useStrndup(size_t n) {
-  if (n == 0) {
-    (void)strndup(0, 20); // no-warning
-    return;
-  } else if (n < 5) {
-    (void)strndup("hi there", n); // no-warning
-    return;
-  } else {
-    (void)strndup("hi there", n);
-    return; // expected-warning{{leak}}
-  }
-}