From f08868f95aecfd043771d2d0288b506f143864e6 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Wed, 2 Jul 2025 16:04:43 +0100 Subject: [PATCH] [cxx-interop] Lifetime dependence on a class is unsafe Swift cannot guarantee exclusivity of a class. On the other hand, lifetimebound annotations on the C++ side do not guarantee exclusivity. To resolve this issue, we ignore lifetime dependency annotations that introduce exclusive dependence on classes and import functions with ignored annotations as unsafe. Currently, Swift has no language feature to support these scenarios but it might get new features in the future to help people work around this problem. rdar://153747746 --- lib/ClangImporter/ImportDecl.cpp | 91 ++++++++++++++----- .../Interop/Cxx/class/safe-interop-mode.swift | 8 ++ test/Interop/Cxx/stdlib/Inputs/std-span.h | 17 ++++ .../Cxx/stdlib/std-span-interface.swift | 11 +++ 4 files changed, 106 insertions(+), 21 deletions(-) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 0627a8e5ed4ee..2b5d0870bcf8a 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -63,6 +63,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjCCommon.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/PrettyPrinter.h" #include "clang/AST/RecordLayout.h" @@ -4194,6 +4195,20 @@ namespace { return false; } + static bool canTypeMutateBuffer(clang::QualType ty) { + // FIXME: better way to detect if a type can mutate the underlying buffer. + if (const auto *rd = ty->getAsRecordDecl()) { + if (rd->isInStdNamespace() && rd->getName() == "span") + return !cast(rd) + ->getTemplateArgs() + .get(0) + .getAsType() + .isConstQualified(); + } + // Conservatively assume most types can mutate the underlying buffer. + return true; + } + void addLifetimeDependencies(const clang::FunctionDecl *decl, AbstractFunctionDecl *result) { if (decl->getTemplatedKind() == clang::FunctionDecl::TK_FunctionTemplate) @@ -4205,12 +4220,23 @@ namespace { !isa(decl)) return; + bool hasSkippedLifetimeAnnotation = false; auto isEscapable = [this](clang::QualType ty) { return evaluateOrDefault( Impl.SwiftContext.evaluator, ClangTypeEscapability({ty.getTypePtr(), &Impl}), CxxEscapability::Unknown) != CxxEscapability::NonEscapable; }; + auto importedAsClass = [this](clang::QualType ty, bool forSelf) { + if (!forSelf) { + if (ty->getPointeeType().isNull()) + return false; + ty = ty->getPointeeType(); + } + if (const auto *rd = ty->getAsRecordDecl()) + return recordHasReferenceSemantics(rd); + return false; + }; auto swiftParams = result->getParameters(); bool hasSelf = @@ -4237,6 +4263,7 @@ namespace { } auto retType = decl->getReturnType(); + bool retMayMutateBuffer = canTypeMutateBuffer(retType); auto warnForEscapableReturnType = [&] { if (isEscapableAnnotatedType(retType.getTypePtr())) { Impl.addImportDiagnostic( @@ -4252,8 +4279,11 @@ namespace { SmallBitVector scopedLifetimeParamIndicesForReturn(dependencyVecSize); SmallBitVector paramHasAnnotation(dependencyVecSize); std::map inheritedArgDependences; - auto processLifetimeBound = [&](unsigned idx, clang::QualType ty) { + auto processLifetimeBound = [&](unsigned idx, clang::QualType ty, + bool forSelf = false) { warnForEscapableReturnType(); + if (retMayMutateBuffer && importedAsClass(ty, forSelf)) + hasSkippedLifetimeAnnotation = true; paramHasAnnotation[idx] = true; if (isEscapable(ty)) scopedLifetimeParamIndicesForReturn[idx] = true; @@ -4302,7 +4332,8 @@ namespace { if (getImplicitObjectParamAnnotation(decl)) processLifetimeBound( result->getSelfIndex(), - cast(decl)->getThisType()->getPointeeType()); + cast(decl)->getThisType()->getPointeeType(), + /*forSelf=*/true); if (auto attr = getImplicitObjectParamAnnotation( decl)) @@ -4352,16 +4383,21 @@ namespace { Impl.SwiftContext.AllocateCopy(lifetimeDependencies)); } - for (auto [idx, param] : llvm::enumerate(decl->parameters())) { - if (isEscapable(param->getType())) - continue; - if (param->hasAttr() || paramHasAnnotation[idx]) - continue; - // We have a nonescapable parameter that does not have its lifetime - // annotated nor is it marked noescape. + if (hasSkippedLifetimeAnnotation) { auto attr = new (ASTContext) UnsafeAttr(/*implicit=*/true); result->getAttrs().add(attr); - break; + } else { + for (auto [idx, param] : llvm::enumerate(decl->parameters())) { + if (isEscapable(param->getType())) + continue; + if (param->hasAttr() || paramHasAnnotation[idx]) + continue; + // We have a nonescapable parameter that does not have its lifetime + // annotated nor is it marked noescape. + auto attr = new (ASTContext) UnsafeAttr(/*implicit=*/true); + result->getAttrs().add(attr); + break; + } } Impl.diagnoseTargetDirectly(decl); @@ -9546,13 +9582,22 @@ void ClangImporter::Implementation::swiftify(AbstractFunctionDecl *MappedDecl) { SIW_DBG(" Found bounds info '" << clang::QualType(CAT, 0) << "' on return value\n"); attachMacro = true; } + auto requiresExclusiveClassDependency = [](ParamDecl *fromParam, + clang::QualType toType) { + return fromParam->getInterfaceType()->isAnyClassReferenceType() && + SwiftDeclConverter::canTypeMutateBuffer(toType); + }; bool returnHasLifetimeInfo = false; if (SwiftDeclConverter::getImplicitObjectParamAnnotation< clang::LifetimeBoundAttr>(ClangDecl)) { SIW_DBG(" Found lifetimebound attribute on implicit 'this'\n"); - printer.printLifetimeboundReturn(SwiftifyInfoPrinter::SELF_PARAM_INDEX, - true); - returnHasLifetimeInfo = true; + if (!requiresExclusiveClassDependency( + MappedDecl->getImplicitSelfDecl(/*createIfNeeded*/ true), + ClangDecl->getReturnType())) { + printer.printLifetimeboundReturn(SwiftifyInfoPrinter::SELF_PARAM_INDEX, + true); + returnHasLifetimeInfo = true; + } } bool isClangInstanceMethod = @@ -9609,14 +9654,18 @@ void ClangImporter::Implementation::swiftify(AbstractFunctionDecl *MappedDecl) { if (clangParam->hasAttr()) { SIW_DBG(" Found lifetimebound attribute on parameter '" << *clangParam << "'\n"); - // If this parameter has bounds info we will tranform it into a Span, - // so then it will no longer be Escapable. - bool willBeEscapable = swiftParamTy->isEscapable() && - (!paramHasBoundsInfo || - mappedIndex == SwiftifyInfoPrinter::SELF_PARAM_INDEX); - printer.printLifetimeboundReturn(mappedIndex, willBeEscapable); - paramHasLifetimeInfo = true; - returnHasLifetimeInfo = true; + if (!requiresExclusiveClassDependency(swiftParam, + ClangDecl->getReturnType())) { + // If this parameter has bounds info we will tranform it into a Span, + // so then it will no longer be Escapable. + bool willBeEscapable = + swiftParamTy->isEscapable() && + (!paramHasBoundsInfo || + mappedIndex == SwiftifyInfoPrinter::SELF_PARAM_INDEX); + printer.printLifetimeboundReturn(mappedIndex, willBeEscapable); + paramHasLifetimeInfo = true; + returnHasLifetimeInfo = true; + } } if (paramIsStdSpan && paramHasLifetimeInfo) { SIW_DBG(" Found both std::span and lifetime info " diff --git a/test/Interop/Cxx/class/safe-interop-mode.swift b/test/Interop/Cxx/class/safe-interop-mode.swift index 2b9187a7a263e..68208b00a1137 100644 --- a/test/Interop/Cxx/class/safe-interop-mode.swift +++ b/test/Interop/Cxx/class/safe-interop-mode.swift @@ -64,10 +64,14 @@ View safeFunc(View v1 [[clang::noescape]], View v2 [[clang::lifetimebound]]); void unsafeFunc(View v1 [[clang::noescape]], View v2); class SharedObject { +public: + View getView() const [[clang::lifetimebound]]; private: int *p; } SWIFT_SHARED_REFERENCE(retainSharedObject, releaseSharedObject); +View getViewFromSharedObject(SharedObject* p [[clang::lifetimebound]]); + inline void retainSharedObject(SharedObject *) {} inline void releaseSharedObject(SharedObject *) {} @@ -161,6 +165,10 @@ func useUnsafeLifetimeAnnotated(v: View) { func useSharedReference(frt: SharedObject, x: OwnedData) { let _ = frt x.takeSharedObject(frt) + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + let _ = frt.getView() // expected-note{{reference to unsafe instance method 'getView()'}} + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + let _ = getViewFromSharedObject(frt) // expected-note{{reference to unsafe global function 'getViewFromSharedObject'}} } @available(SwiftStdlib 5.8, *) diff --git a/test/Interop/Cxx/stdlib/Inputs/std-span.h b/test/Interop/Cxx/stdlib/Inputs/std-span.h index 2657dec2ccbdc..c39660c80a597 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-span.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-span.h @@ -65,6 +65,7 @@ inline SpanOfInt initSpan(int arr[], size_t size) { struct DependsOnSelf { std::vector v; __attribute__((swift_name("get()"))) + __attribute__((swift_attr("@safe"))) ConstSpanOfInt get() const [[clang::lifetimebound]] { return ConstSpanOfInt(v.data(), v.size()); } }; @@ -181,4 +182,20 @@ inline void mutableKeyword(SpanOfInt copy [[clang::noescape]]) {} inline void spanWithoutTypeAlias(std::span s [[clang::noescape]]) {} inline void mutableSpanWithoutTypeAlias(std::span s [[clang::noescape]]) {} +#define IMMORTAL_FRT \ + __attribute__((swift_attr("import_reference"))) \ + __attribute__((swift_attr("retain:immortal"))) \ + __attribute__((swift_attr("release:immortal"))) + +struct IMMORTAL_FRT DependsOnSelfFRT { + std::vector v; + __attribute__((swift_name("get()"))) ConstSpanOfInt get() const + [[clang::lifetimebound]] { + return ConstSpanOfInt(v.data(), v.size()); + } + SpanOfInt getMutable() [[clang::lifetimebound]] { + return SpanOfInt(v.data(), v.size()); + } +}; + #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_SPAN_H diff --git a/test/Interop/Cxx/stdlib/std-span-interface.swift b/test/Interop/Cxx/stdlib/std-span-interface.swift index 680cb5adb16a0..2cd7d11292fec 100644 --- a/test/Interop/Cxx/stdlib/std-span-interface.swift +++ b/test/Interop/Cxx/stdlib/std-span-interface.swift @@ -37,6 +37,17 @@ import CxxStdlib // CHECK-NEXT: mutating func otherTemplatedType2(_ copy: ConstSpanOfInt, _: UnsafeMutablePointer>!) // CHECK-NEXT: } +// CHECK: class DependsOnSelfFRT { +// CHECK-NEXT: init() +// CHECK-NEXT: /// This is an auto-generated wrapper for safer interop +// CHECK-NEXT: @available(visionOS 1.0, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *) +// CHECK-NEXT: @_lifetime(borrow self) +// CHECK-NEXT: @_alwaysEmitIntoClient @_disfavoredOverload public borrowing func get() -> Span +// CHECK-NEXT: borrowing func get() -> ConstSpanOfInt +// CHECK-NEXT: borrowing func {{(__)?}}getMutable{{(Unsafe)?}}() -> SpanOfInt +// CHECK-NEXT: var v: std.{{.*}}vector> +// CHECK-NEXT: } + // CHECK: /// This is an auto-generated wrapper for safer interop // CHECK-NEXT: @available(visionOS 1.0, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *) // CHECK-NEXT: @_lifetime(s: copy s)