[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

Author: Gabor Horvath

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<clang::ClassTemplateSpecializationDecl>(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<clang::CXXMethodDecl, clang::ObjCMethodDecl>(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<unsigned, SmallBitVector> 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<clang::LifetimeBoundAttr>(decl))
         processLifetimeBound(
             result->getSelfIndex(),
-            cast<clang::CXXMethodDecl>(decl)->getThisType()->getPointeeType());
+            cast<clang::CXXMethodDecl>(decl)->getThisType()->getPointeeType(),
+            /*forSelf=*/true);
       if (auto attr =
               getImplicitObjectParamAnnotation<clang::LifetimeCaptureByAttr>(
                   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<clang::NoEscapeAttr>() || 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<clang::NoEscapeAttr>() || 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<clang::LifetimeBoundAttr>()) {
         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 "

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, *)

test/Interop/Cxx/stdlib/Inputs/std-span.h

@@ -65,6 +65,7 @@ inline SpanOfInt initSpan(int arr[], size_t size) {
 struct DependsOnSelf {
   std::vector<int> 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<const int> s [[clang::noescape]]) {}
 inline void mutableSpanWithoutTypeAlias(std::span<int> 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<int> 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

test/Interop/Cxx/stdlib/std-span-interface.swift

@@ -37,6 +37,17 @@ import CxxStdlib
 // CHECK-NEXT:   mutating func otherTemplatedType2(_ copy: ConstSpanOfInt, _: UnsafeMutablePointer<S<CInt>>!)
 // 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<CInt>
+// CHECK-NEXT:   borrowing func get() -> ConstSpanOfInt
+// CHECK-NEXT:   borrowing func {{(__)?}}getMutable{{(Unsafe)?}}() -> SpanOfInt
+// CHECK-NEXT:   var v: std.{{.*}}vector<CInt, std.{{.*}}allocator<CInt>>
+// 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)