[cxx-interop] Pass foreign reference types with correct level of indirection

When calling a C++ function that takes a reference to a pointer to a foreign reference type, Swift would previously pass a pointer to the foreign reference type as an argument (instead of a reference to a pointer), which resulted in invalid memory accesses.

This was observed when using `std::vector<ImmortalRef*>::push_back`.

rdar://150791778

lib/IRGen/GenCall.cpp

@@ -4548,22 +4548,6 @@ void CallEmission::externalizeArguments(IRGenFunction &IGF, const Callee &callee
 
     bool isForwardableArgument = IGF.isForwardableArgument(i - firstParam);
 
-    // In Swift, values that are foreign references types will always be
-    // pointers. Additionally, we only import functions which use foreign
-    // reference types indirectly (as pointers), so we know in every case, if
-    // the argument type is a foreign reference type, the types will match up
-    // and we can simply use the input directly.
-    if (paramType.isForeignReferenceType()) {
-      auto *arg = in.claimNext();
-      if (isIndirectFormalParameter(params[i - firstParam].getConvention())) {
-        auto storageTy = IGF.IGM.getTypeInfo(paramType).getStorageType();
-        arg = IGF.Builder.CreateLoad(arg, storageTy,
-                                     IGF.IGM.getPointerAlignment());
-      }
-      out.add(arg);
-      continue;
-    }
-
     bool passIndirectToDirect = paramInfo.isIndirectInGuaranteed() && paramType.isSensitive();
     if (passIndirectToDirect) {
       llvm::Value *ptr = in.claimNext();

lib/IRGen/GenClangType.cpp

@@ -32,10 +32,6 @@ clang::CanQualType IRGenModule::getClangType(CanType type) {
 }
 
 clang::CanQualType IRGenModule::getClangType(SILType type) {
-  if (type.isForeignReferenceType())
-    return getClangType(type.getASTType()
-                            ->wrapInPointer(PTK_UnsafePointer)
-                            ->getCanonicalType());
   return getClangType(type.getASTType());
 }
 

lib/SIL/IR/SILFunctionType.cpp

@@ -1532,7 +1532,12 @@ static bool isClangTypeMoreIndirectThanSubstType(TypeConverter &TC,
   // Pass C++ const reference types indirectly. Right now there's no way to
   // express immutable borrowed params, so we have to have this hack.
   // Eventually, we should just express these correctly: rdar://89647503
-  if (importer::isCxxConstReferenceType(clangTy))
+  // If this is a const reference to a foreign reference type (const FRT&), this
+  // is equivalent to a pointer to the foreign reference type, which are passed
+  // directly.
+  if (importer::isCxxConstReferenceType(clangTy) &&
+      !(clangTy->getPointeeType()->getAs<clang::RecordType>() &&
+        substTy->isForeignReferenceType()))
     return true;
 
   if (clangTy->isRValueReferenceType())

test/Interop/Cxx/foreign-reference/Inputs/module.modulemap

@@ -63,6 +63,11 @@ module FunctionsAndMethodsReturningFRT {
   requires cplusplus
 }
 
+module PassAsParameter {
+  header "pass-as-parameter.h"
+  requires cplusplus
+}
+
 module Printed {
   header "printed.h"
   requires cplusplus

test/Interop/Cxx/foreign-reference/Inputs/pass-as-parameter.h

@@ -0,0 +1,16 @@
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:immortal")))
+__attribute__((swift_attr("release:immortal"))) IntBox {
+  int value;
+  IntBox(int value) : value(value) {}
+
+  static IntBox *create(int value) { return new IntBox(value); }
+};
+
+inline int extractValueFromPtr(IntBox *b) { return b->value; }
+inline int extractValueFromRef(IntBox &b) { return b.value; }
+inline int extractValueFromConstRef(const IntBox &b) { return b.value; }
+inline int extractValueFromRefToPtr(IntBox *&b) { return b->value; }
+inline int extractValueFromRefToConstPtr(IntBox const *&b) { return b->value; }
+inline int extractValueFromConstRefToPtr(IntBox *const &b) { return b->value; }
+inline int extractValueFromConstRefToConstPtr(IntBox const *const &b) { return b->value; }

test/Interop/Cxx/foreign-reference/pass-as-parameter-irgen.swift

@@ -0,0 +1,39 @@
+// RUN: %target-swift-emit-irgen %s -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -Xcc -fignore-exceptions -disable-availability-checking | %FileCheck %s
+
+import PassAsParameter
+
+public func refToPtr() {
+  var a = IntBox.create(123)
+  let aValue = extractValueFromRefToPtr(&a)
+  print(aValue)
+}
+// CHECK: define{{.*}} void {{.*}}refToPtr{{.*}}()
+// CHECK:   [[PTR_TO_PTR_TO_INT_BOX:%.*]] = alloca %TSo6IntBoxVSg
+// CHECK:   {{.*}} = call {{.*}} @{{.*}}extractValueFromRefToPtr{{.*}}(ptr [[PTR_TO_PTR_TO_INT_BOX]])
+
+public func constRefToPtr() {
+  let a = IntBox.create(456)
+  let aValue = extractValueFromConstRefToPtr(a)
+  print(aValue)
+}
+// CHECK: define{{.*}} void {{.*}}constRefToPtr{{.*}}()
+// CHECK:   [[PTR_TO_PTR_TO_INT_BOX2:%.*]] = alloca %TSo6IntBoxVSg
+// CHECK:   {{.*}} = call {{.*}} @{{.*}}extractValueFromConstRefToPtr{{.*}}(ptr [[PTR_TO_PTR_TO_INT_BOX2]])
+
+public func refToConstPtr() {
+  var a = IntBox.create(321)
+  let aValue = extractValueFromRefToConstPtr(&a)
+  print(aValue)
+}
+// CHECK: define{{.*}} void {{.*}}refToConstPtr{{.*}}()
+// CHECK:   [[PTR_TO_PTR_TO_INT_BOX3:%.*]] = alloca %TSo6IntBoxVSg
+// CHECK:   {{.*}} = call {{.*}} @{{.*}}extractValueFromRefToConstPtr{{.*}}(ptr [[PTR_TO_PTR_TO_INT_BOX3]])
+
+public func constRefToConstPtr() {
+  let a = IntBox.create(789)
+  let aValue = extractValueFromConstRefToConstPtr(a)
+  print(aValue)
+}
+// CHECK: define{{.*}} void {{.*}}constRefToConstPtr{{.*}}()
+// CHECK:   [[PTR_TO_PTR_TO_INT_BOX4:%.*]] = alloca %TSo6IntBoxVSg
+// CHECK:   {{.*}} = call {{.*}} @{{.*}}extractValueFromConstRefToConstPtr{{.*}}(ptr [[PTR_TO_PTR_TO_INT_BOX4]])

test/Interop/Cxx/foreign-reference/pass-as-parameter-module-interface.swift

@@ -0,0 +1,9 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=PassAsParameter -I %S/Inputs -source-filename=x -cxx-interoperability-mode=upcoming-swift | %FileCheck %s
+
+// CHECK: func extractValueFromPtr(_ b: IntBox!) -> Int32
+// CHECK: func extractValueFromRef(_ b: IntBox) -> Int32
+// CHECK: func extractValueFromConstRef(_ b: IntBox) -> Int32
+// CHECK: func extractValueFromRefToPtr(_ b: inout IntBox!) -> Int32
+// CHECK: func extractValueFromRefToConstPtr(_ b: inout IntBox!) -> Int32
+// CHECK: func extractValueFromConstRefToPtr(_ b: IntBox!) -> Int32
+// CHECK: func extractValueFromConstRefToConstPtr(_ b: IntBox!) -> Int32

test/Interop/Cxx/foreign-reference/pass-as-parameter.swift

@@ -0,0 +1,54 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift -Xfrontend -disable-availability-checking)
+
+// Temporarily disable when running with an older runtime (rdar://128681137)
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: back_deployment_runtime
+
+import StdlibUnittest
+import PassAsParameter
+
+var PassAsParameterTestSuite = TestSuite("Passing foreign reference type as parameter")
+
+PassAsParameterTestSuite.test("pass as pointer") {
+  let a = IntBox.create(123)
+  let aValue = extractValueFromPtr(a)
+  expectEqual(aValue, 123)
+}
+
+PassAsParameterTestSuite.test("pass as reference") {
+  let a = IntBox.create(321)!
+  let aValue = extractValueFromRef(a)
+  expectEqual(aValue, 321)
+}
+
+PassAsParameterTestSuite.test("pass as const reference") {
+  let a = IntBox.create(321)!
+  let aValue = extractValueFromConstRef(a)
+  expectEqual(aValue, 321)
+}
+
+PassAsParameterTestSuite.test("pass as reference to pointer") {
+  var a = IntBox.create(123)
+  let aValue = extractValueFromRefToPtr(&a)
+  expectEqual(aValue, 123)
+}
+
+PassAsParameterTestSuite.test("pass as const reference to pointer") {
+  let a = IntBox.create(456)
+  let aValue = extractValueFromConstRefToPtr(a)
+  expectEqual(aValue, 456)
+}
+
+PassAsParameterTestSuite.test("pass as const reference to pointer") {
+  var a = IntBox.create(654)
+  let aValue = extractValueFromConstRefToPtr(a)
+  expectEqual(aValue, 654)
+}
+
+PassAsParameterTestSuite.test("pass as const reference to const pointer") {
+  var a = IntBox.create(789)
+  let aValue = extractValueFromConstRefToConstPtr(a)
+  expectEqual(aValue, 789)
+}
+
+runAllTests()

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

@@ -22,4 +22,12 @@ public:
   using std::vector<std::string>::vector;
 };
 
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:immortal")))
+__attribute__((swift_attr("release:immortal"))) ImmortalRef {
+  int value;
+  static ImmortalRef *create(int value) { return new ImmortalRef({value}); }
+};
+using VectorOfImmortalRefPtr = std::vector<ImmortalRef *>;
+
 #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_VECTOR_H

test/Interop/Cxx/stdlib/use-std-vector.swift

@@ -202,4 +202,13 @@ StdVectorTestSuite.test("VectorOfInt to span").require(.stdlib_6_2).code {
   expectEqual(s[2], 3)
 }
 
+StdVectorTestSuite.test("VectorOfImmortalRefPtr").require(.stdlib_5_8).code {
+    guard #available(SwiftStdlib 5.8, *) else { return }
+
+    var v = VectorOfImmortalRefPtr()
+    let i = ImmortalRef.create(123)
+    v.push_back(i)
+    expectEqual(v[0]?.value, 123)
+}
+
 runAllTests()