From 8178aa8b657faa93636362634197bd6b05f6a63e Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Thu, 21 Aug 2025 19:04:41 +0100 Subject: [PATCH 1/2] [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::push_back`. rdar://150791778 (cherry picked from commit 0a766e59ce90cbe965d1bf210ae4f9f7b6f64466) --- lib/IRGen/GenCall.cpp | 16 ------ lib/IRGen/GenClangType.cpp | 4 -- lib/SIL/IR/SILFunctionType.cpp | 7 ++- .../foreign-reference/Inputs/module.modulemap | 5 ++ .../Inputs/pass-as-parameter.h | 16 ++++++ .../pass-as-parameter-irgen.swift | 39 ++++++++++++++ .../pass-as-parameter-module-interface.swift | 9 ++++ .../foreign-reference/pass-as-parameter.swift | 54 +++++++++++++++++++ test/Interop/Cxx/stdlib/Inputs/std-vector.h | 8 +++ test/Interop/Cxx/stdlib/use-std-vector.swift | 9 ++++ 10 files changed, 146 insertions(+), 21 deletions(-) create mode 100644 test/Interop/Cxx/foreign-reference/Inputs/pass-as-parameter.h create mode 100644 test/Interop/Cxx/foreign-reference/pass-as-parameter-irgen.swift create mode 100644 test/Interop/Cxx/foreign-reference/pass-as-parameter-module-interface.swift create mode 100644 test/Interop/Cxx/foreign-reference/pass-as-parameter.swift diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 069d5b949213b..7279d7f23eba1 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -4551,22 +4551,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(); diff --git a/lib/IRGen/GenClangType.cpp b/lib/IRGen/GenClangType.cpp index 70a0551a58992..83f891cdf6e8f 100644 --- a/lib/IRGen/GenClangType.cpp +++ b/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()); } diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index a08d01b8b15ea..31092411959f3 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/lib/SIL/IR/SILFunctionType.cpp @@ -1537,7 +1537,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() && + substTy->isForeignReferenceType())) return true; if (clangTy->isRValueReferenceType()) diff --git a/test/Interop/Cxx/foreign-reference/Inputs/module.modulemap b/test/Interop/Cxx/foreign-reference/Inputs/module.modulemap index 9b85e2ec69e4c..f50f4a516558b 100644 --- a/test/Interop/Cxx/foreign-reference/Inputs/module.modulemap +++ b/test/Interop/Cxx/foreign-reference/Inputs/module.modulemap @@ -64,6 +64,11 @@ module FunctionsAndMethodsReturningFRT { requires cplusplus } +module PassAsParameter { + header "pass-as-parameter.h" + requires cplusplus +} + module Printed { header "printed.h" requires cplusplus diff --git a/test/Interop/Cxx/foreign-reference/Inputs/pass-as-parameter.h b/test/Interop/Cxx/foreign-reference/Inputs/pass-as-parameter.h new file mode 100644 index 0000000000000..c8221af076a95 --- /dev/null +++ b/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; } diff --git a/test/Interop/Cxx/foreign-reference/pass-as-parameter-irgen.swift b/test/Interop/Cxx/foreign-reference/pass-as-parameter-irgen.swift new file mode 100644 index 0000000000000..17ef5dff24208 --- /dev/null +++ b/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]]) diff --git a/test/Interop/Cxx/foreign-reference/pass-as-parameter-module-interface.swift b/test/Interop/Cxx/foreign-reference/pass-as-parameter-module-interface.swift new file mode 100644 index 0000000000000..f71ea0c581338 --- /dev/null +++ b/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 diff --git a/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift b/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift new file mode 100644 index 0000000000000..a0e8211a4620a --- /dev/null +++ b/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() diff --git a/test/Interop/Cxx/stdlib/Inputs/std-vector.h b/test/Interop/Cxx/stdlib/Inputs/std-vector.h index e562a8227f8d6..a9a5f0ead920b 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-vector.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-vector.h @@ -22,4 +22,12 @@ class VectorOfStringSubclass : public std::vector { using std::vector::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; + #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_VECTOR_H diff --git a/test/Interop/Cxx/stdlib/use-std-vector.swift b/test/Interop/Cxx/stdlib/use-std-vector.swift index 4aa0a86adedc4..5e2a1f0d9c6dc 100644 --- a/test/Interop/Cxx/stdlib/use-std-vector.swift +++ b/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() From da96a98f4563757708badeb881eb460759e8deee Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Mon, 25 Aug 2025 20:28:20 +0100 Subject: [PATCH 2/2] [cxx-interop] Add missing `// REQUIRES: executable_test` (#83896) (cherry picked from commit a78275213183c6e739388ec3b47198a505dd6edb) --- test/Interop/Cxx/foreign-reference/pass-as-parameter.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift b/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift index a0e8211a4620a..7e4ff5d3ddbd2 100644 --- a/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift +++ b/test/Interop/Cxx/foreign-reference/pass-as-parameter.swift @@ -1,5 +1,7 @@ // RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift -Xfrontend -disable-availability-checking) +// REQUIRES: executable_test + // Temporarily disable when running with an older runtime (rdar://128681137) // UNSUPPORTED: use_os_stdlib // UNSUPPORTED: back_deployment_runtime