Skip to content

Commit 14ec176

Browse files
committed
[LifetimeSafety] Track gsl::Pointer types
1 parent ec4b3ed commit 14ec176

File tree

2 files changed

+207
-19
lines changed

2 files changed

+207
-19
lines changed

clang/lib/Analysis/LifetimeSafety.cpp

Lines changed: 130 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,13 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
419419
VisitedStmts.clear();
420420
for (unsigned I = 0; I < Block->size(); ++I) {
421421
const CFGElement &Element = Block->Elements[I];
422-
if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
422+
if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) {
423+
DEBUG_WITH_TYPE("PrintCFG", llvm::dbgs() << " ================== \n");
424+
DEBUG_WITH_TYPE("PrintCFG", CS->dump());
425+
DEBUG_WITH_TYPE("PrintCFG", CS->getStmt()->dumpColor());
423426
Visit(CS->getStmt());
424-
else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
425-
Element.getAs<CFGAutomaticObjDtor>())
427+
} else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
428+
Element.getAs<CFGAutomaticObjDtor>())
426429
handleDestructor(*DtorOpt);
427430
}
428431
FactMgr.addBlockFacts(Block, CurrentBlockFacts);
@@ -443,6 +446,31 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
443446
addAssignOriginFact(*VD, *InitExpr);
444447
}
445448

449+
void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
450+
if (!isGslPointerType(CCE->getType()))
451+
return;
452+
453+
if (CCE->getNumArgs() > 0 && hasOrigin(CCE->getArg(0)->getType()))
454+
// This is a propagation.
455+
addAssignOriginFact(*CCE, *CCE->getArg(0));
456+
else
457+
// This could be a new borrow.
458+
checkForBorrows(CCE, CCE->getConstructor(),
459+
{CCE->getArgs(), CCE->getNumArgs()});
460+
}
461+
462+
void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
463+
if (!isGslPointerType(MCE->getImplicitObjectArgument()->getType()))
464+
return;
465+
// Specifically for conversion operators, like `std::string_view p = a;`
466+
if (isa<CXXConversionDecl>(MCE->getCalleeDecl())) {
467+
// The argument is the implicit object itself.
468+
checkForBorrows(MCE, MCE->getMethodDecl(),
469+
{MCE->getImplicitObjectArgument()});
470+
}
471+
// Note: A more general VisitCallExpr could also be used here.
472+
}
473+
446474
void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
447475
/// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
448476
/// pointers can use the same type of loan.
@@ -495,20 +523,13 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
495523
}
496524

497525
void VisitBinaryOperator(const BinaryOperator *BO) {
498-
if (BO->isAssignmentOp()) {
499-
const Expr *LHSExpr = BO->getLHS();
500-
const Expr *RHSExpr = BO->getRHS();
501-
502-
// We are interested in assignments like `ptr1 = ptr2` or `ptr = &var`
503-
// LHS must be a pointer/reference type that can be an origin.
504-
// RHS must also represent an origin (either another pointer/ref or an
505-
// address-of).
506-
if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr))
507-
if (const auto *VD_LHS =
508-
dyn_cast<ValueDecl>(DRE_LHS->getDecl()->getCanonicalDecl());
509-
VD_LHS && hasOrigin(VD_LHS->getType()))
510-
addAssignOriginFact(*VD_LHS, *RHSExpr);
511-
}
526+
if (BO->isAssignmentOp())
527+
handleAssignment(BO->getLHS(), BO->getRHS());
528+
}
529+
530+
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
531+
if (OCE->isAssignmentOp() && OCE->getNumArgs() == 2)
532+
handleAssignment(OCE->getArg(0), OCE->getArg(1));
512533
}
513534

514535
void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE) {
@@ -520,13 +541,104 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
520541
Base::VisitCXXFunctionalCastExpr(FCE);
521542
}
522543

