[clang] [analyzer] Conversion to CheckerFamily: MallocChecker (PR #147080)
DonĂ¡t Nagy via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 8 03:28:45 PDT 2025
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/147080
>From c0e669a4f31702a871fce4c8c3805b322c331afd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 2 Jul 2025 15:09:42 +0200
Subject: [PATCH 01/10] [analyzer] Connversion to CheckerFamily: MallocChecker
This commit converts MallocChecker to the new checker family framework
that was introduced in the recent commit
6833076a5d9f5719539a24e900037da5a3979289 -- and gets rid of some
awkward unintended interactions between the checker frontends.
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 414 ++++++++----------
clang/test/Analysis/new.cpp | 40 +-
.../Analysis/test-member-invalidation.cpp | 47 ++
3 files changed, 227 insertions(+), 274 deletions(-)
create mode 100644 clang/test/Analysis/test-member-invalidation.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a33e61fabc2c1..9e7540eecc8ee 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -333,11 +333,55 @@ template <typename T> static bool isStandardNewDelete(const T &FD) {
return isStandardDelete(FD) || isStandardNew(FD);
}
+namespace {
+
//===----------------------------------------------------------------------===//
-// Definition of the MallocChecker class.
+// Utility classes that provide access to the bug types and can model that some
+// of the bug types are shared by multiple checker frontends.
//===----------------------------------------------------------------------===//
-namespace {
+#define BUGTYPE_PROVIDER(NAME, DEF) \
+ struct NAME : virtual public CheckerFrontend { \
+ BugType NAME##Bug{this, DEF, categories::MemoryError}; \
+ };
+
+BUGTYPE_PROVIDER(DoubleFree, "Double free")
+// TODO: Remove DoubleDelete as a separate bug type and when it would be
+// emitted, emit DoubleFree reports instead. (Note that DoubleFree is already
+// used for all allocation families, not just malloc/free.)
+BUGTYPE_PROVIDER(DoubleDelete, "Double delete")
+
+struct Leak : virtual public CheckerFrontend {
+ // Leaks should not be reported if they are post-dominated by a sink:
+ // (1) Sinks are higher importance bugs.
+ // (2) NoReturnFunctionChecker uses sink nodes to represent paths ending
+ // with __noreturn functions such as assert() or exit(). We choose not
+ // to report leaks on such paths.
+ BugType LeakBug{this, "Memory leak", categories::MemoryError,
+ /*SuppressOnSink=*/true};
+};
+
+BUGTYPE_PROVIDER(UseFree, "Use-after-free")
+BUGTYPE_PROVIDER(BadFree, "Bad free")
+BUGTYPE_PROVIDER(FreeAlloca, "Free 'alloca()'")
+BUGTYPE_PROVIDER(MismatchedDealloc, "Bad deallocator")
+BUGTYPE_PROVIDER(OffsetFree, "Offset free")
+BUGTYPE_PROVIDER(UseZeroAllocated, "Use of zero allocated")
+
+template <typename... BT_PROVIDERS>
+struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... {
+ template <typename T> const T *getAs() const {
+ if constexpr (std::is_same_v<T, CheckerFrontend>)
+ return static_cast<const T *>(this);
+ if constexpr ((std::is_same_v<T, BT_PROVIDERS> || ...))
+ return static_cast<const T *>(this);
+ return nullptr;
+ }
+};
+
+//===----------------------------------------------------------------------===//
+// Definition of the MallocChecker class.
+//===----------------------------------------------------------------------===//
class MallocChecker
: public Checker<check::DeadSymbols, check::PointerEscape,
@@ -355,26 +399,29 @@ class MallocChecker
bool ShouldRegisterNoOwnershipChangeVisitor = false;
- /// Many checkers are essentially built into this one, so enabling them will
- /// make MallocChecker perform additional modeling and reporting.
- enum CheckKind {
- /// When a subchecker is enabled but MallocChecker isn't, model memory
- /// management but do not emit warnings emitted with MallocChecker only
- /// enabled.
- CK_MallocChecker,
- CK_NewDeleteChecker,
- CK_NewDeleteLeaksChecker,
- CK_MismatchedDeallocatorChecker,
- CK_InnerPointerChecker,
- CK_TaintedAllocChecker,
- CK_NumCheckKinds
- };
+ // This checker family implements many bug types and frontends, and several
+ // bug types are shared between multiple frontends, so most of the frontends
+ // are declared with the helper class DynMemFrontend.
+ // FIXME: There is no clear reason for separating NewDelete vs NewDeleteLeaks
+ // while e.g. MallocChecker covers both non-leak and leak bugs together. It
+ // would be nice to redraw the boundaries between the frontends in a more
+ // logical way.
+ DynMemFrontend<DoubleFree, Leak, UseFree, BadFree, FreeAlloca, OffsetFree,
+ UseZeroAllocated>
+ MallocChecker;
+ DynMemFrontend<DoubleFree, DoubleDelete, UseFree, BadFree, OffsetFree,
+ UseZeroAllocated>
+ NewDeleteChecker;
+ DynMemFrontend<Leak> NewDeleteLeaksChecker;
+ DynMemFrontend<FreeAlloca, MismatchedDealloc> MismatchedDeallocatorChecker;
+ DynMemFrontend<UseFree> InnerPointerChecker;
+ // This last frontend is associated with a single bug type which is not used
+ // elsewhere and has a different bug category, so it's declared separately.
+ CheckerFrontendWithBugType TaintedAllocChecker{"Tainted Memory Allocation",
+ categories::TaintedData};
using LeakInfo = std::pair<const ExplodedNode *, const MemRegion *>;
- bool ChecksEnabled[CK_NumCheckKinds] = {false};
- CheckerNameRef CheckNames[CK_NumCheckKinds];
-
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
@@ -402,16 +449,19 @@ class MallocChecker
const char *NL, const char *Sep) const override;
private:
- mutable std::unique_ptr<BugType> BT_DoubleFree[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_DoubleDelete;
- mutable std::unique_ptr<BugType> BT_Leak[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_UseFree[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_BadFree[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
- mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_TaintedAlloc;
+ /// Helper method to handle the cases where there is no associated frontend
+ /// (just exit early) or the associated frontend is disabled (sink the
+ /// execution path and and then exit early). Intended to be called as
+ /// if (handleNullOrDisabled(Frontend, C))
+ /// return;
+ static bool handleNullOrDisabled(const CheckerFrontend *F,
+ CheckerContext &C) {
+ if (F && F->isEnabled())
+ return false;
+ if (F)
+ C.addSink();
+ return true;
+ }
#define CHECK_FN(NAME) \
void NAME(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) \
@@ -439,7 +489,7 @@ class MallocChecker
CheckerContext &C, bool ShouldFreeOnFail) const;
using CheckFn =
- std::function<void(const MallocChecker *, ProgramStateRef State,
+ std::function<void(const class MallocChecker *, ProgramStateRef State,
const CallEvent &Call, CheckerContext &C)>;
const CallDescriptionMap<CheckFn> PreFnMap{
@@ -773,14 +823,16 @@ class MallocChecker
void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext &C) const;
///@{
- /// Tells if a given family/call/symbol is tracked by the current checker.
- /// Sets CheckKind to the kind of the checker responsible for this
- /// family/call/symbol.
- std::optional<CheckKind> getCheckIfTracked(AllocationFamily Family,
- bool IsALeakCheck = false) const;
-
- std::optional<CheckKind> getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
- bool IsALeakCheck = false) const;
+ /// Returns a pointer to the checker frontend corresponding to the given
+ /// family or symbol. The template argument T may be either CheckerFamily or
+ /// a BUGTYPE_PROVIDER class; in the latter case the query is restricted to
+ /// frontends that descend from that PROVIDER class (i.e. can emit that bug
+ /// type). Note that this may return a frontend which is disabled.
+ template <class T>
+ const T *getRelevantFrontendAs(AllocationFamily Family) const;
+
+ template <class T>
+ const T *getRelevantFrontendAs(CheckerContext &C, SymbolRef Sym) const;
///@}
static bool SummarizeValue(raw_ostream &os, SVal V);
static bool SummarizeRegion(ProgramStateRef State, raw_ostream &os,
@@ -1558,7 +1610,7 @@ void MallocChecker::checkOwnershipAttr(ProgramStateRef State,
if (!FD)
return;
if (ShouldIncludeOwnershipAnnotatedFunctions ||
- ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
+ MismatchedDeallocatorChecker.isEnabled()) {
// Check all the attributes, if there are any.
// There can be multiple of these attributes.
if (FD->hasAttrs())
@@ -1883,11 +1935,8 @@ void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
llvm::ArrayRef<SymbolRef> TaintedSyms,
AllocationFamily Family) const {
if (ExplodedNode *N = C.generateNonFatalErrorNode(State, this)) {
- if (!BT_TaintedAlloc)
- BT_TaintedAlloc.reset(new BugType(CheckNames[CK_TaintedAllocChecker],
- "Tainted Memory Allocation",
- categories::TaintedData));
- auto R = std::make_unique<PathSensitiveBugReport>(*BT_TaintedAlloc, Msg, N);
+ auto R =
+ std::make_unique<PathSensitiveBugReport>(TaintedAllocChecker, Msg, N);
for (const auto *TaintedSym : TaintedSyms) {
R->markInteresting(TaintedSym);
}
@@ -1898,7 +1947,7 @@ void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
void MallocChecker::checkTaintedness(CheckerContext &C, const CallEvent &Call,
const SVal SizeSVal, ProgramStateRef State,
AllocationFamily Family) const {
- if (!ChecksEnabled[CK_TaintedAllocChecker])
+ if (!TaintedAllocChecker.isEnabled())
return;
std::vector<SymbolRef> TaintedSyms =
taint::getTaintedSymbols(State, SizeSVal);
@@ -2348,53 +2397,47 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
RefState::getReleased(Family, ParentExpr));
}
-std::optional<MallocChecker::CheckKind>
-MallocChecker::getCheckIfTracked(AllocationFamily Family,
- bool IsALeakCheck) const {
+template <class T>
+const T *MallocChecker::getRelevantFrontendAs(AllocationFamily Family) const {
switch (Family.Kind) {
case AF_Malloc:
case AF_Alloca:
case AF_Custom:
- case AF_IfNameIndex: {
- if (ChecksEnabled[CK_MallocChecker])
- return CK_MallocChecker;
- return std::nullopt;
- }
+ case AF_IfNameIndex:
+ return MallocChecker.getAs<T>();
case AF_CXXNew:
case AF_CXXNewArray: {
- if (IsALeakCheck) {
- if (ChecksEnabled[CK_NewDeleteLeaksChecker])
- return CK_NewDeleteLeaksChecker;
+ const T *ND = NewDeleteChecker.getAs<T>();
+ const T *NDL = NewDeleteLeaksChecker.getAs<T>();
+ // Bugs corresponding to C++ new/delete allocations are split between these
+ // two frontends.
+ if constexpr (std::is_same_v<T, CheckerFrontend>) {
+ assert(ND && NDL && "Casting to CheckerFrontend always succeeds");
+ // Prefer NewDelete unless it's disabled and NewDeleteLeaks is enabled.
+ return (!ND->isEnabled() && NDL->isEnabled()) ? NDL : ND;
}
- else {
- if (ChecksEnabled[CK_NewDeleteChecker])
- return CK_NewDeleteChecker;
- }
- return std::nullopt;
- }
- case AF_InnerBuffer: {
- if (ChecksEnabled[CK_InnerPointerChecker])
- return CK_InnerPointerChecker;
- return std::nullopt;
+ assert(!(ND && NDL) &&
+ "NewDelete and NewDeleteLeaks must not share a bug type");
+ return ND ? ND : NDL;
}
- case AF_None: {
+ case AF_InnerBuffer:
+ return InnerPointerChecker.getAs<T>();
+ case AF_None:
assert(false && "no family");
- return std::nullopt;
- }
+ return nullptr;
}
assert(false && "unhandled family");
- return std::nullopt;
+ return nullptr;
}
-
-std::optional<MallocChecker::CheckKind>
-MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
- bool IsALeakCheck) const {
+template <class T>
+const T *MallocChecker::getRelevantFrontendAs(CheckerContext &C,
+ SymbolRef Sym) const {
if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym))
- return CK_MallocChecker;
+ return MallocChecker.getAs<T>();
const RefState *RS = C.getState()->get<RegionState>(Sym);
assert(RS);
- return getCheckIfTracked(RS->getAllocationFamily(), IsALeakCheck);
+ return getRelevantFrontendAs<T>(RS->getAllocationFamily());
}
bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
@@ -2490,21 +2533,11 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
SourceRange Range,
const Expr *DeallocExpr,
AllocationFamily Family) const {
-
- if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
- if (!CheckKind)
+ const BadFree *Frontend = getRelevantFrontendAs<BadFree>(Family);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_BadFree[*CheckKind])
- BT_BadFree[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Bad free", categories::MemoryError));
-
SmallString<100> buf;
llvm::raw_svector_ostream os(buf);
@@ -2526,7 +2559,7 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
printExpectedAllocName(os, Family);
- auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind],
+ auto R = std::make_unique<PathSensitiveBugReport>(Frontend->BadFreeBug,
os.str(), N);
R->markInteresting(MR);
R->addRange(Range);
@@ -2536,25 +2569,20 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
void MallocChecker::HandleFreeAlloca(CheckerContext &C, SVal ArgVal,
SourceRange Range) const {
+ const FreeAlloca *Frontend;
- std::optional<MallocChecker::CheckKind> CheckKind;
-
- if (ChecksEnabled[CK_MallocChecker])
- CheckKind = CK_MallocChecker;
- else if (ChecksEnabled[CK_MismatchedDeallocatorChecker])
- CheckKind = CK_MismatchedDeallocatorChecker;
+ if (MallocChecker.isEnabled())
+ Frontend = &MallocChecker;
+ else if (MismatchedDeallocatorChecker.isEnabled())
+ Frontend = &MismatchedDeallocatorChecker;
else {
C.addSink();
return;
}
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_FreeAlloca[*CheckKind])
- BT_FreeAlloca[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Free 'alloca()'", categories::MemoryError));
-
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_FreeAlloca[*CheckKind],
+ Frontend->FreeAllocaBug,
"Memory allocated by 'alloca()' should not be deallocated", N);
R->markInteresting(ArgVal.getAsRegion());
R->addRange(Range);
@@ -2567,18 +2595,12 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C,
const Expr *DeallocExpr,
const RefState *RS, SymbolRef Sym,
bool OwnershipTransferred) const {
-
- if (!ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
+ if (!MismatchedDeallocatorChecker.isEnabled()) {
C.addSink();
return;
}
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_MismatchedDealloc)
- BT_MismatchedDealloc.reset(
- new BugType(CheckNames[CK_MismatchedDeallocatorChecker],
- "Bad deallocator", categories::MemoryError));
-
SmallString<100> buf;
llvm::raw_svector_ostream os(buf);
@@ -2612,8 +2634,8 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C,
printOwnershipTakesList(os, C, DeallocExpr);
}
- auto R = std::make_unique<PathSensitiveBugReport>(*BT_MismatchedDealloc,
- os.str(), N);
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ MismatchedDeallocatorChecker.MismatchedDeallocBug, os.str(), N);
R->markInteresting(Sym);
R->addRange(Range);
R->addVisitor<MallocBugVisitor>(Sym);
@@ -2625,24 +2647,14 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
SourceRange Range, const Expr *DeallocExpr,
AllocationFamily Family,
const Expr *AllocExpr) const {
-
- if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
- if (!CheckKind)
+ const OffsetFree *Frontend = getRelevantFrontendAs<OffsetFree>(Family);
+ if (handleNullOrDisabled(Frontend, C))
return;
ExplodedNode *N = C.generateErrorNode();
if (!N)
return;
- if (!BT_OffsetFree[*CheckKind])
- BT_OffsetFree[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Offset free", categories::MemoryError));
-
SmallString<100> buf;
llvm::raw_svector_ostream os(buf);
SmallString<20> AllocNameBuf;
@@ -2672,7 +2684,7 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
else
os << "allocated memory";
- auto R = std::make_unique<PathSensitiveBugReport>(*BT_OffsetFree[*CheckKind],
+ auto R = std::make_unique<PathSensitiveBugReport>(Frontend->OffsetFreeBug,
os.str(), N);
R->markInteresting(MR->getBaseRegion());
R->addRange(Range);
@@ -2681,27 +2693,16 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const {
-
- if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker] &&
- !ChecksEnabled[CK_InnerPointerChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
- if (!CheckKind)
+ const UseFree *Frontend = getRelevantFrontendAs<UseFree>(C, Sym);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_UseFree[*CheckKind])
- BT_UseFree[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Use-after-free", categories::MemoryError));
-
AllocationFamily AF =
C.getState()->get<RegionState>(Sym)->getAllocationFamily();
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_UseFree[*CheckKind],
+ Frontend->UseFreeBug,
AF.Kind == AF_InnerBuffer
? "Inner pointer of container used after re/deallocation"
: "Use of memory after it is freed",
@@ -2721,23 +2722,13 @@ void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range,
void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range,
bool Released, SymbolRef Sym,
SymbolRef PrevSym) const {
-
- if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
- if (!CheckKind)
+ const DoubleFree *Frontend = getRelevantFrontendAs<DoubleFree>(C, Sym);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_DoubleFree[*CheckKind])
- BT_DoubleFree[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Double free", categories::MemoryError));
-
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_DoubleFree[*CheckKind],
+ Frontend->DoubleFreeBug,
(Released ? "Attempt to free released memory"
: "Attempt to free non-owned memory"),
N);
@@ -2751,24 +2742,14 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range,
}
void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
-
- if (!ChecksEnabled[CK_NewDeleteChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
- if (!CheckKind)
+ const DoubleDelete *Frontend = getRelevantFrontendAs<DoubleDelete>(C, Sym);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_DoubleDelete)
- BT_DoubleDelete.reset(new BugType(CheckNames[CK_NewDeleteChecker],
- "Double delete",
- categories::MemoryError));
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_DoubleDelete, "Attempt to delete released memory", N);
+ Frontend->DoubleDeleteBug, "Attempt to delete released memory", N);
R->markInteresting(Sym);
R->addVisitor<MallocBugVisitor>(Sym);
@@ -2778,26 +2759,15 @@ void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const {
-
- if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
-
- if (!CheckKind)
+ const UseZeroAllocated *Frontend =
+ getRelevantFrontendAs<UseZeroAllocated>(C, Sym);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_UseZerroAllocated[*CheckKind])
- BT_UseZerroAllocated[*CheckKind].reset(
- new BugType(CheckNames[*CheckKind], "Use of zero allocated",
- categories::MemoryError));
-
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_UseZerroAllocated[*CheckKind],
- "Use of memory allocated with size zero", N);
+ Frontend->UseZeroAllocatedBug, "Use of memory allocated with size zero",
+ N);
R->addRange(Range);
if (Sym) {
@@ -2812,20 +2782,11 @@ void MallocChecker::HandleFunctionPtrFree(CheckerContext &C, SVal ArgVal,
SourceRange Range,
const Expr *FreeExpr,
AllocationFamily Family) const {
- if (!ChecksEnabled[CK_MallocChecker]) {
- C.addSink();
- return;
- }
-
- std::optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
- if (!CheckKind)
+ const BadFree *Frontend = getRelevantFrontendAs<BadFree>(Family);
+ if (handleNullOrDisabled(Frontend, C))
return;
if (ExplodedNode *N = C.generateErrorNode()) {
- if (!BT_BadFree[*CheckKind])
- BT_BadFree[*CheckKind].reset(new BugType(
- CheckNames[*CheckKind], "Bad free", categories::MemoryError));
-
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
@@ -2839,7 +2800,7 @@ void MallocChecker::HandleFunctionPtrFree(CheckerContext &C, SVal ArgVal,
Os << " is a function pointer";
- auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind],
+ auto R = std::make_unique<PathSensitiveBugReport>(Frontend->BadFreeBug,
Os.str(), N);
R->markInteresting(MR);
R->addRange(Range);
@@ -3016,10 +2977,7 @@ MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
CheckerContext &C) const {
-
- if (!ChecksEnabled[CK_MallocChecker] &&
- !ChecksEnabled[CK_NewDeleteLeaksChecker])
- return;
+ assert(N && "HandleLeak is only called with a non-null node");
const RefState *RS = C.getState()->get<RegionState>(Sym);
assert(RS && "cannot leak an untracked symbol");
@@ -3028,24 +2986,10 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
if (Family.Kind == AF_Alloca)
return;
- std::optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(Family, true);
-
- if (!CheckKind)
+ const Leak *Frontend = getRelevantFrontendAs<Leak>(Family);
+ if (handleNullOrDisabled(Frontend, C))
return;
- assert(N);
- if (!BT_Leak[*CheckKind]) {
- // Leaks should not be reported if they are post-dominated by a sink:
- // (1) Sinks are higher importance bugs.
- // (2) NoReturnFunctionChecker uses sink nodes to represent paths ending
- // with __noreturn functions such as assert() or exit(). We choose not
- // to report leaks on such paths.
- BT_Leak[*CheckKind].reset(new BugType(CheckNames[*CheckKind], "Memory leak",
- categories::MemoryError,
- /*SuppressOnSink=*/true));
- }
-
// Most bug reports are cached at the location where they occurred.
// With leaks, we want to unique them by the location where they were
// allocated, and only report a single path.
@@ -3070,7 +3014,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
}
auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_Leak[*CheckKind], os.str(), N, LocUsedForUniqueing,
+ Frontend->LeakBug, os.str(), N, LocUsedForUniqueing,
AllocNode->getLocationContext()->getDecl());
R->markInteresting(Sym);
R->addVisitor<MallocBugVisitor>(Sym, true);
@@ -3150,7 +3094,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
if (const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call)) {
const CXXDeleteExpr *DE = DC->getOriginExpr();
- if (!ChecksEnabled[CK_NewDeleteChecker])
+ if (!NewDeleteChecker.isEnabled())
if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
checkUseAfterFree(Sym, C, DE->getArgument());
@@ -3187,7 +3131,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
if (!FD)
return;
- if (ChecksEnabled[CK_MallocChecker] && isFreeingCall(Call))
+ if (MallocChecker.isEnabled() && isFreeingCall(Call))
return;
}
@@ -3902,16 +3846,15 @@ void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State,
for (auto [Sym, Data] : RS) {
const RefState *RefS = State->get<RegionState>(Sym);
AllocationFamily Family = RefS->getAllocationFamily();
- std::optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(Family);
- if (!CheckKind)
- CheckKind = getCheckIfTracked(Family, true);
+
+ const CheckerFrontend *Frontend =
+ getRelevantFrontendAs<CheckerFrontend>(Family);
Sym->dumpToStream(Out);
Out << " : ";
Data.dump(Out);
- if (CheckKind)
- Out << " (" << CheckNames[*CheckKind] << ")";
+ if (Frontend)
+ Out << " (" << Frontend->getName() << ")";
Out << NL;
}
}
@@ -3933,36 +3876,31 @@ markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
// Intended to be used in InnerPointerChecker to register the part of
// MallocChecker connected to it.
-void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
- MallocChecker *checker = mgr.getChecker<MallocChecker>();
- checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
- checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
- mgr.getCurrentCheckerName();
+void ento::registerInnerPointerCheckerAux(CheckerManager &Mgr) {
+ Mgr.getChecker<MallocChecker>()->InnerPointerChecker.enable(Mgr);
}
-void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
- auto *checker = mgr.registerChecker<MallocChecker>();
- checker->ShouldIncludeOwnershipAnnotatedFunctions =
- mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "Optimistic");
- checker->ShouldRegisterNoOwnershipChangeVisitor =
- mgr.getAnalyzerOptions().getCheckerBooleanOption(
- checker, "AddNoOwnershipChangeNotes");
+void ento::registerDynamicMemoryModeling(CheckerManager &Mgr) {
+ auto *Chk = Mgr.registerChecker<MallocChecker>();
+ Chk->ShouldIncludeOwnershipAnnotatedFunctions =
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Optimistic");
+ Chk->ShouldRegisterNoOwnershipChangeVisitor =
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Chk, "AddNoOwnershipChangeNotes");
}
bool ento::shouldRegisterDynamicMemoryModeling(const CheckerManager &mgr) {
return true;
}
-#define REGISTER_CHECKER(name) \
- void ento::register##name(CheckerManager &mgr) { \
- MallocChecker *checker = mgr.getChecker<MallocChecker>(); \
- checker->ChecksEnabled[MallocChecker::CK_##name] = true; \
- checker->CheckNames[MallocChecker::CK_##name] = \
- mgr.getCurrentCheckerName(); \
+#define REGISTER_CHECKER(NAME) \
+ void ento::register##NAME(CheckerManager &Mgr) { \
+ Mgr.getChecker<MallocChecker>()->NAME.enable(Mgr); \
} \
\
- bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
+ bool ento::shouldRegister##NAME(const CheckerManager &) { return true; }
+// TODO: NewDelete and NewDeleteLeaks shouldn't be registered when not in C++.
REGISTER_CHECKER(MallocChecker)
REGISTER_CHECKER(NewDeleteChecker)
REGISTER_CHECKER(NewDeleteLeaksChecker)
diff --git a/clang/test/Analysis/new.cpp b/clang/test/Analysis/new.cpp
index 3542b5c594b50..8439a4e55d812 100644
--- a/clang/test/Analysis/new.cpp
+++ b/clang/test/Analysis/new.cpp
@@ -327,39 +327,7 @@ void testArrayDestr() {
clang_analyzer_warnIfReached(); // no-warning
}
-// Invalidate Region even in case of default destructor
-class InvalidateDestTest {
-public:
- int x;
- int *y;
- ~InvalidateDestTest();
-};
-
-int test_member_invalidation() {
-
- //test invalidation of member variable
- InvalidateDestTest *test = new InvalidateDestTest();
- test->x = 5;
- int *k = &(test->x);
- clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
- delete test;
- clang_analyzer_eval(*k == 5); // expected-warning{{UNKNOWN}}
-
- //test invalidation of member pointer
- int localVar = 5;
- test = new InvalidateDestTest();
- test->y = &localVar;
- delete test;
- clang_analyzer_eval(localVar == 5); // expected-warning{{UNKNOWN}}
-
- // Test aray elements are invalidated.
- int Var1 = 5;
- int Var2 = 5;
- InvalidateDestTest *a = new InvalidateDestTest[2];
- a[0].y = &Var1;
- a[1].y = &Var2;
- delete[] a;
- clang_analyzer_eval(Var1 == 5); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(Var2 == 5); // expected-warning{{UNKNOWN}}
- return 0;
-}
+// See also test-member-invalidation.cpp which validates that calling an
+// unknown destructor invalidates the members of an object. This behavior
+// cannot be tested in this file because here `MallocChecker.cpp` sinks
+// execution paths that refer to members of a deleted object.
diff --git a/clang/test/Analysis/test-member-invalidation.cpp b/clang/test/Analysis/test-member-invalidation.cpp
new file mode 100644
index 0000000000000..cbff3986325df
--- /dev/null
+++ b/clang/test/Analysis/test-member-invalidation.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify=expected,nosink -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -std=c++11 -verify=expected,sink -analyzer-config eagerly-assume=false %s
+
+// This test validates that calling an unknown destructor invalidates the
+// members of an object. This was originally a part of the test file `new.cpp`,
+// but was split off into a separate file because the checker family
+// implemented in `MallocChecker.cpp` (which is activated via unix.Malloc in
+// `new.cpp` sinks all execution paths that refer to members of a deleted object.
+
+void clang_analyzer_eval(bool);
+
+// Invalidate Region even in case of default destructor
+class InvalidateDestTest {
+public:
+ int x;
+ int *y;
+ ~InvalidateDestTest();
+};
+
+int test_member_invalidation() {
+
+ //test invalidation of member variable
+ InvalidateDestTest *test = new InvalidateDestTest();
+ test->x = 5;
+ int *k = &(test->x);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+ delete test;
+ clang_analyzer_eval(*k == 5); // nosink-warning{{UNKNOWN}}
+
+ //test invalidation of member pointer
+ int localVar = 5;
+ test = new InvalidateDestTest();
+ test->y = &localVar;
+ delete test;
+ clang_analyzer_eval(localVar == 5); // nosink-warning{{UNKNOWN}}
+
+ // Test aray elements are invalidated.
+ int Var1 = 5;
+ int Var2 = 5;
+ InvalidateDestTest *a = new InvalidateDestTest[2];
+ a[0].y = &Var1;
+ a[1].y = &Var2;
+ delete[] a;
+ clang_analyzer_eval(Var1 == 5); // nosink-warning{{UNKNOWN}}
+ clang_analyzer_eval(Var2 == 5); // nosink-warning{{UNKNOWN}}
+ return 0;
+}
>From 69f8436613b99bd706001f89ad3a1b8fa9482f5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 7 Jul 2025 17:30:01 +0200
Subject: [PATCH 02/10] Don't call `getName()` on disabled frontend
...because it would crash by dereferencing nullopt.
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 9e7540eecc8ee..88816a9ecedec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3853,7 +3853,7 @@ void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State,
Sym->dumpToStream(Out);
Out << " : ";
Data.dump(Out);
- if (Frontend)
+ if (Frontend && Frontend->isEnabled())
Out << " (" << Frontend->getName() << ")";
Out << NL;
}
>From 0e6aeb000b06f551161adb6bc0d6df5a56521313 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 7 Jul 2025 17:32:59 +0200
Subject: [PATCH 03/10] Don't replace leak reports with sinks
`handleNullOrDisabled` creates a sink when there is a checker frontend that _would report_ the situation but it's disabled. However, this is inappropriate for `Leak`s because that bug report is a non-fatal error so it should not be replaced by a sink.
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 88816a9ecedec..642fcf003ae31 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2987,7 +2987,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
return;
const Leak *Frontend = getRelevantFrontendAs<Leak>(Family);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend || !Frontend->isEnabled())
return;
// Most bug reports are cached at the location where they occurred.
>From dc5b258778fe3a14fb49ce8d1e28605162889620 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 7 Jul 2025 18:30:16 +0200
Subject: [PATCH 04/10] Don't hide addSink in helper method
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 56 ++++++++++++-------
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 642fcf003ae31..6b60a72541811 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -449,20 +449,6 @@ class MallocChecker
const char *NL, const char *Sep) const override;
private:
- /// Helper method to handle the cases where there is no associated frontend
- /// (just exit early) or the associated frontend is disabled (sink the
- /// execution path and and then exit early). Intended to be called as
- /// if (handleNullOrDisabled(Frontend, C))
- /// return;
- static bool handleNullOrDisabled(const CheckerFrontend *F,
- CheckerContext &C) {
- if (F && F->isEnabled())
- return false;
- if (F)
- C.addSink();
- return true;
- }
-
#define CHECK_FN(NAME) \
void NAME(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) \
const;
@@ -2534,8 +2520,12 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
const Expr *DeallocExpr,
AllocationFamily Family) const {
const BadFree *Frontend = getRelevantFrontendAs<BadFree>(Family);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
SmallString<100> buf;
@@ -2648,8 +2638,12 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
AllocationFamily Family,
const Expr *AllocExpr) const {
const OffsetFree *Frontend = getRelevantFrontendAs<OffsetFree>(Family);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
ExplodedNode *N = C.generateErrorNode();
if (!N)
@@ -2694,8 +2688,12 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const {
const UseFree *Frontend = getRelevantFrontendAs<UseFree>(C, Sym);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
AllocationFamily AF =
@@ -2723,8 +2721,12 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range,
bool Released, SymbolRef Sym,
SymbolRef PrevSym) const {
const DoubleFree *Frontend = getRelevantFrontendAs<DoubleFree>(C, Sym);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
auto R = std::make_unique<PathSensitiveBugReport>(
@@ -2743,8 +2745,12 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range,
void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
const DoubleDelete *Frontend = getRelevantFrontendAs<DoubleDelete>(C, Sym);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
@@ -2761,8 +2767,12 @@ void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const {
const UseZeroAllocated *Frontend =
getRelevantFrontendAs<UseZeroAllocated>(C, Sym);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
auto R = std::make_unique<PathSensitiveBugReport>(
@@ -2783,8 +2793,12 @@ void MallocChecker::HandleFunctionPtrFree(CheckerContext &C, SVal ArgVal,
const Expr *FreeExpr,
AllocationFamily Family) const {
const BadFree *Frontend = getRelevantFrontendAs<BadFree>(Family);
- if (handleNullOrDisabled(Frontend, C))
+ if (!Frontend)
return;
+ if (!Frontend->isEnabled()) {
+ C.addSink();
+ return;
+ }
if (ExplodedNode *N = C.generateErrorNode()) {
SmallString<100> Buf;
>From 128d25af2da8033528606e8f0604570688881382 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 7 Jul 2025 18:46:46 +0200
Subject: [PATCH 05/10] Fix an obsolete comment, add FIXMEs
I hope that I can address these FIXMEs in a follow-up commit.
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 6b60a72541811..5426336b5f32d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3108,6 +3108,10 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
if (const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call)) {
const CXXDeleteExpr *DE = DC->getOriginExpr();
+ // FIXME: I don't see a good reason for restricting the check against
+ // use-after-free violations to the case when NewDeleteChecker is disabled.
+ // (However, if NewDeleteChecker is enabled, perhaps it would be better to
+ // do this check a bit later?)
if (!NewDeleteChecker.isEnabled())
if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
checkUseAfterFree(Sym, C, DE->getArgument());
@@ -3139,12 +3143,18 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
return;
}
- // We will check for double free in the post visit.
+ // We will check for double free in the `evalCall` callback.
+ // FIXME: It would be more logical to emit double free and use-after-free
+ // reports via the same pathway (because double free is essentially a specia
+ // case of use-after-free).
if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
const FunctionDecl *FD = FC->getDecl();
if (!FD)
return;
+ // FIXME: I suspect we should remove `MallocChecker.isEnabled() &&` because
+ // it's fishy that the enabled/disabled state of one frontend may influence
+ // reports produced by other frontends.
if (MallocChecker.isEnabled() && isFreeingCall(Call))
return;
}
>From 0105e6d3fb41c81e806983411f812c5c73667120 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 7 Jul 2025 18:53:30 +0200
Subject: [PATCH 06/10] Add another comment to explain unusual behavior
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 5426336b5f32d..57c035f405b1d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3001,6 +3001,9 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
return;
const Leak *Frontend = getRelevantFrontendAs<Leak>(Family);
+ // Note that for leaks we don't add a sink when the relevant frontend is
+ // disabled because the leak is reported with a non-fatal error node, while
+ // the sink would be the "silent" alternative of a (fatal) error node.
if (!Frontend || !Frontend->isEnabled())
return;
>From 74b59e98eac7a2724451c58b20d1875dbd518db3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 8 Jul 2025 10:54:35 +0200
Subject: [PATCH 07/10] Actually convert MallocChecker to a CheckerFamily
(oops)
---
.../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 15 ++++++++++-----
.../Checkers/NoOwnershipChangeVisitor.h | 4 ++--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 57c035f405b1d..526e1e6e871cd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -384,13 +384,14 @@ struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... {
//===----------------------------------------------------------------------===//
class MallocChecker
- : public Checker<check::DeadSymbols, check::PointerEscape,
+ : public CheckerFamily<check::DeadSymbols, check::PointerEscape,
check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
check::EndFunction, check::PreCall, check::PostCall,
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.
/// In optimistic mode, the checker assumes that all user-defined functions
@@ -448,6 +449,8 @@ class MallocChecker
void printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const override;
+ StringRef getDebugTag() const override { return "MallocChecker"; }
+
private:
#define CHECK_FN(NAME) \
void NAME(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) \
@@ -3908,12 +3911,14 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &Mgr) {
}
void ento::registerDynamicMemoryModeling(CheckerManager &Mgr) {
- auto *Chk = Mgr.registerChecker<MallocChecker>();
+ auto *Chk = Mgr.getChecker<MallocChecker>();
+ // FIXME: This is a "hidden" undocumented frontend but there are public
+ // checker options which are attached to it.
+ CheckerNameRef DMMName = Mgr.getCurrentCheckerName();
Chk->ShouldIncludeOwnershipAnnotatedFunctions =
- Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Optimistic");
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(DMMName, "Optimistic");
Chk->ShouldRegisterNoOwnershipChangeVisitor =
- Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Chk, "AddNoOwnershipChangeNotes");
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(DMMName, "AddNoOwnershipChangeNotes");
}
bool ento::shouldRegisterDynamicMemoryModeling(const CheckerManager &mgr) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
index 027f1a156a7c0..44fed68b97acb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
+++ b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
@@ -17,7 +17,7 @@ class NoOwnershipChangeVisitor : public NoStateChangeFuncVisitor {
protected:
// The symbol whose (lack of) ownership change we are interested in.
SymbolRef Sym;
- const CheckerBase &Checker;
+ const CheckerBackend &Checker;
LLVM_DUMP_METHOD static std::string
getFunctionName(const ExplodedNode *CallEnterN);
@@ -63,7 +63,7 @@ class NoOwnershipChangeVisitor : public NoStateChangeFuncVisitor {
OwnerSet getOwnersAtNode(const ExplodedNode *N);
public:
- NoOwnershipChangeVisitor(SymbolRef Sym, const CheckerBase *Checker)
+ NoOwnershipChangeVisitor(SymbolRef Sym, const CheckerBackend *Checker)
: NoStateChangeFuncVisitor(bugreporter::TrackingKind::Thorough), Sym(Sym),
Checker(*Checker) {}
>From 746ab7bef5f547b7005bac5f2046ddb0e5c03234 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 8 Jul 2025 12:26:14 +0200
Subject: [PATCH 08/10] Satisfy git-clang-format
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 526e1e6e871cd..073d80dc87401 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -384,14 +384,13 @@ struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... {
//===----------------------------------------------------------------------===//
class MallocChecker
- : public CheckerFamily<check::DeadSymbols, check::PointerEscape,
- check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
- check::EndFunction, check::PreCall, check::PostCall,
- eval::Call, check::NewAllocator,
- check::PostStmt<BlockExpr>, check::PostObjCMessage,
- check::Location, eval::Assume> {
+ : public CheckerFamily<
+ check::DeadSymbols, check::PointerEscape, check::ConstPointerEscape,
+ check::PreStmt<ReturnStmt>, check::EndFunction, check::PreCall,
+ check::PostCall, 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.
/// In optimistic mode, the checker assumes that all user-defined functions
@@ -3918,7 +3917,8 @@ void ento::registerDynamicMemoryModeling(CheckerManager &Mgr) {
Chk->ShouldIncludeOwnershipAnnotatedFunctions =
Mgr.getAnalyzerOptions().getCheckerBooleanOption(DMMName, "Optimistic");
Chk->ShouldRegisterNoOwnershipChangeVisitor =
- Mgr.getAnalyzerOptions().getCheckerBooleanOption(DMMName, "AddNoOwnershipChangeNotes");
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ DMMName, "AddNoOwnershipChangeNotes");
}
bool ento::shouldRegisterDynamicMemoryModeling(const CheckerManager &mgr) {
>From 43eb9222e92dd75b246cd0a786fcce377d95793f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 8 Jul 2025 12:26:44 +0200
Subject: [PATCH 09/10] Undef BUGTYPE_PROVIDER
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 073d80dc87401..af1cac80b1c1c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -368,6 +368,8 @@ BUGTYPE_PROVIDER(MismatchedDealloc, "Bad deallocator")
BUGTYPE_PROVIDER(OffsetFree, "Offset free")
BUGTYPE_PROVIDER(UseZeroAllocated, "Use of zero allocated")
+#undef BUGTYPE_PROVIDER
+
template <typename... BT_PROVIDERS>
struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... {
template <typename T> const T *getAs() const {
>From 6f4cc0cd3d133c54d51ddf80411e7eccfc3c9734 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 8 Jul 2025 12:28:03 +0200
Subject: [PATCH 10/10] Merge two ifs with identical bodies
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index af1cac80b1c1c..0a58d7cc2635a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -373,9 +373,8 @@ BUGTYPE_PROVIDER(UseZeroAllocated, "Use of zero allocated")
template <typename... BT_PROVIDERS>
struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... {
template <typename T> const T *getAs() const {
- if constexpr (std::is_same_v<T, CheckerFrontend>)
- return static_cast<const T *>(this);
- if constexpr ((std::is_same_v<T, BT_PROVIDERS> || ...))
+ if constexpr (std::is_same_v<T, CheckerFrontend> ||
+ (std::is_same_v<T, BT_PROVIDERS> || ...))
return static_cast<const T *>(this);
return nullptr;
}
More information about the cfe-commits
mailing list