Skip to content

Commit 7e0758d

Browse files
committed
[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.
1 parent 3f1a391 commit 7e0758d

File tree

13 files changed

+221
-46
lines changed

13 files changed

+221
-46
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
//===-- MemSpaces.h -----------------------------------------------*- C++ -*--//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// TODO
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H
14+
#define LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H
15+
16+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
17+
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
18+
19+
namespace clang {
20+
namespace ento {
21+
22+
class MemRegion;
23+
class MemSpaceRegion;
24+
25+
namespace memspace {
26+
27+
[[nodiscard]] ProgramStateRef setMemSpaceTrait(ProgramStateRef State,
28+
const MemRegion *MR,
29+
const MemSpaceRegion *MS);
30+
31+
[[nodiscard]] const MemSpaceRegion *getMemSpaceTrait(ProgramStateRef State,
32+
const MemRegion *MR);
33+
34+
[[nodiscard]] bool hasMemSpaceTrait(ProgramStateRef State, const MemRegion *MR);
35+
36+
template <typename FirstT, typename... RestT>
37+
[[nodiscard]] bool isMemSpaceOrTrait(ProgramStateRef State,
38+
const MemRegion *MR) {
39+
return isa<FirstT, RestT...>(MR->getMemorySpace()) ||
40+
isa_and_nonnull<FirstT, RestT...>(getMemSpaceTrait(State, MR));
41+
}
42+
43+
} // namespace memspace
44+
} // namespace ento
45+
} // namespace clang
46+
47+
#endif // LLVM_CLANG_STATICANALYZER_CHECKERS_MEMSPACES_H

clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
2424
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
25+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
2526
#include "llvm/ADT/APSInt.h"
2627
#include "llvm/ADT/SmallString.h"
2728
#include "llvm/Support/FormatVariadic.h"

clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "clang/StaticAnalyzer/Core/Checker.h"
2323
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
2424
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
25+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
2526
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
2627
#include "llvm/ADT/SmallString.h"
2728
#include "llvm/ADT/StringSwitch.h"
@@ -75,10 +76,11 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
7576
if (!R)
7677
return;
7778

79+
ProgramStateRef State = C.getState();
80+
7881
// Global variables are fine.
7982
const MemRegion *RB = R->getBaseRegion();
80-
const MemSpaceRegion *RS = RB->getMemorySpace();
81-
if (isa<GlobalsSpaceRegion>(RS))
83+
if (memspace::isMemSpaceOrTrait<GlobalsSpaceRegion>(State, RB))
8284
return;
8385

8486
// 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,
117119
if (IVR != R)
118120
os << " memory within";
119121
os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
120-
} else if (isa<HeapSpaceRegion>(RS)) {
122+
} else if (memspace::isMemSpaceOrTrait<HeapSpaceRegion>(State, RB)) {
121123
os << " heap-allocated memory";
122-
} else if (isa<UnknownSpaceRegion>(RS)) {
124+
} else if (isa<UnknownSpaceRegion>(RB->getMemorySpace())) {
123125
// Presence of an IVar superregion has priority over this branch, because
124126
// ObjC objects are on the heap even if the core doesn't realize this.
125127
// Presence of a block variable base region has priority over this branch,

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
7373
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
7474
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
75+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
7576
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
7677
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
7778
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
@@ -784,7 +785,8 @@ class MallocChecker
784785
bool IsALeakCheck = false) const;
785786
///@}
786787
static bool SummarizeValue(raw_ostream &os, SVal V);
787-
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
788+
static bool SummarizeRegion(ProgramStateRef State, raw_ostream &os,
789+
const MemRegion *MR);
788790

789791
void HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal, SourceRange Range,
790792
const Expr *DeallocExpr,
@@ -2206,13 +2208,21 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
22062208