544+
void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) {
545+
if (!hasOrigin(MTE->getType()))
546+
return;
547+
// A temporary object's origin is the same as the origin of the
548+
// expression that initializes it.
549+
addAssignOriginFact(*MTE, *MTE->getSubExpr());
550+
}
551+
523552
private:
553+
static bool isGslPointerType(QualType QT) {
554+
// if (QT->isFunctionType())
555+
// return false;
556+
if (const auto *RD = QT->getAsCXXRecordDecl()) {
557+
// We need to check the template definition for specializations.
558+
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
559+
return CTSD->getSpecializedTemplate()
560+
->getTemplatedDecl()
561+
->hasAttr<PointerAttr>();
562+
return RD->hasAttr<PointerAttr>();
563+
}
564+
return false;
565+
}
566+
524567
// Check if a type has an origin.
525-
bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
568+
static bool hasOrigin(QualType QT) {
569+
if (QT->isFunctionPointerType())
570+
return false;
571+
return QT->isPointerOrReferenceType() || isGslPointerType(QT);
572+
}
573+
574+
/// Checks if a call-like expression creates a borrow by passing a local
575+
/// value to a reference parameter, creating an IssueFact if it does.
576+
void checkForBorrows(const Expr *Call, const FunctionDecl *FD,
577+
ArrayRef<const Expr *> Args) {
578+
if (!FD)
579+
return;
580+
581+
for (unsigned I = 0; I < Args.size(); ++I) {
582+
if (I >= FD->getNumParams())
583+
break;
584+
585+
const ParmVarDecl *Param = FD->getParamDecl(I);
586+
const Expr *Arg = Args[I];
587+
588+
// This is the core condition for a new borrow: a value type (no origin)
589+
// is passed to a reference parameter.
590+
if (Param->getType()->isReferenceType() && !hasOrigin(Arg->getType())) {
591+
if (const Loan *L = createLoanFrom(Arg, Call)) {
592+
OriginID OID = FactMgr.getOriginMgr().getOrCreate(*Call);
593+
CurrentBlockFacts.push_back(
594+
FactMgr.createFact<IssueFact>(L->ID, OID));
595+
// For view creation, we assume the first borrow is the significant
596+
// one.
597+
return;
598+
}
599+
}
600+
}
601+
}
602+
603+
/// Attempts to create a loan by analyzing the source expression of a borrow.
604+
/// This method is the single point for creating loans, allowing for future
605+
/// expansion to handle temporaries, field members, etc.
606+
/// \param SourceExpr The expression representing the object being borrowed
607+
/// from.
608+
/// \param IssueExpr The expression that triggers the borrow (e.g., a
609+
/// constructor call).
610+
/// \return The new Loan on success, nullptr on failure.
611+
const Loan *createLoanFrom(const Expr *SourceExpr, const Expr *IssueExpr) {
612+
// For now, we only handle direct borrows from local variables.
613+
// In the future, this can be extended to handle MaterializeTemporaryExpr,
614+
// etc.
615+
if (const auto *DRE =
616+
dyn_cast<DeclRefExpr>(SourceExpr->IgnoreParenImpCasts())) {
617+
if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) {
618+
AccessPath Path(VD);
619+
return &FactMgr.getLoanMgr().addLoan(Path, IssueExpr);
620+
}
621+
}
622+
return nullptr;
623+
}
624+
625+
void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr) {
626+
// Find the underlying variable declaration for the left-hand side.
627+
if (const auto *DRE_LHS =
628+
dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenImpCasts()))
629+
if (const auto *VD_LHS = dyn_cast<ValueDecl>(DRE_LHS->getDecl()))
630+
if (hasOrigin(VD_LHS->getType()))
631+
// We are interested in assignments like `ptr1 = ptr2` or `ptr = &var`
632+
// LHS must be a pointer/reference type that can be an origin.
633+
// RHS must also represent an origin (either another pointer/ref or an
634+
// address-of).
635+
addAssignOriginFact(*VD_LHS, *RHSExpr);
636+
}
526637

