[clang] [analyzer] Refactor MallocChecker to use `BindExpr` in `evalCall` (PR #106081)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 15 20:59:57 PDT 2024
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/106081
>From 82e3d871766b132d0ce0b9e8e74371d8598d2431 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Tue, 6 Aug 2024 19:12:01 +0300
Subject: [PATCH 1/4] wip
---
.../Core/PathSensitive/DynamicExtent.h | 2 +-
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 355 +++++++++++-------
.../Checkers/VLASizeChecker.cpp | 3 +-
.../lib/StaticAnalyzer/Core/DynamicExtent.cpp | 2 +-
.../Core/ExprEngineCallAndReturn.cpp | 3 +-
.../test/Analysis/NewDelete-checker-test.cpp | 13 -
.../test/Analysis/NewDelete-intersections.mm | 6 +-
clang/test/Analysis/malloc-interprocedural.c | 35 --
8 files changed, 233 insertions(+), 186 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
index 50d5d254415af3..1a9bef06b15a44 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
+++ b/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.
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 3ddcb7e94ae5d6..1f524481049fa4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/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 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 isStandardNewDelete(cast<FunctionDecl>(Call.getDecl()));
+ 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 @@ class MallocChecker
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 @@ class MallocChecker
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 @@ class MallocChecker
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 @@ class MallocChecker
{{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 @@ class MallocChecker
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 @@ class MallocChecker
// 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 @@ class MallocChecker
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 @@ class MallocChecker
/// \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 @@ class MallocChecker
[[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 bindinded
+ [[nodiscard]] ProgramStateRef MallocBindRetval(CheckerContext &C,
+ const CallEvent &Call,
+ ProgramStateRef State,
+ bool isAlloca) const;
/// Models memory allocation.
///
@@ -1031,13 +1067,28 @@ class StopTrackingCallback final : public SymbolVisitor {
};
} // 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)
+ 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_New && Kind != OO_Array_New && Kind != OO_Delete &&
- Kind != OO_Array_Delete)
+ 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) : false;
+}
+
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) : false;
+}
+
+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,56 +1545,78 @@ 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;
+ }
+
+ 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;
}
- checkOwnershipAttr(Call, C);
+ 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())) {
Arg = CE->getArg(IndexOfSizeArg);
+ if (!RetVal)
+ RetVal = State->getSVal(CE, C.getLocationContext());
+ ;
} else if (const CXXNewExpr *NE =
dyn_cast<CXXNewExpr>(Call.getOriginExpr())) {
if (NE->isArray()) {
Arg = *NE->getArraySize();
+ if (!RetVal)
+ RetVal = State->getSVal(NE, C.getLocationContext());
+ ;
} else {
return State;
}
@@ -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,25 @@ 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)
+ .castAs<DefinedSVal>());
+ return State->BindExpr(CE, C.getLocationContext(), RetVal);
+}
+
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
const CallEvent &Call,
const Expr *SizeEx, SVal Init,
@@ -1814,20 +1913,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 +1931,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 +1945,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,11 +1953,8 @@ 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.
+ // FIXME: Following if fails for `alloca()` calls (because
+ // `AllocaRegion`s are not symbolic);
// 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
@@ -1874,8 +1962,10 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
// proper tracking of memory allocated by `alloca()` -- and after that change
// this assertion would become valid again.
- // 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));
+ else
+ return State;
}
ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
@@ -2781,6 +2871,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
return stateMalloc;
}
+ // Proccess as allocation of 0 bytes.
if (PrtIsNull && SizeIsZero)
return State;
@@ -2815,7 +2906,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 +3105,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 +3146,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;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 87d255eeffc177..8d17ba5d690b90 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/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!
diff --git a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
index 6cf06413b5372c..f0c6501758b4d6 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ b/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())
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 9d3e4fc944fb7b..2ca24d0c5ab22b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/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);
}
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 1100b49e84d3ac..ac8fcdbd0ef1e4 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -67,19 +67,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;
diff --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm
index 9ac471600e8b9b..968ef9f6e2b56f 100644
--- a/clang/test/Analysis/NewDelete-intersections.mm
+++ b/clang/test/Analysis/NewDelete-intersections.mm
@@ -8,8 +8,6 @@
// 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 \
@@ -60,14 +58,14 @@ void testFreeOpNew() {
void *p = operator new(0);
free(p);
// mismatch-warning at -1{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'free()'}}
-}
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
void testFreeNewExpr() {
int *p = new int;
free(p);
// mismatch-warning at -1{{Memory allocated by 'new' should be deallocated by 'delete', not 'free()'}}
free(p);
-}
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
void testObjcFreeNewed() {
int *p = new int;
diff --git a/clang/test/Analysis/malloc-interprocedural.c b/clang/test/Analysis/malloc-interprocedural.c
index ae7a4626288e68..5e5232af7b46e8 100644
--- a/clang/test/Analysis/malloc-interprocedural.c
+++ b/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}}
- }
-}
>From d0ab7314013158b0183861d0faaae001f246c87f Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Wed, 28 Aug 2024 11:09:05 +0300
Subject: [PATCH 2/4] fix style
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 52 +++++++++----------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 1f524481049fa4..092c1c8bc0ca1d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -585,8 +585,8 @@ class MallocChecker
/// \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 bindinded
- [[nodiscard]] ProgramStateRef MallocBindRetval(CheckerContext &C,
+ /// \returns The ProgramState with returnValue bound
+ [[nodiscard]] ProgramStateRef MallocBindRetVal(CheckerContext &C,
const CallEvent &Call,
ProgramStateRef State,
bool isAlloca) const;
@@ -1106,7 +1106,7 @@ static bool isStandardDelete(const FunctionDecl *FD) {
bool MallocChecker::isFreeingOwnershipAttrCall(const CallEvent &Call) {
const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
- return Func ? isFreeingOwnershipAttrCall(Func) : false;
+ return Func && isFreeingOwnershipAttrCall(Func);
}
bool MallocChecker::isFreeingOwnershipAttrCall(const FunctionDecl *Func) {
@@ -1130,7 +1130,7 @@ bool MallocChecker::isFreeingCall(const CallEvent &Call) const {
bool MallocChecker::isAllocatingOwnershipAttrCall(const CallEvent &Call) {
const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
- return Func ? isAllocatingOwnershipAttrCall(Func) : false;
+ return Func && isAllocatingOwnershipAttrCall(Func);
}
bool MallocChecker::isAllocatingOwnershipAttrCall(const FunctionDecl *Func) {
@@ -1557,27 +1557,30 @@ bool MallocChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
}
if (const CheckFn *Callback = AllocatingMemFnMap.lookup(Call)) {
- State = MallocBindRetval(C, Call, State, false);
+ State = MallocBindRetVal(C, Call, State, false);
(*Callback)(this, State, Call, C);
return true;
}
if (const CheckFn *Callback = ReallocatingMemFnMap.lookup(Call)) {
- State = MallocBindRetval(C, Call, State, false);
+ State = MallocBindRetVal(C, Call, State, false);
(*Callback)(this, State, Call, C);
return true;
}
- if (isStandardNewDelete(Call)) {
- if (isStandardNew(Call))
- State = MallocBindRetval(C, Call, State, false);
+ if (isStandardNew(Call)) {
+ State = MallocBindRetVal(C, Call, State, false);
+ checkCXXNewOrCXXDelete(State, Call, C);
+ return true;
+ }
+ if (isStandardDelete(Call)) {
checkCXXNewOrCXXDelete(State, Call, C);
return true;
}
if (const CheckFn *Callback = AllocaMemFnMap.lookup(Call)) {
- State = MallocBindRetval(C, Call, State, true);
+ State = MallocBindRetVal(C, Call, State, true);
(*Callback)(this, State, Call, C);
return true;
}
@@ -1588,7 +1591,7 @@ bool MallocChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
}
if (isAllocatingOwnershipAttrCall(Call)) {
- State = MallocBindRetval(C, Call, State, false);
+ State = MallocBindRetVal(C, Call, State, false);
checkOwnershipAttr(State, Call, C);
return true;
}
@@ -1607,16 +1610,10 @@ ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
if (const CallExpr *CE = dyn_cast<CallExpr>(Call.getOriginExpr())) {
Arg = CE->getArg(IndexOfSizeArg);
- if (!RetVal)
- RetVal = State->getSVal(CE, C.getLocationContext());
- ;
} else if (const CXXNewExpr *NE =
dyn_cast<CXXNewExpr>(Call.getOriginExpr())) {
if (NE->isArray()) {
Arg = *NE->getArraySize();
- if (!RetVal)
- RetVal = State->getSVal(NE, C.getLocationContext());
- ;
} else {
return State;
}
@@ -1625,6 +1622,9 @@ ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
return nullptr;
}
+ if (!RetVal)
+ RetVal = State->getSVal(Call.getOriginExpr(), C.getLocationContext());
+
assert(Arg);
auto DefArgVal =
@@ -1816,7 +1816,7 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, Family);
}
-ProgramStateRef MallocChecker::MallocBindRetval(CheckerContext &C,
+ProgramStateRef MallocChecker::MallocBindRetVal(CheckerContext &C,
const CallEvent &Call,
ProgramStateRef State,
bool isAlloca) const {
@@ -1953,19 +1953,15 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
SymbolRef Sym = RetVal->getAsLocSymbol();
- // FIXME: Following if fails for `alloca()` calls (because
- // `AllocaRegion`s are not symbolic);
- // 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`.
if (Sym)
return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
- else
- return State;
+
+ return State;
}
ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
>From ea0536963809bc3b1911d06e05c8af1197ac26fc Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Wed, 28 Aug 2024 11:38:48 +0300
Subject: [PATCH 3/4] invalidate memory region after
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 8 ++++++++
clang/test/Analysis/NewDelete-checker-test.cpp | 4 ----
clang/test/Analysis/NewDelete-intersections.mm | 6 ++++--
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 092c1c8bc0ca1d..b53827494f2a6c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2300,6 +2300,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,
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index ac8fcdbd0ef1e4..21b4cf817b5df6 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/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);
diff --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm
index 968ef9f6e2b56f..e897f48b839620 100644
--- a/clang/test/Analysis/NewDelete-intersections.mm
+++ b/clang/test/Analysis/NewDelete-intersections.mm
@@ -3,6 +3,8 @@
// 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 \
@@ -58,14 +60,14 @@ void testFreeOpNew() {
void *p = operator new(0);
free(p);
// mismatch-warning at -1{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'free()'}}
-} // leak-warning{{Potential leak of memory pointed to by 'p'}}
+}
void testFreeNewExpr() {
int *p = new int;
free(p);
// mismatch-warning at -1{{Memory allocated by 'new' should be deallocated by 'delete', not 'free()'}}
free(p);
-} // leak-warning{{Potential leak of memory pointed to by 'p'}}
+}
void testObjcFreeNewed() {
int *p = new int;
>From 7910b7c4a831c50028c5e514d64ef503f567900e Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Wed, 28 Aug 2024 11:55:55 +0300
Subject: [PATCH 4/4] make getConjuredHeapSymbolVal return DefinedSVal
---
.../Core/PathSensitive/SValBuilder.h | 12 +++++-----
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 5 ++---
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 22 ++++++++++---------
3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index a560f274c43ccd..6eedaf0544559b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -215,17 +215,17 @@ class SValBuilder {
/// 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.
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index b53827494f2a6c..596a885f886e7e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1829,9 +1829,8 @@ ProgramStateRef MallocChecker::MallocBindRetVal(CheckerContext &C,
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)
- .castAs<DefinedSVal>());
+ DefinedSVal RetVal = isAlloca ? SVB.getAllocaRegionVal(CE, LCtx, Count)
+ : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count);
return State->BindExpr(CE, C.getLocationContext(), RetVal);
}
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index eb9cde5c8918fc..7eca0579143f44 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/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));
More information about the cfe-commits
mailing list