Skip to content

Commit cd8578d

Browse files
committed
[CIR][Lifetime] Detect underlying changes in Owner types and invalidate tainted Ptr usage
1 parent f793f37 commit cd8578d

File tree

2 files changed

+63
-7
lines changed

2 files changed

+63
-7
lines changed

clang/lib/CIR/Dialect/Transforms/LifetimeCheck.cpp

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ struct LifetimeCheckPass : public LifetimeCheckBase<LifetimeCheckPass> {
169169

170170
// Provides p1179's 'KILL' functionality. See implementation for more
171171
// information.
172-
void kill(const State &s);
172+
void kill(const State &s, llvm::Optional<mlir::Location> killLoc = {});
173173
void killInPset(PSetType &pset, const State &s);
174174

175175
// Local pointers
@@ -306,7 +306,8 @@ void LifetimeCheckPass::killInPset(PSetType &pset, const State &s) {
306306
// in the pmap with invalid. For example, if pmap is {(p1,{a}), (p2,{a'})},
307307
// KILL(a') would invalidate only p2, and KILL(a) would invalidate both p1 and
308308
// p2.
309-
void LifetimeCheckPass::kill(const State &s) {
309+
void LifetimeCheckPass::kill(const State &s,
310+
llvm::Optional<mlir::Location> killLoc) {
310311
assert(s.hasValue() && "does not know how to kill other data types");
311312
mlir::Value v = s.getData();
312313
for (auto &mapEntry : getPmap()) {
@@ -321,12 +322,23 @@ void LifetimeCheckPass::kill(const State &s) {
321322
//
322323
auto &pset = mapEntry.second;
323324

325+
// Record if pmap(ptr) is invalid already.
326+
bool wasInvalid = pset.count(State::getInvalid());
327+
324328
// FIXME: add x'', x''', etc...
325329
if (s.isLocalValue() && owners.count(v))
326330
killInPset(pset, State::getOwnedBy(v));
327331

328332
killInPset(pset, s);
329-
pmapInvalidHist[ptr] = std::make_pair(getEndLocForHist(*currScope), v);
333+
334+
// If pset(ptr) was already invalid, do not polute the history.
335+
if (!wasInvalid) {
336+
// FIXME: support invalidation history and types.
337+
if (!killLoc)
338+
pmapInvalidHist[ptr] = std::make_pair(getEndLocForHist(*currScope), v);
339+
else
340+
pmapInvalidHist[ptr] = std::make_pair(killLoc, llvm::None);
341+
}
330342
}
331343

332344
// Delete the local value from pmap, since its now gone.
@@ -954,6 +966,14 @@ static bool isOperatorStar(const clang::CXXMethodDecl *m) {
954966
return m->getOverloadedOperator() == clang::OverloadedOperatorKind::OO_Star;
955967
}
956968

969+
static bool sinkUnsupportedOperator(const clang::CXXMethodDecl *m) {
970+
if (!m->isOverloadedOperator())
971+
return false;
972+
if (!isOperatorStar(m))
973+
llvm_unreachable("NYI");
974+
return false;
975+
}
976+
957977
void LifetimeCheckPass::checkOperatorStar(CallOp callOp) {
958978
auto addr = callOp.getOperand(0);
959979
if (!ptrs.count(addr))
@@ -980,22 +1000,45 @@ void LifetimeCheckPass::checkCall(CallOp callOp) {
9801000
if (!methodDecl)
9811001
return;
9821002

983-
// TODO: only ctor init implemented, assign ops and others needed.
9841003
if (auto ctor = dyn_cast<clang::CXXConstructorDecl>(methodDecl))
9851004
return checkCtor(callOp, ctor);
9861005
if (methodDecl->isMoveAssignmentOperator())
9871006
return checkMoveAssignment(callOp, methodDecl);
1007+
if (methodDecl->isCopyAssignmentOperator())
1008+
llvm_unreachable("NYI");
9881009
if (isOperatorStar(methodDecl))
9891010
return checkOperatorStar(callOp);
1011+
if (sinkUnsupportedOperator(methodDecl))
1012+
return;
9901013

9911014
// For any other methods...
9921015

9931016
// Non-const member call to a Owner invalidates any of its users.
9941017
if (isNonConstUseOfOwner(callOp, methodDecl)) {
995-
// auto addr = callOp.getOperand(0);
996-
// TODO: kill(a')
997-
llvm_unreachable("NYI");
1018+
auto addr = callOp.getOperand(0);
1019+
// 2.4.2 - On every non-const use of a local Owner o:
1020+
//
1021+
// - For each entry e in pset(s): Remove e from pset(s), and if no other
1022+
// Owner’s pset contains only e, then KILL(e).
1023+
kill(State::getOwnedBy(addr), callOp.getLoc());
1024+
1025+
// - Set pset(o) = {o__N'}, where N is one higher than the highest
1026+
// previously used suffix. For example, initially pset(o) is {o__1'}, on
1027+
// o’s first non-const use pset(o) becomes {o__2'}, on o’s second non-const
1028+
// use pset(o) becomes {o__3'}, and so on.
1029+
// FIXME: for now we set pset(o) = { invalid }
1030+
auto &pset = getPmap()[addr];
1031+
pset.clear();
1032+
pset.insert(State::getInvalid());
1033+
pmapInvalidHist[addr] = std::make_pair(callOp.getLoc(), llvm::None);
1034+
return;
9981035
}
1036+
1037+
// Take a pset(Ptr) = { Ownr' } where Own got invalidated, this will become
1038+
// invalid access to Ptr if any of its methods are used.
1039+
auto addr = callOp.getOperand(0);
1040+
if (ptrs.count(addr))
1041+
return checkPointerDeref(addr, callOp.getLoc());
9991042
}
10001043

10011044
void LifetimeCheckPass::checkOperation(Operation *op) {

clang/test/CIR/Transforms/lifetime-check-owner.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
struct [[gsl::Owner(int)]] MyIntOwner {
44
int val;
55
MyIntOwner(int v) : val(v) {}
6+
void changeInt(int i);
67
int &operator*();
8+
int read() const;
79
};
810

911
struct [[gsl::Pointer(int)]] MyIntPointer {
@@ -12,6 +14,7 @@ struct [[gsl::Pointer(int)]] MyIntPointer {
1214
MyIntPointer(const MyIntOwner &);
1315
int &operator*();
1416
MyIntOwner toOwner();
17+
int read() { return *ptr; }
1518
};
1619

1720
void yolo() {
@@ -23,4 +26,14 @@ void yolo() {
2326
} // expected-note {{pointee 'o' invalidated at end of scope}}
2427
*p = 4; // expected-warning {{use of invalid pointer 'p'}}
2528
// expected-remark@-1 {{pset => { invalid }}}
29+
}
30+
31+
void yolo2() {
32+
MyIntPointer p;
33+
MyIntOwner o(1);
34+
p = o;
35+
(void)o.read();
36+
o.changeInt(42); // expected-note {{uninitialized here}}
37+
(void)p.read(); // expected-warning {{use of invalid pointer 'p'}}
38+
// expected-remark@-1 {{pset => { invalid }}}
2639
}

0 commit comments

Comments
 (0)