22072209
// Parameters, locals, statics, globals, and memory returned by
22082210
// __builtin_alloca() shouldn't be freed.
2209-
if (!isa<UnknownSpaceRegion, HeapSpaceRegion>(MS)) {
2211+
// Should skip this check if:
2212+
// - The region is in the heap
2213+
// - The region has unknown memspace and no trait for further clarification
2214+
// (i.e., just unknown)
2215+
// - The region has unknown memspace and a heap memspace trait
2216+
const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, R);
2217+
bool HasHeapOrUnknownTrait = !MSTrait || isa<HeapSpaceRegion>(MSTrait);
2218+
if (!(isa<HeapSpaceRegion>(MS) ||
2219+
(isa<UnknownSpaceRegion>(MS) && HasHeapOrUnknownTrait))) {
22102220
// Regions returned by malloc() are represented by SymbolicRegion objects
22112221
// within HeapSpaceRegion. Of course, free() can work on memory allocated
22122222
// outside the current function, so UnknownSpaceRegion is also a
22132223
// possibility here.
22142224

2215-
if (isa<AllocaRegion>(R))
2225+
if (isa<AllocaRegion>(R) || isa_and_nonnull<AllocaRegion>(MSTrait))
22162226
HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
22172227
else
22182228
HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
@@ -2384,7 +2394,7 @@ bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
23842394
return true;
23852395
}
23862396