527638
template <typename Destination, typename Source>
528639
void addAssignOriginFact(const Destination &D, const Source &S) {
529640
OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D);
641+
Visit(&S);
530642
OriginID SrcOID = FactMgr.getOriginMgr().get(S);
531643
CurrentBlockFacts.push_back(
532644
FactMgr.createFact<AssignOriginFact>(DestOID, SrcOID));

clang/unittests/Analysis/LifetimeSafetyTest.cpp

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "clang/ASTMatchers/ASTMatchers.h"
1212
#include "clang/Testing/TestAST.h"
1313
#include "llvm/ADT/StringMap.h"
14-
#include "llvm/Testing/Support/Error.h"
1514
#include "gmock/gmock.h"
1615
#include "gtest/gtest.h"
1716
#include <optional>
@@ -31,7 +30,13 @@ class LifetimeTestRunner {
3130
LifetimeTestRunner(llvm::StringRef Code) {
3231
std::string FullCode = R"(
3332
#define POINT(name) void("__lifetime_test_point_" #name)
33+
3434
struct MyObj { ~MyObj() {} int i; };
35+
36+
struct [[gsl::Pointer()]] View {
37+
View(const MyObj&);
38+
View();
39+
};
3540
)";
3641
FullCode += Code.str();
3742

@@ -741,5 +746,76 @@ TEST_F(LifetimeAnalysisTest, NoDuplicateLoansForImplicitCastToConst) {
741746
EXPECT_THAT(Helper->getLoansForVar("a"), SizeIs(2));
742747
}
743748

749+
TEST_F(LifetimeAnalysisTest, GslPointerSimpleLoan) {
750+
SetupTest(R"(
751+
void target() {
752+
MyObj a;
753+
View x = a;
754+
POINT(p1);
755+
}
756+
)");
757+
EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1"));
758+
}
759+
760+
TEST_F(LifetimeAnalysisTest, GslPointerPropagation) {
761+
SetupTest(R"(
762+
void target() {
763+
MyObj a;
764+
View x = a;
765+
POINT(p1);
766+
767+
View y = x; // Propagation via copy-construction
768+
POINT(p2);
769+
770+
View z;
771+
z = x; // Propagation via copy-assignment
772+
POINT(p3);
773+
}
774+
)");
775+
776+
EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1"));
777+
EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p2"));
778+
EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p3"));
779+
}
780+
781+
TEST_F(LifetimeAnalysisTest, GslPointerLoanExpiration) {
782+
SetupTest(R"(
783+
void target() {
784+
View x;
785+
{
786+
MyObj a;
787+
x = a;
788+
POINT(before_expiry);
789+
} // `a` is destroyed here.
790+
POINT(after_expiry);
791+
}
792+
)");
793+
794+
EXPECT_THAT(NoLoans(), AreExpiredAt("before_expiry"));
795+
EXPECT_THAT(LoansTo({"a"}), AreExpiredAt("after_expiry"));
796+
}
797+
798+
TEST_F(LifetimeAnalysisTest, GslPointerReassignment) {
799+
SetupTest(R"(
800+
void target() {
801+
MyObj safe;
802+
View v;
803+
v = safe;
804+
POINT(p1);
805+
{
806+
MyObj unsafe;
807+
v = unsafe;
808+
POINT(p2);
809+
} // `unsafe` expires here.
810+
POINT(p3);
811+
}
812+
)");
813+
814+
EXPECT_THAT(Origin("v"), HasLoansTo({"safe"}, "p1"));
815+
EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p2"));
816+
EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p3")); // The loan is still held, just expired.
817+
EXPECT_THAT(LoansTo({"unsafe"}), AreExpiredAt("p3"));
818+
}
819+
744820
} // anonymous namespace
745821
} // namespace clang::lifetimes::internal

0 commit comments

Comments
 (0)