llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/223635.diff 4 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h (+5) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+6-2) - (modified) clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp (+5) - (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+38) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index b7ee742445654..9097bbb267124 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -113,6 +113,11 @@ bool isGslPointerType(QualType QT); bool isGslOwnerType(QualType QT); bool isGslOwnerType(const CXXRecordDecl *RD); +// Tells whether the given constructor belongs to an Owner type and the +// parameter is of pointer type. This is useful to disable inference on owning +// pointers being captured by owners. +bool isOwnerPtrCtor(const CXXConstructorDecl *Ctor, const ParmVarDecl *PVD); + // Returns true if the given method is std::unique_ptr::release(). // This is treated as a move in lifetime analysis to avoid false-positives // when ownership is manually transferred. diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index a358215a295d1..c49b9102ef869 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -153,8 +153,12 @@ class LifetimeChecker { if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF)) AnnotationWarningsMap.try_emplace(PVD, ReturnEsc->getReturnExpr()); else if (auto *FieldEsc = dyn_cast<FieldEscapeFact>(OEF); - FieldEsc && isa<CXXConstructorDecl>(FD)) - AnnotationWarningsMap.try_emplace(PVD, FieldEsc->getFieldDecl()); + FieldEsc && isa<CXXConstructorDecl>(FD)) { + // Disable inference for pointers being captured by an owner type, + // as owners typically consume these pointers rather than borrow them. + if (!isOwnerPtrCtor(dyn_cast<CXXConstructorDecl>(FD), PVD)) + AnnotationWarningsMap.try_emplace(PVD, FieldEsc->getFieldDecl()); + } } // TODO: Suggest lifetime_capture_by(this) for parameter escaping to a // field! diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 725e6dcd5ebdd..98b2a5a867752 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -387,6 +387,11 @@ bool isGslOwnerType(const CXXRecordDecl *RD) { return isRecordWithAttr<OwnerAttr>(RD); } +bool isOwnerPtrCtor(const CXXConstructorDecl *Ctor, const ParmVarDecl *PVD) { + return Ctor && PVD->getType()->isPointerType() && + isGslOwnerType(Ctor->getParent()); +} + static StringRef getName(const CXXRecordDecl &RD) { if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(&RD)) return CTSD->getSpecializedTemplate()->getName(); diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 851f558c85dcd..c87acc9c58aeb 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -4283,3 +4283,41 @@ void test_cyclic_cfg(int n) { } // expected-note {{local variable 'a' is destroyed here}} v.use(); // expected-note {{later used here}} } + +namespace TakeOwnershipTests { +std::unique_ptr<int> takeOwnership(int* i) { return std::unique_ptr<int>(i); } + +void doubleFree() { + std::unique_ptr<int> up; + { + int a = 42; + // No use-after-scope warning here. + // This is a double-free due to multiple ownership which is currently not supported. + up = takeOwnership(&a); + } + (void)up.get(); +} + +void ok() { + std::unique_ptr<int> up; + { + int* a = new int(42); + up = takeOwnership(a); // Ok. + } + (void)up.get(); +} + +void take(std::unique_ptr<int> o); + +void foo() { + int* p; + std::unique_ptr<int> up; + { + std::unique_ptr<int> o = std::unique_ptr<int>(new int(42)); + p = o.get(); // expected-warning {{local variable 'o' may not live long enough}} \ + // expected-note {{result of call to 'get' aliases the storage of local variable 'o' because the implicit object parameter is inferred as lifetimebound}} + up = std::move(o); // expected-note {{potentially moved here}} + } // expected-note {{local variable 'o' is destroyed here}} + (void)*p; // expected-note {{later used here}} +} +} // namespace TakeOwnershipTests `````````` </details> https://github.com/llvm/llvm-project/pull/223635 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