2387-
bool MallocChecker::SummarizeRegion(raw_ostream &os,
2397+
bool MallocChecker::SummarizeRegion(ProgramStateRef State, raw_ostream &os,
23882398
const MemRegion *MR) {
23892399
switch (MR->getKind()) {
23902400
case MemRegion::FunctionCodeRegionKind: {
@@ -2404,8 +2414,10 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
24042414
return true;
24052415
default: {
24062416
const MemSpaceRegion *MS = MR->getMemorySpace();
2417+
const MemSpaceRegion *MSTrait = memspace::getMemSpaceTrait(State, MR);
24072418

2408-
if (isa<StackLocalsSpaceRegion>(MS)) {
2419+
if (isa<StackLocalsSpaceRegion>(MS) ||
2420+
isa_and_nonnull<StackLocalsSpaceRegion>(MSTrait)) {
24092421
const VarRegion *VR = dyn_cast<VarRegion>(MR);
24102422
const VarDecl *VD;
24112423
if (VR)
@@ -2420,7 +2432,8 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
24202432
return true;
24212433
}
24222434

2423-
if (isa<StackArgumentsSpaceRegion>(MS)) {
2435+
if (isa<StackArgumentsSpaceRegion>(MS) ||
2436+
isa_and_nonnull<StackArgumentsSpaceRegion>(MSTrait)) {
24242437
const VarRegion *VR = dyn_cast<VarRegion>(MR);
24252438
const VarDecl *VD;
24262439
if (VR)
@@ -2435,7 +2448,8 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
24352448
return true;
24362449
}
24372450

2438-
if (isa<GlobalsSpaceRegion>(MS)) {
2451+
if (isa<GlobalsSpaceRegion>(MS) ||
2452+
isa_and_nonnull<GlobalsSpaceRegion>(MSTrait)) {
24392453
const VarRegion *VR = dyn_cast<VarRegion>(MR);
24402454
const VarDecl *VD;
24412455
if (VR)
@@ -2489,8 +2503,8 @@ void MallocChecker::HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal,
24892503
os << "deallocator";
24902504

24912505
os << " is ";
2492-
bool Summarized = MR ? SummarizeRegion(os, MR)
2493-
: SummarizeValue(os, ArgVal);
2506+
bool Summarized =
2507+
MR ? SummarizeRegion(C.getState(), os, MR) : SummarizeValue(os, ArgVal);
24942508
if (Summarized)
24952509
os << ", which is not memory allocated by ";
24962510
else

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
2424
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
25+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
2526
#include "llvm/ADT/StringSet.h"
2627

2728
using namespace clang;
@@ -145,12 +146,14 @@ class MoveChecker
145146

146147
// Obtains ObjectKind of an object. Because class declaration cannot always
147148
// be easily obtained from the memory region, it is supplied separately.
148-
ObjectKind classifyObject(const MemRegion *MR, const CXXRecordDecl *RD) const;
149+
ObjectKind classifyObject(ProgramStateRef State, const MemRegion *MR,
150+
const CXXRecordDecl *RD) const;
149151

150152
// Classifies the object and dumps a user-friendly description string to
151153
// the stream.
152-
void explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
153-
const CXXRecordDecl *RD, MisuseKind MK) const;
154+
void explainObject(ProgramStateRef State, llvm::raw_ostream &OS,
155+
const MemRegion *MR, const CXXRecordDecl *RD,
156+
MisuseKind MK) const;
154157

155158
bool belongsTo(const CXXRecordDecl *RD, const llvm::StringSet<> &Set) const;
156159

@@ -299,12 +302,12 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
299302
SmallString<128> Str;
300303
llvm::raw_svector_ostream OS(Str);
301304

302-
ObjectKind OK = Chk.classifyObject(Region, RD);
305+
ObjectKind OK = Chk.classifyObject(State, Region, RD);
303306
switch (OK.StdKind) {
304307
case SK_SmartPtr:
305308
if (MK == MK_Dereference) {
306309
OS << "Smart pointer";
307-
Chk.explainObject(OS, Region, RD, MK);
310+
Chk.explainObject(State, OS, Region, RD, MK);
308311
OS << " is reset to null when moved from";
309312
break;
310313
}
@@ -315,12 +318,12 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
315318
case SK_NonStd:
316319
case SK_Safe:
317320
OS << "Object";
318-
Chk.explainObject(OS, Region, RD, MK);
321+
Chk.explainObject(State, OS, Region, RD, MK);
319322
OS << " is moved";
320323
break;
321324
case SK_Unsafe:
322325
OS << "Object";
323-
Chk.explainObject(OS, Region, RD, MK);
326+
Chk.explainObject(State, OS, Region, RD, MK);
324327
OS << " is left in a valid but unspecified state after move";
325328
break;
326329
}
@@ -353,7 +356,7 @@ void MoveChecker::modelUse(ProgramStateRef State, const MemRegion *Region,
353356
CheckerContext &C) const {
354357
assert(!C.isDifferent() && "No transitions should have been made by now");
355358
const RegionState *RS = State->get<TrackedRegionMap>(Region);
356-
ObjectKind OK = classifyObject(Region, RD);
359+
ObjectKind OK = classifyObject(State, Region, RD);
357360

358361
// Just in case: if it's not a smart pointer but it does have operator *,
359362
// we shouldn't call the bug a dereference.
@@ -406,24 +409,25 @@ ExplodedNode *MoveChecker::tryToReportBug(const MemRegion *Region,
406409
// Creating the error message.
407410
llvm::SmallString<128> Str;
408411
llvm::raw_svector_ostream OS(Str);
412+
ProgramStateRef ErrorNodeState = N->getState();
409413
switch(MK) {
410414
case MK_FunCall:
411415
OS << "Method called on moved-from object";
412-
explainObject(OS, Region, RD, MK);
416+
explainObject(ErrorNodeState, OS, Region, RD, MK);
413417
break;
414418
case MK_Copy:
415419
OS << "Moved-from object";
416-
explainObject(OS, Region, RD, MK);
420+
explainObject(ErrorNodeState, OS, Region, RD, MK);
417421
OS << " is copied";
418422
break;
419423
case MK_Move:
420424
OS << "Moved-from object";
421-
explainObject(OS, Region, RD, MK);
425+
explainObject(ErrorNodeState, OS, Region, RD, MK);
422426
OS << " is moved";
423427
break;
424428
case MK_Dereference:
425429
OS << "Dereference of null smart pointer";
426-
explainObject(OS, Region, RD, MK);
430+
explainObject(ErrorNodeState, OS, Region, RD, MK);
427431
break;
428432
}
429433

@@ -482,7 +486,7 @@ void MoveChecker::checkPostCall(const CallEvent &Call,
482486
return;
483487

484488
const CXXRecordDecl *RD = MethodDecl->getParent();
485-
ObjectKind OK = classifyObject(ArgRegion, RD);
489+
ObjectKind OK = classifyObject(State, ArgRegion, RD);
486490
if (shouldBeTracked(OK)) {
487491
// Mark object as moved-from.
488492
State = State->set<TrackedRegionMap>(ArgRegion, RegionState::getMoved());
@@ -549,15 +553,15 @@ bool MoveChecker::belongsTo(const CXXRecordDecl *RD,
549553
}
550554

551555
MoveChecker::ObjectKind
552-
MoveChecker::classifyObject(const MemRegion *MR,
556+
MoveChecker::classifyObject(ProgramStateRef State, const MemRegion *MR,
553557
const CXXRecordDecl *RD) const {
554558
// Local variables and local rvalue references are classified as "Local".
555559
// For the purposes of this checker, we classify move-safe STL types
556560
// as not-"STL" types, because that's how the checker treats them.
557561
MR = unwrapRValueReferenceIndirection(MR);
558562
bool IsLocal =
559563
isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
560-
isa<StackSpaceRegion>(MR->getMemorySpace());
564+
memspace::isMemSpaceOrTrait<StackSpaceRegion>(State, MR);
561565

562566
if (!RD || !RD->getDeclContext()->isStdNamespace())
563567
return { IsLocal, SK_NonStd };
@@ -571,8 +575,9 @@ MoveChecker::classifyObject(const MemRegion *MR,
571575
return { IsLocal, SK_Unsafe };
572576
}
573577

574-
void MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
575-
const CXXRecordDecl *RD, MisuseKind MK) const {
578+
void MoveChecker::explainObject(ProgramStateRef State, llvm::raw_ostream &OS,
579+
const MemRegion *MR, const CXXRecordDecl *RD,
580+
MisuseKind MK) const {
576581
// We may need a leading space every time we actually explain anything,
577582
// and we never know if we are to explain anything until we try.
578583
if (const auto DR =
@@ -581,7 +586,7 @@ void MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
581586
OS << " '" << RegionDecl->getDeclName() << "'";
582587
}
583588

584-
ObjectKind OK = classifyObject(MR, RD);
589+
ObjectKind OK = classifyObject(State, MR, RD);
585590
switch (OK.StdKind) {
586591
case SK_NonStd:
587592
case SK_Safe:

clang/lib/StaticAnalyzer/Checkers/PutenvStackArrayChecker.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
2222
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
24+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
2425

2526
using namespace clang;
2627
using namespace ento;
@@ -45,10 +46,15 @@ void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
4546
SVal ArgV = Call.getArgSVal(0);
4647
const Expr *ArgExpr = Call.getArgExpr(0);
4748

48-
const auto *SSR =
49-
dyn_cast<StackSpaceRegion>(ArgV.getAsRegion()->getMemorySpace());
49+
const MemRegion *MR = ArgV.getAsRegion();
50+
51+
const auto *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
52+
if (!SSR)
53+
SSR = dyn_cast_if_present<StackSpaceRegion>(
54+
memspace::getMemSpaceTrait(C.getState(), MR));
5055
if (!SSR)
5156
return;
57+
5258
const auto *StackFrameFuncD =
5359
dyn_cast_or_null<FunctionDecl>(SSR->getStackFrame()->getDecl());
5460
if (StackFrameFuncD && StackFrameFuncD->isMain())

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include "RetainCountDiagnostics.h"
1515
#include "RetainCountChecker.h"
16+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemSpaces.h"
1617
#include "llvm/ADT/STLExtras.h"
1718
#include "llvm/ADT/SmallVector.h"
1819
#include <optional>
@@ -690,9 +691,14 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
690691
const MemRegion *R = FB.getRegion();
691692
// Do not show local variables belonging to a function other than
692693
// where the error is reported.
693-
if (auto MR = dyn_cast<StackSpaceRegion>(R->getMemorySpace()))
694-
if (MR->getStackFrame() == LeakContext->getStackFrame())
695-
FirstBinding = R;
694+
const StackSpaceRegion *MR =
695+
dyn_cast<StackSpaceRegion>(R->getMemorySpace());
696+
if (!MR)
697+
MR = dyn_cast_if_present<StackSpaceRegion>(
698+
memspace::getMemSpaceTrait(St, R));
699+
700+
if (MR && MR->getStackFrame() == LeakContext->getStackFrame())
701+
FirstBinding = R;
696702
}
697703

698704
// AllocationNode is the last node in which the symbol was tracked.

0 commit comments

Comments
 (0)