[clang] [nfc][analyzer] Add MemSpace trait to program state (PR #123003)
Michael Flanders via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 15 12:03:12 PST 2025
https://github.com/Flandini updated https://github.com/llvm/llvm-project/pull/123003
>From 7e0758d2ead53bd4288989b8b2eda218cd6dc34a Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 13 Jan 2025 12:34:50 -0600
Subject: [PATCH 01/14] [analyzer] Add MemSpace trait to program state
This trait associates MemSpaceRegions with MemRegions to allow refining or
changing information known about memory regions after they are created, since
they are immutable. This commit is an intermediate step towards moving
MemSpaceRegion out of the MemRegion class hierarchy and moving all notion of
MemSpaceRegion to the trait. The intermediate step is that for now, only
MemRegions with UnknownSpaceRegion are mapped in the trait and checked in
checkers/core.
---
.../Core/PathSensitive/MemSpaces.h | 47 ++++++++++++++
.../Checkers/ArrayBoundCheckerV2.cpp | 1 +
.../Checkers/MacOSXAPIChecker.cpp | 10 +--
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 32 +++++++---
.../StaticAnalyzer/Checkers/MoveChecker.cpp | 41 ++++++------
.../Checkers/PutenvStackArrayChecker.cpp | 10 ++-
.../RetainCountDiagnostics.cpp | 12 +++-
.../Checkers/StackAddrEscapeChecker.cpp | 26 +++++---
.../Checkers/UnixAPIChecker.cpp | 3 +-
.../Core/BugReporterVisitors.cpp | 12 +++-
clang/lib/StaticAnalyzer/Core/CMakeLists.txt | 1 +
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 10 +++
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 62 +++++++++++++++++++
13 files changed, 221 insertions(+), 46 deletions(-)
create mode 100644 clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
create mode 100644 clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
new file mode 100644
index 00000000000000..178a6b60c1cb1a
--- /dev/null
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -0,0 +1,47 @@
+//===-- MemSpaces.h -----------------------------------------------*- C++ -*--//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// TODO
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H
+#define LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+
+namespace clang {
+namespace ento {
+
+class MemRegion;
+class MemSpaceRegion;
+
+namespace memspace {
+
+[[nodiscard]] ProgramStateRef setMemSpaceTrait(ProgramStateRef State,
+ const MemRegion *MR,
+ const MemSpaceRegion *MS);
+
+[[nodiscard]] const MemSpaceRegion *getMemSpaceTrait(ProgramStateRef State,
+ const MemRegion *MR);
+
+[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
+
+template <typename FirstT, typename... RestT>
+[[nodiscard]] bool isMemSpaceOrTrait(ProgramStateRef State,
+ const MemRegion *MR) {
+ return isa<FirstT, RestT...>(MR->getMemorySpace()) ||
+ isa_and_nonnull<FirstT, RestT...>(getMemSpaceTrait(State, MR));
+}
+
+} // namespace memspace
+} // namespace ento
+} // namespace clang
+
+#endif // LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 6422933c8828a9..e97be53fee4c7f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -22,6 +22,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/FormatVariadic.h"
diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
index 754b167642961c..858b6a37551f5d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -22,6 +22,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringSwitch.h"
@@ -75,10 +76,11 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
if (!R)
return;
+ ProgramStateRef State = C.getState();
+
// Global variables are fine.
const MemRegion *RB = R->getBaseRegion();
- const MemSpaceRegion *RS = RB->getMemorySpace();
- if (isa<GlobalsSpaceRegion>(RS))
+ if (memspace::isMemSpaceOrTrait<GlobalsSpaceRegion>(State, RB))
return;
// Handle _dispatch_once. In some versions of the OS X SDK we have the case
@@ -117,9 +119,9 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
if (IVR != R)
os << " memory within";
os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
- } else if (isa<HeapSpaceRegion>(RS)) {
+ } else if (memspace::isMemSpaceOrTrait<HeapSpaceRegion>(State, RB)) {
os << " heap-allocated memory";
- } else if (isa<UnknownSpaceRegion>(RS)) {
+ } else if (isa<UnknownSpaceRegion>(RB->getMemorySpace())) {
// Presence of an IVar superregion has priority over this branch, because
// ObjC objects are on the heap even if the core doesn't realize this.
// Presence of a block variable base region has priority over this branch,
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 4166cf14391e2d..76fee2f1322e17 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -72,6 +72,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
@@ -784,7 +785,8 @@ class MallocChecker
bool IsALeakCheck = false) const;
///@}
static bool SummarizeValue(raw_ostream &os, SVal V);
- static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
+ static bool SummarizeRegion(ProgramStateRef State, raw_ostream &os,
+ const MemRegion *MR);
void HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal, SourceRange Range,
const Expr *DeallocExpr,
@@ -2206,13 +2208,21 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
// Parameters, locals, statics, globals, and memory returned by
// __builtin_alloca() shouldn't be freed.
- if (!isa<UnknownSpaceRegion, HeapSpaceRegion>(MS)) {
+ // Should skip this check if:
+ // - The region is in the heap
+ // - The region has unknown memspace and no trait for further clarification
+ // (i.e., just unknown)
+ // - The region has unknown memspace and a heap memspace trait
+ const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, R);
+ bool HasHeapOrUnknownTrait = !MSTrait || isa<HeapSpaceRegion>(MSTrait);
+ if (!(isa<HeapSpaceRegion>(MS) ||
+ (isa<UnknownSpaceRegion>(MS) && HasHeapOrUnknownTrait))) {
// Regions returned by malloc() are represented by SymbolicRegion objects
// within HeapSpaceRegion. Of course, free() can work on memory allocated
// outside the current function, so UnknownSpaceRegion is also a
// possibility here.
- if (isa<AllocaRegion>(R))
+ if (isa<AllocaRegion>(R) || isa_and_nonnull<AllocaRegion>(MSTrait))
HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
else
HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
@@ -2384,7 +2394,7 @@ bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
return true;
}
-bool MallocChecker::SummarizeRegion(raw_ostream &os,
+bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
const MemRegion *MR) {
switch (MR->getKind()) {
case MemRegion::FunctionCodeRegionKind: {
@@ -2404,8 +2414,10 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
return true;
default: {
const MemSpaceRegion *MS = MR->getMemorySpace();
+ const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, MR);
- if (isa<StackLocalsSpaceRegion>(MS)) {
+ if (isa<StackLocalsSpaceRegion>(MS) ||
+ isa_and_nonnull<StackLocalsSpaceRegion>(MSTrait)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
@@ -2420,7 +2432,8 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
return true;
}
- if (isa<StackArgumentsSpaceRegion>(MS)) {
+ if (isa<StackArgumentsSpaceRegion>(MS) ||
+ isa_and_nonnull<StackArgumentsSpaceRegion>(MSTrait)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
@@ -2435,7 +2448,8 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
return true;
}
- if (isa<GlobalsSpaceRegion>(MS)) {
+ if (isa<GlobalsSpaceRegion>(MS) ||
+ isa_and_nonnull<GlobalsSpaceRegion>(MSTrait)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
@@ -2489,8 +2503,8 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
os << "deallocator";
os << " is ";
- bool Summarized = MR ? SummarizeRegion(os, MR)
- : SummarizeValue(os, ArgVal);
+ bool Summarized =
+ MR ? SummarizeRegion(C.getState(), os, MR) : SummarizeValue(os, ArgVal);
if (Summarized)
os << ", which is not memory allocated by ";
else
diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index 52416e21399147..d4ff8186ea69c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -22,6 +22,7 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "llvm/ADT/StringSet.h"
using namespace clang;
@@ -145,12 +146,14 @@ class MoveChecker
// Obtains ObjectKind of an object. Because class declaration cannot always
// be easily obtained from the memory region, it is supplied separately.
- ObjectKind classifyObject(const MemRegion *MR, const CXXRecordDecl *RD) const;
+ ObjectKind classifyObject(ProgramStateRef State, const MemRegion *MR,
+ const CXXRecordDecl *RD) const;
// Classifies the object and dumps a user-friendly description string to
// the stream.
- void explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
- const CXXRecordDecl *RD, MisuseKind MK) const;
+ void explainObject(ProgramStateRef State, llvm::raw_ostream &OS,
+ const MemRegion *MR, const CXXRecordDecl *RD,
+ MisuseKind MK) const;
bool belongsTo(const CXXRecordDecl *RD, const llvm::StringSet<> &Set) const;
@@ -299,12 +302,12 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
SmallString<128> Str;
llvm::raw_svector_ostream OS(Str);
- ObjectKind OK = Chk.classifyObject(Region, RD);
+ ObjectKind OK = Chk.classifyObject(State, Region, RD);
switch (OK.StdKind) {
case SK_SmartPtr:
if (MK == MK_Dereference) {
OS << "Smart pointer";
- Chk.explainObject(OS, Region, RD, MK);
+ Chk.explainObject(State, OS, Region, RD, MK);
OS << " is reset to null when moved from";
break;
}
@@ -315,12 +318,12 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
case SK_NonStd:
case SK_Safe:
OS << "Object";
- Chk.explainObject(OS, Region, RD, MK);
+ Chk.explainObject(State, OS, Region, RD, MK);
OS << " is moved";
break;
case SK_Unsafe:
OS << "Object";
- Chk.explainObject(OS, Region, RD, MK);
+ Chk.explainObject(State, OS, Region, RD, MK);
OS << " is left in a valid but unspecified state after move";
break;
}
@@ -353,7 +356,7 @@ void MoveChecker::modelUse(ProgramStateRef State, const MemRegion *Region,
CheckerContext &C) const {
assert(!C.isDifferent() && "No transitions should have been made by now");
const RegionState *RS = State->get<TrackedRegionMap>(Region);
- ObjectKind OK = classifyObject(Region, RD);
+ ObjectKind OK = classifyObject(State, Region, RD);
// Just in case: if it's not a smart pointer but it does have operator *,
// we shouldn't call the bug a dereference.
@@ -406,24 +409,25 @@ ExplodedNode *MoveChecker::tryToReportBug(const MemRegion *Region,
// Creating the error message.
llvm::SmallString<128> Str;
llvm::raw_svector_ostream OS(Str);
+ ProgramStateRef ErrorNodeState = N->getState();
switch(MK) {
case MK_FunCall:
OS << "Method called on moved-from object";
- explainObject(OS, Region, RD, MK);
+ explainObject(ErrorNodeState, OS, Region, RD, MK);
break;
case MK_Copy:
OS << "Moved-from object";
- explainObject(OS, Region, RD, MK);
+ explainObject(ErrorNodeState, OS, Region, RD, MK);
OS << " is copied";
break;
case MK_Move:
OS << "Moved-from object";
- explainObject(OS, Region, RD, MK);
+ explainObject(ErrorNodeState, OS, Region, RD, MK);
OS << " is moved";
break;
case MK_Dereference:
OS << "Dereference of null smart pointer";
- explainObject(OS, Region, RD, MK);
+ explainObject(ErrorNodeState, OS, Region, RD, MK);
break;
}
@@ -482,7 +486,7 @@ void MoveChecker::checkPostCall(const CallEvent &Call,
return;
const CXXRecordDecl *RD = MethodDecl->getParent();
- ObjectKind OK = classifyObject(ArgRegion, RD);
+ ObjectKind OK = classifyObject(State, ArgRegion, RD);
if (shouldBeTracked(OK)) {
// Mark object as moved-from.
State = State->set<TrackedRegionMap>(ArgRegion, RegionState::getMoved());
@@ -549,7 +553,7 @@ bool MoveChecker::belongsTo(const CXXRecordDecl *RD,
}
MoveChecker::ObjectKind
-MoveChecker::classifyObject(const MemRegion *MR,
+MoveChecker::classifyObject(ProgramStateRef State, const MemRegion *MR,
const CXXRecordDecl *RD) const {
// Local variables and local rvalue references are classified as "Local".
// For the purposes of this checker, we classify move-safe STL types
@@ -557,7 +561,7 @@ MoveChecker::classifyObject(const MemRegion *MR,
MR = unwrapRValueReferenceIndirection(MR);
bool IsLocal =
isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
- isa<StackSpaceRegion>(MR->getMemorySpace());
+ memspace::isMemSpaceOrTrait<StackSpaceRegion>(State, MR);
if (!RD || !RD->getDeclContext()->isStdNamespace())
return { IsLocal, SK_NonStd };
@@ -571,8 +575,9 @@ MoveChecker::classifyObject(const MemRegion *MR,
return { IsLocal, SK_Unsafe };
}
-void MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
- const CXXRecordDecl *RD, MisuseKind MK) const {
+void MoveChecker::explainObject(ProgramStateRef State, llvm::raw_ostream &OS,
+ const MemRegion *MR, const CXXRecordDecl *RD,
+ MisuseKind MK) const {
// We may need a leading space every time we actually explain anything,
// and we never know if we are to explain anything until we try.
if (const auto DR =
@@ -581,7 +586,7 @@ void MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
OS << " '" << RegionDecl->getDeclName() << "'";
}
- ObjectKind OK = classifyObject(MR, RD);
+ ObjectKind OK = classifyObject(State, MR, RD);
switch (OK.StdKind) {
case SK_NonStd:
case SK_Safe:
diff --git a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
index bf81d57bf82fd3..4449cb4ae68eb9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
using namespace clang;
using namespace ento;
@@ -45,10 +46,15 @@ void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
SVal ArgV = Call.getArgSVal(0);
const Expr *ArgExpr = Call.getArgExpr(0);
- const auto *SSR =
- dyn_cast<StackSpaceRegion>(ArgV.getAsRegion()->getMemorySpace());
+ const MemRegion *MR = ArgV.getAsRegion();
+
+ const auto *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
+ if (!SSR)
+ SSR = dyn_cast_if_present<StackSpaceRegion>(
+ memspace::getMemSpaceTrait(C.getState(), MR));
if (!SSR)
return;
+
const auto *StackFrameFuncD =
dyn_cast_or_null<FunctionDecl>(SSR->getStackFrame()->getDecl());
if (StackFrameFuncD && StackFrameFuncD->isMain())
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 456132ef0a0a22..ecc7563ceb4bba 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -13,6 +13,7 @@
#include "RetainCountDiagnostics.h"
#include "RetainCountChecker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include <optional>
@@ -690,9 +691,14 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
const MemRegion *R = FB.getRegion();
// Do not show local variables belonging to a function other than
// where the error is reported.
- if (auto MR = dyn_cast<StackSpaceRegion>(R->getMemorySpace()))
- if (MR->getStackFrame() == LeakContext->getStackFrame())
- FirstBinding = R;
+ const StackSpaceRegion *MR =
+ dyn_cast<StackSpaceRegion>(R->getMemorySpace());
+ if (!MR)
+ MR = dyn_cast_if_present<StackSpaceRegion>(
+ memspace::getMemSpaceTrait(St, R));
+
+ if (MR && MR->getStackFrame() == LeakContext->getStackFrame())
+ FirstBinding = R;
}
// AllocationNode is the last node in which the symbol was tracked.
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index f4de3b500499c4..4d8c8eea14ea63 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -19,6 +19,7 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/raw_ostream.h"
@@ -61,7 +62,7 @@ class StackAddrEscapeChecker
ASTContext &Ctx);
static SmallVector<const MemRegion *, 4>
getCapturedStackRegions(const BlockDataRegion &B, CheckerContext &C);
- static bool isNotInCurrentFrame(const MemRegion *R, CheckerContext &C);
+ static bool isNotInCurrentFrame(const MemSpaceRegion *MS, CheckerContext &C);
};
} // namespace
@@ -117,9 +118,9 @@ SourceRange StackAddrEscapeChecker::genName(raw_ostream &os, const MemRegion *R,
return range;
}
-bool StackAddrEscapeChecker::isNotInCurrentFrame(const MemRegion *R,
+bool StackAddrEscapeChecker::isNotInCurrentFrame(const MemSpaceRegion *MS,
CheckerContext &C) {
- const StackSpaceRegion *S = cast<StackSpaceRegion>(R->getMemorySpace());
+ const StackSpaceRegion *S = cast<StackSpaceRegion>(MS);
return S->getStackFrame() != C.getStackFrame();
}
@@ -138,10 +139,11 @@ SmallVector<const MemRegion *, 4>
StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
CheckerContext &C) {
SmallVector<const MemRegion *, 4> Regions;
+ ProgramStateRef State = C.getState();
for (auto Var : B.referenced_vars()) {
- SVal Val = C.getState()->getSVal(Var.getCapturedRegion());
+ SVal Val = State->getSVal(Var.getCapturedRegion());
const MemRegion *Region = Val.getAsRegion();
- if (Region && isa<StackSpaceRegion>(Region->getMemorySpace()))
+ if (Region && memspace::isMemSpaceOrTrait<StackSpaceRegion>(State, Region))
Regions.push_back(Region);
}
return Regions;
@@ -212,7 +214,7 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
void StackAddrEscapeChecker::checkReturnedBlockCaptures(
const BlockDataRegion &B, CheckerContext &C) const {
for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
- if (isNotInCurrentFrame(Region, C))
+ if (isNotInCurrentFrame(Region->getMemorySpace(), C))
continue;
ExplodedNode *N = C.generateNonFatalErrorNode();
if (!N)
@@ -265,7 +267,14 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
checkReturnedBlockCaptures(*B, C);
- if (!isa<StackSpaceRegion>(R->getMemorySpace()) || isNotInCurrentFrame(R, C))
+ if (isa<UnknownSpaceRegion>(R)) {
+ const MemSpaceRegion *MS = memspace::getMemSpaceTrait(C.getState(), R);
+ if (!isa_and_nonnull<StackSpaceRegion>(MS) || isNotInCurrentFrame(MS, C))
+ return;
+ }
+
+ const MemSpaceRegion *MS = R->getMemorySpace();
+ if (!isa<StackSpaceRegion>(MS) || isNotInCurrentFrame(MS, C))
return;
// Returning a record by value is fine. (In this case, the returned
@@ -468,7 +477,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
if (!isa_and_nonnull<GlobalsSpaceRegion>(
getStackOrGlobalSpaceRegion(Region)))
return true;
- if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
+ if (VR && VR->hasStackStorage() &&
+ !isNotInCurrentFrame(VR->getMemorySpace(), Ctx))
V.emplace_back(Region, VR);
return true;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index da2d16ca9b5dd7..1c6c5fa14bceec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
@@ -411,7 +412,7 @@ void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C,
// because that's likely to be bad news.
ProgramStateRef state = C.getState();
const MemRegion *R = Call.getArgSVal(0).getAsRegion();
- if (!R || !isa<StackSpaceRegion>(R->getMemorySpace()))
+ if (!R || !memspace::isMemSpaceOrTrait<StackSpaceRegion>(state, R))
return;
ExplodedNode *N = C.generateErrorNode(state);
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index a9b4dbb39b5bd6..e708bd8ec2f72d 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -40,6 +40,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
@@ -1193,7 +1194,16 @@ static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
return false;
const MemSpaceRegion *VarSpace = VR->getMemorySpace();
- const auto *FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace);
+ const StackSpaceRegion *FrameSpace;
+
+ if (isa<UnknownSpaceRegion>(VarSpace)) {
+ ProgramStateRef State = N->getState();
+ const MemSpaceRegion *MemSpace = memspace::getMemSpaceTrait(State, VR);
+ FrameSpace = dyn_cast_if_present<StackSpaceRegion>(MemSpace);
+ } else {
+ FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace);
+ }
+
if (!FrameSpace) {
// If we ever directly evaluate global DeclStmts, this assertion will be
// invalid, but this still seems preferable to silently accepting an
diff --git a/clang/lib/StaticAnalyzer/Core/CMakeLists.txt b/clang/lib/StaticAnalyzer/Core/CMakeLists.txt
index fb9394a519eb71..993d37bb052e9b 100644
--- a/clang/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -36,6 +36,7 @@ add_clang_library(clangStaticAnalyzerCore
LoopUnrolling.cpp
LoopWidening.cpp
MemRegion.cpp
+ MemSpaces.cpp
PlistDiagnostics.cpp
ProgramState.cpp
RangeConstraintManager.cpp
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 70e95c2c644c09..f22d4e7bf9dbd0 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -53,6 +53,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
@@ -3516,6 +3517,15 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
continue;
}
+ // Case (2) continued.
+ if (isa<UnknownSpaceRegion>(MR)) {
+ const MemSpaceRegion *MS = memspace::getMemSpaceTrait(State, MR);
+ if (!isa<StackSpaceRegion, StaticGlobalSpaceRegion>(MS)) {
+ Escaped.push_back(LocAndVal.second);
+ continue;
+ }
+ }
+
// Case (3).
if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
new file mode 100644
index 00000000000000..909d11d6456758
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -0,0 +1,62 @@
+//===-- MemSpaces.cpp ---------------------------------------------*- C++ -*--//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// TODO
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include <cassert>
+
+REGISTER_MAP_WITH_PROGRAMSTATE(MemSpacesMap, const clang::ento::MemRegion *,
+ const clang::ento::MemSpaceRegion *)
+
+namespace clang {
+namespace ento {
+namespace memspace {
+
+ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
+ const MemSpaceRegion *MS) {
+ // for now, this should only be called to update the trait for mem regions
+ // that have an unknown mem spaces since we assume everywhere else that the
+ // memspace trait is set only for unknown mem spaces (setting this info
+ // otherwise would go unused).
+ assert(isa<UnknownSpaceRegion>(MR->getMemorySpace()));
+
+ // Shouldn't use the memspace trait to associate UnknownSpaceRegion with an
+ // already UnknownSpaceRegion
+ assert(!isa<UnknownSpaceRegion>(MS));
+
+ ProgramStateRef NewState = State->set<MemSpacesMap>(MR, MS);
+ return NewState;
+}
+
+bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR) {
+ if (!isa<UnknownSpaceRegion>(MR->getMemorySpace()))
+ return false;
+
+ const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
+ return Result;
+}
+
+const MemSpaceRegion *getMemSpaceTrait(ProgramStateRef State,
+ const MemRegion *MR) {
+ if (!isa<UnknownSpaceRegion>(MR->getMemorySpace()))
+ return nullptr;
+
+ const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
+ if (!Result)
+ return nullptr;
+ return *Result;
+}
+
+} // namespace memspace
+} // namespace ento
+} // namespace clang
>From 94c0e4da693faeacb108e3dc239ed56d99242b3c Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 09:13:10 -0600
Subject: [PATCH 02/14] Uppercase start and s/mem/memory/ in long comment
---
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index 909d11d6456758..886359fdbfab3d 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -24,13 +24,13 @@ namespace memspace {
ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
const MemSpaceRegion *MS) {
- // for now, this should only be called to update the trait for mem regions
- // that have an unknown mem spaces since we assume everywhere else that the
- // memspace trait is set only for unknown mem spaces (setting this info
+ // For now, this should only be called to update the trait for memory regions
+ // that have an unknown memory spaces since we assume everywhere else that the
+ // memory space trait is set only for unknown memory spaces (setting this info
// otherwise would go unused).
assert(isa<UnknownSpaceRegion>(MR->getMemorySpace()));
- // Shouldn't use the memspace trait to associate UnknownSpaceRegion with an
+ // Shouldn't use the memory space trait to associate UnknownSpaceRegion with an
// already UnknownSpaceRegion
assert(!isa<UnknownSpaceRegion>(MS));
>From 79a7c900303b84164aa8b473f0e241e44a80062b Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 09:22:54 -0600
Subject: [PATCH 03/14] Fill in TODO file headers description and format
comments
---
.../clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h | 7 ++++++-
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 8 +++++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index 178a6b60c1cb1a..4f03738254b936 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -6,7 +6,12 @@
//
//===----------------------------------------------------------------------===//
//
-// TODO
+// This file declares getters and setters for the memory space trait which
+// associates memory regions with memory spaces in the program state. We are
+// still transitioning to having all memory space information stored in the
+// trait, so currently memory regions still have memory spaces as a class field.
+// Because of this, these getters still consider memory spaces set as class
+// fields in memory region types.
//
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index 886359fdbfab3d..47ad03c04b2ece 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -6,7 +6,9 @@
//
//===----------------------------------------------------------------------===//
//
-// TODO
+// This file defines getters and setters for the memory space trait which
+// associates memory regions with memory spaces in the program state. It also
+// defines the MemSpacesMap which maps memory regions to memory spaces.
//
//===----------------------------------------------------------------------===//
@@ -30,8 +32,8 @@ ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
// otherwise would go unused).
assert(isa<UnknownSpaceRegion>(MR->getMemorySpace()));
- // Shouldn't use the memory space trait to associate UnknownSpaceRegion with an
- // already UnknownSpaceRegion
+ // Shouldn't use the memory space trait to associate UnknownSpaceRegion with
+ // an already UnknownSpaceRegion
assert(!isa<UnknownSpaceRegion>(MS));
ProgramStateRef NewState = State->set<MemSpacesMap>(MR, MS);
>From bcc2f937bcc22d98ee06ebea9e3944fc16089997 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 11:33:52 -0600
Subject: [PATCH 04/14] getMemSpaceTrait->getMemSpace
---
.../StaticAnalyzer/Core/PathSensitive/MemSpaces.h | 4 ++--
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 4 ++--
.../Checkers/PutenvStackArrayChecker.cpp | 2 +-
.../RetainCountChecker/RetainCountDiagnostics.cpp | 2 +-
.../Checkers/StackAddrEscapeChecker.cpp | 2 +-
.../lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 12 +++++-------
8 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index 4f03738254b936..b6ca1f9e4ab78e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -33,7 +33,7 @@ namespace memspace {
const MemRegion *MR,
const MemSpaceRegion *MS);
-[[nodiscard]] const MemSpaceRegion *getMemSpaceTrait(ProgramStateRef State,
+[[nodiscard]] const MemSpaceRegion *getMemSpace(ProgramStateRef State,
const MemRegion *MR);
[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
@@ -42,7 +42,7 @@ template <typename FirstT, typename... RestT>
[[nodiscard]] bool isMemSpaceOrTrait(ProgramStateRef State,
const MemRegion *MR) {
return isa<FirstT, RestT...>(MR->getMemorySpace()) ||
- isa_and_nonnull<FirstT, RestT...>(getMemSpaceTrait(State, MR));
+ isa_and_nonnull<FirstT, RestT...>(getMemSpace(State, MR));
}
} // namespace memspace
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 76fee2f1322e17..7d13e82472d7bd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2213,7 +2213,7 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
// - The region has unknown memspace and no trait for further clarification
// (i.e., just unknown)
// - The region has unknown memspace and a heap memspace trait
- const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, R);
+ const MemSpaceRegion *MSTrait = memspace::getMemSpace(State, R);
bool HasHeapOrUnknownTrait = !MSTrait || isa<HeapSpaceRegion>(MSTrait);
if (!(isa<HeapSpaceRegion>(MS) ||
(isa<UnknownSpaceRegion>(MS) && HasHeapOrUnknownTrait))) {
@@ -2414,7 +2414,7 @@ bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
return true;
default: {
const MemSpaceRegion *MS = MR->getMemorySpace();
- const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, MR);
+ const MemSpaceRegion *MSTrait = memspace::getMemSpace(State, MR);
if (isa<StackLocalsSpaceRegion>(MS) ||
isa_and_nonnull<StackLocalsSpaceRegion>(MSTrait)) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
index 4449cb4ae68eb9..fddef2ee0ac161 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
@@ -51,7 +51,7 @@ void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
const auto *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
if (!SSR)
SSR = dyn_cast_if_present<StackSpaceRegion>(
- memspace::getMemSpaceTrait(C.getState(), MR));
+ memspace::getMemSpace(C.getState(), MR));
if (!SSR)
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index ecc7563ceb4bba..80fa8d3e3f921e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -695,7 +695,7 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
dyn_cast<StackSpaceRegion>(R->getMemorySpace());
if (!MR)
MR = dyn_cast_if_present<StackSpaceRegion>(
- memspace::getMemSpaceTrait(St, R));
+ memspace::getMemSpace(St, R));
if (MR && MR->getStackFrame() == LeakContext->getStackFrame())
FirstBinding = R;
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 4d8c8eea14ea63..43a444e3b8ee84 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -268,7 +268,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
checkReturnedBlockCaptures(*B, C);
if (isa<UnknownSpaceRegion>(R)) {
- const MemSpaceRegion *MS = memspace::getMemSpaceTrait(C.getState(), R);
+ const MemSpaceRegion *MS = memspace::getMemSpace(C.getState(), R);
if (!isa_and_nonnull<StackSpaceRegion>(MS) || isNotInCurrentFrame(MS, C))
return;
}
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index e708bd8ec2f72d..ad2429d61de927 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1198,7 +1198,7 @@ static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
if (isa<UnknownSpaceRegion>(VarSpace)) {
ProgramStateRef State = N->getState();
- const MemSpaceRegion *MemSpace = memspace::getMemSpaceTrait(State, VR);
+ const MemSpaceRegion *MemSpace = memspace::getMemSpace(State, VR);
FrameSpace = dyn_cast_if_present<StackSpaceRegion>(MemSpace);
} else {
FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index f22d4e7bf9dbd0..82274f3327f16d 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3519,7 +3519,7 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
// Case (2) continued.
if (isa<UnknownSpaceRegion>(MR)) {
- const MemSpaceRegion *MS = memspace::getMemSpaceTrait(State, MR);
+ const MemSpaceRegion *MS = memspace::getMemSpace(State, MR);
if (!isa<StackSpaceRegion, StaticGlobalSpaceRegion>(MS)) {
Escaped.push_back(LocAndVal.second);
continue;
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index 47ad03c04b2ece..1dd6568cfa80d2 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -48,15 +48,13 @@ bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR) {
return Result;
}
-const MemSpaceRegion *getMemSpaceTrait(ProgramStateRef State,
+const MemSpaceRegion *getMemSpace(ProgramStateRef State,
const MemRegion *MR) {
- if (!isa<UnknownSpaceRegion>(MR->getMemorySpace()))
- return nullptr;
-
const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
- if (!Result)
- return nullptr;
- return *Result;
+ if (Result)
+ return *Result;
+
+ return MR->getMemorySpace();
}
} // namespace memspace
>From 28bc5a3009236f3d50a9db9f7580ffd295a2e31c Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 11:49:27 -0600
Subject: [PATCH 05/14] add comment about invariant
---
.../Core/PathSensitive/MemSpaces.h | 2 +-
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 22 ++++++++++++++++++-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index b6ca1f9e4ab78e..0f8f72fd6993fb 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -34,7 +34,7 @@ namespace memspace {
const MemSpaceRegion *MS);
[[nodiscard]] const MemSpaceRegion *getMemSpace(ProgramStateRef State,
- const MemRegion *MR);
+ const MemRegion *MR);
[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index 1dd6568cfa80d2..28749364982545 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -9,6 +9,14 @@
// This file defines getters and setters for the memory space trait which
// associates memory regions with memory spaces in the program state. It also
// defines the MemSpacesMap which maps memory regions to memory spaces.
+//
+// These functions keep the following invariants over the MemSpacesMap:
+// 1. (Temporary as an intermediate step) Memory space traits are only
+// mapped for memory regions that have an unknown memory space
+// 2. Only base memory regions are mapped in the trait
+// 3. Memory regions which have no mapping in the trait are assumed to be
+// unknown; no memory region is allowed to be mapped to an unknown memory
+// space in the trait;
//
//===----------------------------------------------------------------------===//
@@ -24,8 +32,16 @@ namespace clang {
namespace ento {
namespace memspace {
+// Canonicalize to base, in case of subregions, we don't want base regions and subregions
+// to have different memory spaces
+[[nodiscard]] static const MemRegion *canonicalizeMemRegion(const MemRegion *MR) {
+ return MR->getBaseRegion();
+}
+
ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
const MemSpaceRegion *MS) {
+ MR = canonicalizeMemRegion(MR);
+
// For now, this should only be called to update the trait for memory regions
// that have an unknown memory spaces since we assume everywhere else that the
// memory space trait is set only for unknown memory spaces (setting this info
@@ -41,6 +57,8 @@ ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
}
bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR) {
+ MR = canonicalizeMemRegion(MR);
+
if (!isa<UnknownSpaceRegion>(MR->getMemorySpace()))
return false;
@@ -49,7 +67,9 @@ bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR) {
}
const MemSpaceRegion *getMemSpace(ProgramStateRef State,
- const MemRegion *MR) {
+ const MemRegion *MR) {
+ MR = canonicalizeMemRegion(MR);
+
const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
if (Result)
return *Result;
>From c6482c4327fd21de71e3e9591137b49e6f055eb0 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 12:00:06 -0600
Subject: [PATCH 06/14] isMemSpaceOrTrait->isMemSpace
---
.../clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h | 4 ++--
clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp | 4 ++--
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp | 2 +-
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 2 +-
clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index 0f8f72fd6993fb..132261391c2716 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -39,8 +39,8 @@ namespace memspace {
[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
template <typename FirstT, typename... RestT>
-[[nodiscard]] bool isMemSpaceOrTrait(ProgramStateRef State,
- const MemRegion *MR) {
+[[nodiscard]] bool isMemSpace(ProgramStateRef State,
+ const MemRegion *MR) {
return isa<FirstT, RestT...>(MR->getMemorySpace()) ||
isa_and_nonnull<FirstT, RestT...>(getMemSpace(State, MR));
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
index 858b6a37551f5d..c81f1bfa7eaf32 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -80,7 +80,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
// Global variables are fine.
const MemRegion *RB = R->getBaseRegion();
- if (memspace::isMemSpaceOrTrait<GlobalsSpaceRegion>(State, RB))
+ if (memspace::isMemSpace<GlobalsSpaceRegion>(State, RB))
return;
// Handle _dispatch_once. In some versions of the OS X SDK we have the case
@@ -119,7 +119,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
if (IVR != R)
os << " memory within";
os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
- } else if (memspace::isMemSpaceOrTrait<HeapSpaceRegion>(State, RB)) {
+ } else if (memspace::isMemSpace<HeapSpaceRegion>(State, RB)) {
os << " heap-allocated memory";
} else if (isa<UnknownSpaceRegion>(RB->getMemorySpace())) {
// Presence of an IVar superregion has priority over this branch, because
diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index d4ff8186ea69c9..042c4e0c1ab2e6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -561,7 +561,7 @@ MoveChecker::classifyObject(ProgramStateRef State, const MemRegion *MR,
MR = unwrapRValueReferenceIndirection(MR);
bool IsLocal =
isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
- memspace::isMemSpaceOrTrait<StackSpaceRegion>(State, MR);
+ memspace::isMemSpace<StackSpaceRegion>(State, MR);
if (!RD || !RD->getDeclContext()->isStdNamespace())
return { IsLocal, SK_NonStd };
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 43a444e3b8ee84..9f126cd8d3d23c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -143,7 +143,7 @@ StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
for (auto Var : B.referenced_vars()) {
SVal Val = State->getSVal(Var.getCapturedRegion());
const MemRegion *Region = Val.getAsRegion();
- if (Region && memspace::isMemSpaceOrTrait<StackSpaceRegion>(State, Region))
+ if (Region && memspace::isMemSpace<StackSpaceRegion>(State, Region))
Regions.push_back(Region);
}
return Regions;
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 1c6c5fa14bceec..0da562a9fe51db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -412,7 +412,7 @@ void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C,
// because that's likely to be bad news.
ProgramStateRef state = C.getState();
const MemRegion *R = Call.getArgSVal(0).getAsRegion();
- if (!R || !memspace::isMemSpaceOrTrait<StackSpaceRegion>(state, R))
+ if (!R || !memspace::isMemSpace<StackSpaceRegion>(state, R))
return;
ExplodedNode *N = C.generateErrorNode(state);
>From 4dd1432f15428fb05f46f8fc56c2c9d583c80809 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 12:59:00 -0600
Subject: [PATCH 07/14] change MacOSXApiChecker and MallocChecker
---
.../Core/PathSensitive/MemSpaces.h | 5 +----
.../Checkers/MacOSXAPIChecker.cpp | 2 +-
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 21 +++++--------------
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 10 ---------
4 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index 132261391c2716..419d24ac58ef6d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -36,13 +36,10 @@ namespace memspace {
[[nodiscard]] const MemSpaceRegion *getMemSpace(ProgramStateRef State,
const MemRegion *MR);
-[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
-
template <typename FirstT, typename... RestT>
[[nodiscard]] bool isMemSpace(ProgramStateRef State,
const MemRegion *MR) {
- return isa<FirstT, RestT...>(MR->getMemorySpace()) ||
- isa_and_nonnull<FirstT, RestT...>(getMemSpace(State, MR));
+ return isa_and_nonnull<FirstT, RestT...>(getMemSpace(State, MR));
}
} // namespace memspace
diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
index c81f1bfa7eaf32..e8d68e27fdaff3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -121,7 +121,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
} else if (memspace::isMemSpace<HeapSpaceRegion>(State, RB)) {
os << " heap-allocated memory";
- } else if (isa<UnknownSpaceRegion>(RB->getMemorySpace())) {
+ } else if (memspace::isMemSpace<UnknownSpaceRegion>(State, RB)) {
// Presence of an IVar superregion has priority over this branch, because
// ObjC objects are on the heap even if the core doesn't realize this.
// Presence of a block variable base region has priority over this branch,
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 7d13e82472d7bd..7d127facd1a36a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2204,8 +2204,6 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
return nullptr;
}
- const MemSpaceRegion *MS = R->getMemorySpace();
-
// Parameters, locals, statics, globals, and memory returned by
// __builtin_alloca() shouldn't be freed.
// Should skip this check if:
@@ -2213,16 +2211,13 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
// - The region has unknown memspace and no trait for further clarification
// (i.e., just unknown)
// - The region has unknown memspace and a heap memspace trait
- const MemSpaceRegion *MSTrait = memspace::getMemSpace(State, R);
- bool HasHeapOrUnknownTrait = !MSTrait || isa<HeapSpaceRegion>(MSTrait);
- if (!(isa<HeapSpaceRegion>(MS) ||
- (isa<UnknownSpaceRegion>(MS) && HasHeapOrUnknownTrait))) {
+ if (!memspace::isMemSpace<HeapSpaceRegion, UnknownSpaceRegion>(State, R)) {
// Regions returned by malloc() are represented by SymbolicRegion objects
// within HeapSpaceRegion. Of course, free() can work on memory allocated
// outside the current function, so UnknownSpaceRegion is also a
// possibility here.
- if (isa<AllocaRegion>(R) || isa_and_nonnull<AllocaRegion>(MSTrait))
+ if (isa<AllocaRegion>(R))
HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
else
HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
@@ -2413,11 +2408,7 @@ bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
os << "a block";
return true;
default: {
- const MemSpaceRegion *MS = MR->getMemorySpace();
- const MemSpaceRegion *MSTrait = memspace::getMemSpace(State, MR);
-
- if (isa<StackLocalsSpaceRegion>(MS) ||
- isa_and_nonnull<StackLocalsSpaceRegion>(MSTrait)) {
+ if (memspace::isMemSpace<StackLocalsSpaceRegion>(State, MR)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
@@ -2432,8 +2423,7 @@ bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
return true;
}
- if (isa<StackArgumentsSpaceRegion>(MS) ||
- isa_and_nonnull<StackArgumentsSpaceRegion>(MSTrait)) {
+ if (memspace::isMemSpace<StackArgumentsSpaceRegion>(State, MR)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
@@ -2448,8 +2438,7 @@ bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
return true;
}
- if (isa<GlobalsSpaceRegion>(MS) ||
- isa_and_nonnull<GlobalsSpaceRegion>(MSTrait)) {
+ if (memspace::isMemSpace<GlobalsSpaceRegion>(State, MR)) {
const VarRegion *VR = dyn_cast<VarRegion>(MR);
const VarDecl *VD;
if (VR)
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index 28749364982545..e20c1d15e4c518 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -56,16 +56,6 @@ ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
return NewState;
}
-bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR) {
- MR = canonicalizeMemRegion(MR);
-
- if (!isa<UnknownSpaceRegion>(MR->getMemorySpace()))
- return false;
-
- const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
- return Result;
-}
-
const MemSpaceRegion *getMemSpace(ProgramStateRef State,
const MemRegion *MR) {
MR = canonicalizeMemRegion(MR);
>From 2f2730b0d6c634c732358e9fa081f86c3c5d36ef Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:21:31 -0600
Subject: [PATCH 08/14] PutenvStackArrayChecker converted
---
.../lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
index fddef2ee0ac161..455cb6cc37a1fe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
@@ -48,10 +48,7 @@ void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
const MemRegion *MR = ArgV.getAsRegion();
- const auto *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
- if (!SSR)
- SSR = dyn_cast_if_present<StackSpaceRegion>(
- memspace::getMemSpace(C.getState(), MR));
+ const StackSpaceRegion *SSR = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(C.getState(), MR));
if (!SSR)
return;
>From 7f7c274078a90d67299c7144257c32928b2d96da Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:21:45 -0600
Subject: [PATCH 09/14] ExprEngine
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 82274f3327f16d..1a056d8fc5970c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3518,12 +3518,9 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
}
// Case (2) continued.
- if (isa<UnknownSpaceRegion>(MR)) {
- const MemSpaceRegion *MS = memspace::getMemSpace(State, MR);
- if (!isa<StackSpaceRegion, StaticGlobalSpaceRegion>(MS)) {
- Escaped.push_back(LocAndVal.second);
- continue;
- }
+ if (memspace::isMemSpace<StackSpaceRegion, StaticGlobalSpaceRegion>(State, MR)) {
+ Escaped.push_back(LocAndVal.second);
+ continue;
}
// Case (3).
>From 8fb29b7644c0ed25d6b5ce92bcee21e92fe6c34f Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:21:58 -0600
Subject: [PATCH 10/14] BugReporterVisitors
---
.../lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index ad2429d61de927..458d3e545ccd2f 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1194,16 +1194,8 @@ static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
return false;
const MemSpaceRegion *VarSpace = VR->getMemorySpace();
- const StackSpaceRegion *FrameSpace;
-
- if (isa<UnknownSpaceRegion>(VarSpace)) {
- ProgramStateRef State = N->getState();
- const MemSpaceRegion *MemSpace = memspace::getMemSpace(State, VR);
- FrameSpace = dyn_cast_if_present<StackSpaceRegion>(MemSpace);
- } else {
- FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace);
- }
-
+ const StackSpaceRegion *FrameSpace = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(N->getState(), VR));
+
if (!FrameSpace) {
// If we ever directly evaluate global DeclStmts, this assertion will be
// invalid, but this still seems preferable to silently accepting an
>From 5503e7ad396a94a64e11f371d88fb7caedfd9230 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:22:12 -0600
Subject: [PATCH 11/14] StackAddrEscapeChecker
---
.../StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 9f126cd8d3d23c..25323b6bea3d4d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -267,13 +267,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
checkReturnedBlockCaptures(*B, C);
- if (isa<UnknownSpaceRegion>(R)) {
- const MemSpaceRegion *MS = memspace::getMemSpace(C.getState(), R);
- if (!isa_and_nonnull<StackSpaceRegion>(MS) || isNotInCurrentFrame(MS, C))
- return;
- }
-
- const MemSpaceRegion *MS = R->getMemorySpace();
+ const MemSpaceRegion *MS = memspace::getMemSpace(C.getState(), R);
if (!isa<StackSpaceRegion>(MS) || isNotInCurrentFrame(MS, C))
return;
>From a51d7576945d539483c8b17feefba3d8c34fa767 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:22:26 -0600
Subject: [PATCH 12/14] RetainCountDiagnostics
---
.../RetainCountChecker/RetainCountDiagnostics.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 80fa8d3e3f921e..0d4a102538a6d1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -691,13 +691,9 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
const MemRegion *R = FB.getRegion();
// Do not show local variables belonging to a function other than
// where the error is reported.
- const StackSpaceRegion *MR =
- dyn_cast<StackSpaceRegion>(R->getMemorySpace());
- if (!MR)
- MR = dyn_cast_if_present<StackSpaceRegion>(
- memspace::getMemSpace(St, R));
+ const StackSpaceRegion *SSR = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(St, R));
- if (MR && MR->getStackFrame() == LeakContext->getStackFrame())
+ if (SSR && SSR->getStackFrame() == LeakContext->getStackFrame())
FirstBinding = R;
}
>From c67671e27d593147691fa9f7ffdd71f131b19a32 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 13:52:02 -0600
Subject: [PATCH 13/14] ExprEngine remove redundant added case, invert
condition bugfix
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 1a056d8fc5970c..7a9177b3bcc770 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3512,13 +3512,7 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
// Cases (1) and (2).
const MemRegion *MR = LocAndVal.first.getAsRegion();
if (!MR ||
- !isa<StackSpaceRegion, StaticGlobalSpaceRegion>(MR->getMemorySpace())) {
- Escaped.push_back(LocAndVal.second);
- continue;
- }
-
- // Case (2) continued.
- if (memspace::isMemSpace<StackSpaceRegion, StaticGlobalSpaceRegion>(State, MR)) {
+ !memspace::isMemSpace<StackSpaceRegion, StaticGlobalSpaceRegion>(State, MR)) {
Escaped.push_back(LocAndVal.second);
continue;
}
>From 80b9b62e068aed8f53214086432e2e45b92981e0 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 15 Jan 2025 14:02:55 -0600
Subject: [PATCH 14/14] formatting
---
.../StaticAnalyzer/Core/PathSensitive/MemSpaces.h | 3 +--
.../Checkers/PutenvStackArrayChecker.cpp | 3 ++-
.../RetainCountChecker/RetainCountDiagnostics.cpp | 3 ++-
.../lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 5 +++--
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 4 ++--
clang/lib/StaticAnalyzer/Core/MemSpaces.cpp | 12 ++++++------
6 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
index 419d24ac58ef6d..8d6679545a8266 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h
@@ -37,8 +37,7 @@ namespace memspace {
const MemRegion *MR);
template <typename FirstT, typename... RestT>
-[[nodiscard]] bool isMemSpace(ProgramStateRef State,
- const MemRegion *MR) {
+[[nodiscard]] bool isMemSpace(ProgramStateRef State, const MemRegion *MR) {
return isa_and_nonnull<FirstT, RestT...>(getMemSpace(State, MR));
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
index 455cb6cc37a1fe..a200f54a97b8c6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp
@@ -48,7 +48,8 @@ void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
const MemRegion *MR = ArgV.getAsRegion();
- const StackSpaceRegion *SSR = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(C.getState(), MR));
+ const StackSpaceRegion *SSR = dyn_cast_if_present<StackSpaceRegion>(
+ memspace::getMemSpace(C.getState(), MR));
if (!SSR)
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 0d4a102538a6d1..b3a3aeac3a99e8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -691,7 +691,8 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
const MemRegion *R = FB.getRegion();
// Do not show local variables belonging to a function other than
// where the error is reported.
- const StackSpaceRegion *SSR = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(St, R));
+ const StackSpaceRegion *SSR =
+ dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(St, R));
if (SSR && SSR->getStackFrame() == LeakContext->getStackFrame())
FirstBinding = R;
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 458d3e545ccd2f..83befeb99b5f28 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1194,8 +1194,9 @@ static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
return false;
const MemSpaceRegion *VarSpace = VR->getMemorySpace();
- const StackSpaceRegion *FrameSpace = dyn_cast_if_present<StackSpaceRegion>(memspace::getMemSpace(N->getState(), VR));
-
+ const StackSpaceRegion *FrameSpace = dyn_cast_if_present<StackSpaceRegion>(
+ memspace::getMemSpace(N->getState(), VR));
+
if (!FrameSpace) {
// If we ever directly evaluate global DeclStmts, this assertion will be
// invalid, but this still seems preferable to silently accepting an
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7a9177b3bcc770..eabadfddedebbb 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3511,8 +3511,8 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
for (const std::pair<SVal, SVal> &LocAndVal : LocAndVals) {
// Cases (1) and (2).
const MemRegion *MR = LocAndVal.first.getAsRegion();
- if (!MR ||
- !memspace::isMemSpace<StackSpaceRegion, StaticGlobalSpaceRegion>(State, MR)) {
+ if (!MR || !memspace::isMemSpace<StackSpaceRegion, StaticGlobalSpaceRegion>(
+ State, MR)) {
Escaped.push_back(LocAndVal.second);
continue;
}
diff --git a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
index e20c1d15e4c518..25846527264ab6 100644
--- a/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemSpaces.cpp
@@ -9,7 +9,7 @@
// This file defines getters and setters for the memory space trait which
// associates memory regions with memory spaces in the program state. It also
// defines the MemSpacesMap which maps memory regions to memory spaces.
-//
+//
// These functions keep the following invariants over the MemSpacesMap:
// 1. (Temporary as an intermediate step) Memory space traits are only
// mapped for memory regions that have an unknown memory space
@@ -32,9 +32,10 @@ namespace clang {
namespace ento {
namespace memspace {
-// Canonicalize to base, in case of subregions, we don't want base regions and subregions
-// to have different memory spaces
-[[nodiscard]] static const MemRegion *canonicalizeMemRegion(const MemRegion *MR) {
+// Canonicalize to base, in case of subregions, we don't want base regions and
+// subregions to have different memory spaces
+[[nodiscard]] static const MemRegion *
+canonicalizeMemRegion(const MemRegion *MR) {
return MR->getBaseRegion();
}
@@ -56,8 +57,7 @@ ProgramStateRef setMemSpaceTrait(ProgramStateRef State, const MemRegion *MR,
return NewState;
}
-const MemSpaceRegion *getMemSpace(ProgramStateRef State,
- const MemRegion *MR) {
+const MemSpaceRegion *getMemSpace(ProgramStateRef State, const MemRegion *MR) {
MR = canonicalizeMemRegion(MR);
const MemSpaceRegion *const *Result = State->get<MemSpacesMap>(MR);
More information about the cfe-commits
mailing list