[clang] [analyzer] Connversion to CheckerFamily: MallocChecker (PR #147080)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 4 08:31:00 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Donát Nagy (NagyDonat)
<details>
<summary>Changes</summary>
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.
---
Patch is 33.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147080.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+176-238)
- (modified) clang/test/Analysis/new.cpp (+4-36)
- (added) clang/test/Analysis/test-member-invalidation.cpp (+47)
``````````diff
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 Ma...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/147080
More information about the cfe-commits
mailing list