Skip to content

Commit aae6e66

Browse files
committed
[CIR][Lifetime] Detect dangling pointers on Owner types
First example now works and we can successfully diagnose a basic dangling ref to owner type. - Handle kills for references owners o', but kill(o') not yet implemented, should be done together with its own use/test case. - Make sure invalidating moved-from adds proper state. - Add testcase.
1 parent d04788c commit aae6e66

File tree

2 files changed

+57
-12
lines changed

2 files changed

+57
-12
lines changed

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -297,18 +297,34 @@ void LifetimeCheckPass::LexicalScopeGuard::cleanup() {
297297
if (pointee == ptr)
298298
continue;
299299

300-
// If the local value is part of this pset, it means
301-
// we need to invalidate it, otherwise keep searching.
302-
// FIXME: add support for x', x'', etc...
300+
// If the local value is part of this pset, it means we need to invalidate
301+
// it, otherwise keep searching. Note that this assumes a current pset
302+
// cannot have multiple entries for values and owned values at the same
303+
// time, for example this should not be possible: pset(s) = {o, o'}.
303304
auto &pset = mapEntry.second;
304-
State valState = State::getLocalValue(pointee);
305-
if (!pset.contains(valState))
306-
continue;
307-
308-
// Erase the reference and mark this invalid.
309-
// FIXME: add a way to just mutate the state.
310-
pset.erase(valState);
311-
pset.insert(State::getInvalid());
305+
auto killValueInPset = [&](mlir::Value v) {
306+
State valState = State::getLocalValue(v);
307+
if (pset.contains(valState)) {
308+
// Erase the reference and mark this invalid.
309+
// FIXME: add a way to just mutate the state.
310+
pset.erase(valState);
311+
pset.insert(State::getInvalid());
312+
return;
313+
}
314+
315+
if (Pass.owners.count(v)) {
316+
valState = State::getOwnedBy(v);
317+
if (pset.contains(valState)) {
318+
pset.erase(valState);
319+
pset.insert(State::getInvalid());
320+
return;
321+
}
322+
// TODO: o'', ...
323+
}
324+
};
325+
326+
// KILL(x) for a particular pset.
327+
killValueInPset(pointee);
312328
Pass.pmapInvalidHist[ptr] =
313329
std::make_pair(getEndLocForHist(*Pass.currScope), pointee);
314330
}
@@ -846,7 +862,10 @@ void LifetimeCheckPass::checkMoveAssignment(CallOp callOp,
846862
// where we finally materialize it back to the original pointer category.
847863
// TODO: should CIR ops retain xvalue information somehow?
848864
getPmap()[dst] = getPmap()[src];
849-
getPmap()[src].clear(); // TODO: should we add null to 'src' pset?
865+
// TODO: should this be null? or should we swap dst/src pset state?
866+
// For now just consider moved-from state as invalid.
867+
getPmap()[src].clear();
868+
getPmap()[src].insert(State::getInvalid());
850869
}
851870

852871
// User defined ctors that initialize from owner types is one
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir-enable -fclangir-lifetime-check="history=all;remarks=all" -clangir-verify-diagnostics -emit-cir %s -o %t.cir
2+
3+
struct [[gsl::Owner(int)]] MyIntOwner {
4+
int val;
5+
MyIntOwner(int v) : val(v) {}
6+
int &operator*();
7+
};
8+
9+
struct [[gsl::Pointer(int)]] MyIntPointer {
10+
int *ptr;
11+
MyIntPointer(int *p = nullptr) : ptr(p) {}
12+
MyIntPointer(const MyIntOwner &);
13+
int &operator*();
14+
MyIntOwner toOwner();
15+
};
16+
17+
void yolo() {
18+
MyIntPointer p;
19+
{
20+
MyIntOwner o(1);
21+
p = o;
22+
*p = 3; // expected-remark {{pset => { o' }}}
23+
} // expected-note {{pointee 'o' invalidated at end of scope}}
24+
*p = 4; // expected-warning {{use of invalid pointer 'p'}}
25+
// expected-remark@-1 {{pset => { invalid }}}
26+
}

0 commit comments

Comments
 (0)