Skip to content

Commit f73c2e5

Browse files
ahatanakaahatanaka
authored
Revert "[SILGen] Fix the type of closure thunks that are passed const reference structs (#76903)" (#77309)
This reverts commit 9c44b79. The commit caused swift's deserialization code to crash. rdar://138726860
1 parent dc74ebf commit f73c2e5

File tree

13 files changed

+34
-189
lines changed

13 files changed

+34
-189
lines changed

‎include/swift/SIL/SILBridging.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ struct BridgedSuccessorArray {
927927
};
928928

929929
struct BridgedDeclRef {
930-
uint64_t storage[4];
930+
uint64_t storage[3];
931931

932932
BRIDGED_INLINE BridgedDeclRef(swift::SILDeclRef declRef);
933933
BRIDGED_INLINE swift::SILDeclRef unbridged() const;
@@ -938,7 +938,7 @@ struct BridgedDeclRef {
938938
};
939939

940940
struct BridgedVTableEntry {
941-
uint64_t storage[6];
941+
uint64_t storage[5];
942942

943943
enum class Kind {
944944
Normal,
@@ -980,7 +980,7 @@ struct OptionalBridgedVTable {
980980
};
981981

982982
struct BridgedWitnessTableEntry {
983-
uint64_t storage[6];
983+
uint64_t storage[5];
984984

985985
enum class Kind {
986986
invalid,

‎include/swift/SIL/SILDeclRef.h

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ namespace llvm {
3232
class raw_ostream;
3333
}
3434

35-
namespace clang {
36-
class Type;
37-
}
38-
3935
namespace swift {
4036
enum class EffectsKind : uint8_t;
4137
class AbstractFunctionDecl;
@@ -212,9 +208,6 @@ struct SILDeclRef {
212208
const GenericSignatureImpl *, CustomAttr *>
213209
pointer;
214210

215-
// Type of closure thunk.
216-
const clang::Type *thunkType = nullptr;
217-
218211
/// Returns the type of AST node location being stored by the SILDeclRef.
219212
LocKind getLocKind() const {
220213
if (loc.is<ValueDecl *>())
@@ -268,10 +261,11 @@ struct SILDeclRef {
268261
/// for the containing ClassDecl.
269262
/// - If 'loc' is a global VarDecl, this returns its GlobalAccessor
270263
/// SILDeclRef.
271-
explicit SILDeclRef(Loc loc, bool isForeign = false,
272-
bool isDistributed = false,
273-
bool isDistributedLocal = false,
274-
const clang::Type *thunkType = nullptr);
264+
explicit SILDeclRef(
265+
Loc loc,
266+
bool isForeign = false,
267+
bool isDistributed = false,
268+
bool isDistributedLocal = false);
275269

276270
/// See above put produces a prespecialization according to the signature.
277271
explicit SILDeclRef(Loc loc, GenericSignature prespecializationSig);

‎lib/SIL/IR/SILDeclRef.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,11 @@ SILDeclRef::SILDeclRef(ValueDecl *vd, SILDeclRef::Kind kind, bool isForeign,
135135
isAsyncLetClosure(0), pointer(derivativeId) {}
136136

137137
SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
138-
bool asDistributed, bool asDistributedKnownToBeLocal,
139-
const clang::Type *thunkType)
138+
bool asDistributed, bool asDistributedKnownToBeLocal)
140139
: isRuntimeAccessible(false),
141140
backDeploymentKind(SILDeclRef::BackDeploymentKind::None),
142141
defaultArgIndex(0), isAsyncLetClosure(0),
143-
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr),
144-
thunkType(thunkType) {
145-
assert((!thunkType || baseLoc.is<AbstractClosureExpr *>()) &&
146-
"thunk type is needed only for closures");
142+
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr) {
147143
if (auto *vd = baseLoc.dyn_cast<ValueDecl*>()) {
148144
if (auto *fd = dyn_cast<FuncDecl>(vd)) {
149145
// Map FuncDecls directly to Func SILDeclRefs.
@@ -173,8 +169,6 @@ SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
173169
llvm_unreachable("invalid loc decl for SILDeclRef!");
174170
}
175171
} else if (auto *ACE = baseLoc.dyn_cast<AbstractClosureExpr *>()) {
176-
assert((!asForeign || thunkType) &&
177-
"thunk type needed for foreign type for closures");
178172
loc = ACE;
179173
kind = Kind::Func;
180174
if (ACE->getASTContext().LangOpts.hasFeature(

‎lib/SIL/IR/SILFunctionType.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3991,10 +3991,12 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
39913991
// The type of the native-to-foreign thunk for a swift closure.
39923992
if (constant.isForeign && constant.hasClosureExpr() &&
39933993
shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
3994-
assert(!extInfoBuilder.getClangTypeInfo().empty() &&
3995-
"clang type not found");
3996-
AbstractionPattern pattern = AbstractionPattern(
3997-
origLoweredInterfaceType, extInfoBuilder.getClangTypeInfo().getType());
3994+
auto clangType = TC.Context.getClangFunctionType(
3995+
origLoweredInterfaceType->getParams(),
3996+
origLoweredInterfaceType->getResult(),
3997+
FunctionTypeRepresentation::CFunctionPointer);
3998+
AbstractionPattern pattern =
3999+
AbstractionPattern(origLoweredInterfaceType, clangType);
39984000
return getSILFunctionTypeForAbstractCFunction(
39994001
TC, pattern, origLoweredInterfaceType, extInfoBuilder, constant);
40004002
}
@@ -4502,13 +4504,9 @@ getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
45024504
if (!constant.isForeign)
45034505
return AbstractionPattern(fnType);
45044506

4505-
if (constant.thunkType)
4506-
return AbstractionPattern(fnType, constant.thunkType);
4507-
45084507
auto bridgedFn = getBridgedFunction(constant);
45094508
if (!bridgedFn)
45104509
return AbstractionPattern(fnType);
4511-
45124510
const clang::Decl *clangDecl = bridgedFn->getClangDecl();
45134511
if (!clangDecl)
45144512
return AbstractionPattern(fnType);

‎lib/SILGen/SILGenBridging.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,9 +1315,8 @@ static SILValue emitObjCUnconsumedArgument(SILGenFunction &SGF,
13151315
SILLocation loc,
13161316
SILValue arg) {
13171317
auto &lowering = SGF.getTypeLowering(arg->getType());
1318-
// If arg is non-trivial and has an address type, make a +1 copy and operate
1319-
// on that.
1320-
if (!lowering.isTrivial() && arg->getType().isAddress()) {
1318+
// If address-only, make a +1 copy and operate on that.
1319+
if (lowering.isAddressOnly() && SGF.useLoweredAddresses()) {
13211320
auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
13221321
SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
13231322
return tmp;
@@ -1449,11 +1448,6 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
14491448
auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
14501449
native.forwardInto(SGF, loc, buf);
14511450
native = SGF.emitManagedBufferWithCleanup(buf);
1452-
} else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
1453-
native.getType().isAddress()) {
1454-
// Load the value if the argument has an address type and the native
1455-
// function expects the argument to be passed directly.
1456-
native = SGF.emitManagedLoadCopy(loc, native.getValue());
14571451
}
14581452

14591453
if (nativeInputs[i].isConsumedInCaller()) {

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,19 +1723,7 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
17231723
FunctionConversionExpr *e,
17241724
SILType loweredResultTy,
17251725
llvm::function_ref<ManagedValue ()> fnEmitter) {
1726-
SILType loweredDestTy;
1727-
auto destTy = e->getType();
1728-
auto clangInfo =
1729-
destTy->castTo<AnyFunctionType>()->getExtInfo().getClangTypeInfo();
1730-
if (clangInfo.empty())
1731-
loweredDestTy = SGF.getLoweredType(destTy);
1732-
else
1733-
// This won't be necessary after we stop dropping clang types when
1734-
// canonicalizing function types.
1735-
loweredDestTy = SGF.getLoweredType(
1736-
AbstractionPattern(destTy->getCanonicalType(), clangInfo.getType()),
1737-
destTy);
1738-
1726+
SILType loweredDestTy = SGF.getLoweredType(e->getType());
17391727
ManagedValue result;
17401728

17411729
// We're converting between C function pointer types. They better be
@@ -1806,9 +1794,7 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18061794
#endif
18071795
semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr();
18081796
}
1809-
1810-
const clang::Type *destFnType = nullptr;
1811-
1797+
18121798
if (auto declRef = dyn_cast<DeclRefExpr>(semanticExpr)) {
18131799
setLocFromConcreteDeclRef(declRef->getDeclRef());
18141800
} else if (auto memberRef = dyn_cast<MemberRefExpr>(semanticExpr)) {
@@ -1822,18 +1808,12 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18221808
loc = closure;
18231809
return ManagedValue();
18241810
});
1825-
auto clangInfo = conversionExpr->getType()
1826-
->castTo<FunctionType>()
1827-
->getExtInfo()
1828-
.getClangTypeInfo();
1829-
if (!clangInfo.empty())
1830-
destFnType = clangInfo.getType();
18311811
} else {
18321812
llvm_unreachable("c function pointer converted from a non-concrete decl ref");
18331813
}
18341814

18351815
// Produce a reference to the C-compatible entry point for the function.
1836-
SILDeclRef constant(loc, /*foreign*/ true, false, false, destFnType);
1816+
SILDeclRef constant(loc, /*foreign*/ true);
18371817
SILConstantInfo constantInfo =
18381818
SGF.getConstantInfo(SGF.getTypeExpansionContext(), constant);
18391819

‎lib/Serialization/Deserialization.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8177,6 +8177,9 @@ class SwiftToClangBasicReader :
81778177

81788178
llvm::Expected<const clang::Type *>
81798179
ModuleFile::getClangType(ClangTypeID TID) {
8180+
if (!getContext().LangOpts.UseClangFunctionTypes)
8181+
return nullptr;
8182+
81808183
if (TID == 0)
81818184
return nullptr;
81828185

‎lib/Serialization/Serialization.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5715,7 +5715,10 @@ class Serializer::TypeSerializer : public TypeVisitor<TypeSerializer> {
57155715
using namespace decls_block;
57165716

57175717
auto resultType = S.addTypeRef(fnTy->getResult());
5718-
auto clangType = S.addClangTypeRef(fnTy->getClangTypeInfo().getType());
5718+
auto clangType =
5719+
S.getASTContext().LangOpts.UseClangFunctionTypes
5720+
? S.addClangTypeRef(fnTy->getClangTypeInfo().getType())
5721+
: ClangTypeID(0);
57195722

57205723
auto isolation = encodeIsolation(fnTy->getIsolation());
57215724

‎test/Interop/Cxx/class/Inputs/closure.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@ struct NonTrivial {
1010
int *p;
1111
};
1212

13-
struct Trivial {
14-
int i;
15-
};
16-
1713
void cfunc(void (^ _Nonnull block)(NonTrivial)) noexcept {
1814
block(NonTrivial());
1915
}
@@ -49,13 +45,4 @@ void cfuncARCWeak(ARCWeak) noexcept;
4945
void (* _Nonnull getFnPtr() noexcept)(NonTrivial) noexcept;
5046
void (* _Nonnull getFnPtr2() noexcept)(ARCWeak) noexcept;
5147

52-
void cfuncConstRefNonTrivial(void (*_Nonnull)(const NonTrivial &));
53-
void cfuncConstRefTrivial(void (*_Nonnull)(const Trivial &));
54-
void blockConstRefNonTrivial(void (^_Nonnull)(const NonTrivial &));
55-
void blockConstRefTrivial(void (^_Nonnull)(const Trivial &));
56-
#if __OBJC__
57-
void cfuncConstRefStrong(void (*_Nonnull)(const ARCStrong &));
58-
void blockConstRefStrong(void (^_Nonnull)(const ARCStrong &));
59-
#endif
60-
6148
#endif // __CLOSURE__

‎test/Interop/Cxx/class/closure-thunk-macosx.swift

Lines changed: 0 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -35,88 +35,3 @@ public func testClosureToFuncPtr() {
3535
public func testClosureToBlockReturnNonTrivial() {
3636
cfuncReturnNonTrivial({() -> NonTrivial in return NonTrivial() })
3737
}
38-
39-
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_To : $@convention(c) (@in_guaranteed NonTrivial) -> () {
40-
// CHECK: bb0(%[[V0:.*]] : $*NonTrivial):
41-
// CHECK: %[[V1:.*]] = alloc_stack $NonTrivial
42-
// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*NonTrivial
43-
// CHECK: %[[V3:.*]] = function_ref @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_ : $@convention(thin) (@in_guaranteed NonTrivial) -> ()
44-
// CHECK: %[[V4:.*]] = apply %[[V3]](%[[V1]]) : $@convention(thin) (@in_guaranteed NonTrivial) -> ()
45-
// CHECK: destroy_addr %[[V1]] : $*NonTrivial
46-
// CHECK: dealloc_stack %[[V1]] : $*NonTrivial
47-
// CHECK: return %[[V4]] : $()
48-
49-
public func testConstRefNonTrivial() {
50-
cfuncConstRefNonTrivial({S in });
51-
}
52-
53-
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main19testConstRefTrivialyyFySo0E0VcfU_To : $@convention(c) (@in_guaranteed Trivial) -> () {
54-
// CHECK: bb0(%[[V0:.*]] : $*Trivial):
55-
// CHECK: %[[V1:.*]] = load [trivial] %[[V0]] : $*Trivial
56-
// CHECK: %[[V2:.*]] = function_ref @$s4main19testConstRefTrivialyyFySo0E0VcfU_ : $@convention(thin) (Trivial) -> ()
57-
// CHECK: %[[V3:.*]] = apply %[[V2]](%[[V1]]) : $@convention(thin) (Trivial) -> ()
58-
// CHECK: return %[[V3]] : $()
59-
60-
public func testConstRefTrivial() {
61-
cfuncConstRefTrivial({S in });
62-
}
63-
64-
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_To : $@convention(c) (@in_guaranteed ARCStrong) -> () {
65-
// CHECK: bb0(%[[V0:.*]] : $*ARCStrong):
66-
// CHECK: %[[V1:.*]] = alloc_stack $ARCStrong
67-
// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*ARCStrong
68-
// CHECK: %[[V3:.*]] = load [copy] %[[V1]] : $*ARCStrong
69-
// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $ARCStrong
70-
// CHECK: %[[V5:.*]] = function_ref @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_ : $@convention(thin) (@guaranteed ARCStrong) -> ()
71-
// CHECK: %[[V6:.*]] = apply %[[V5]](%[[V4]]) : $@convention(thin) (@guaranteed ARCStrong) -> ()
72-
// CHECK: end_borrow %[[V4]] : $ARCStrong
73-
// CHECK: destroy_value %[[V3]] : $ARCStrong
74-
// CHECK: destroy_addr %[[V1]] : $*ARCStrong
75-
// CHECK: dealloc_stack %[[V1]] : $*ARCStrong
76-
// CHECK: return %[[V6]] : $()
77-
78-
public func testConstRefStrong() {
79-
cfuncConstRefStrong({S in });
80-
}
81-
82-
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo10NonTrivialVIegn_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), @in_guaranteed NonTrivial) -> () {
83-
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), %[[V1:.*]] : $*NonTrivial):
84-
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> ()
85-
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
86-
// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
87-
// CHECK: apply %[[V4]](%[[V1]]) : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
88-
// CHECK: end_borrow %[[V4]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
89-
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
90-
91-
public func testBlockConstRefNonTrivial() {
92-
blockConstRefNonTrivial({S in });
93-
}
94-
95-
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo7TrivialVIegy_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (Trivial) -> (), @in_guaranteed Trivial) -> () {
96-
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (Trivial) -> (), %[[V1:.*]] : $*Trivial):
97-
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (Trivial) -> ()
98-
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (Trivial) -> ()
99-
// CHECK: %[[V4:.*]] = load [trivial] %[[V1]] : $*Trivial
100-
// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (Trivial) -> ()
101-
// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (Trivial) -> ()
102-
// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (Trivial) -> ()
103-
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (Trivial) -> ()
104-
105-
public func testBlockConstRefTrivial() {
106-
blockConstRefTrivial({S in });
107-
}
108-
109-
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo9ARCStrongVIegg_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), @in_guaranteed ARCStrong) -> () {
110-
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), %[[V1:.*]] : $*ARCStrong):
111-
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> ()
112-
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@guaranteed ARCStrong) -> ()
113-
// CHECK: %[[V4:.*]] = load_borrow %[[V1]] : $*ARCStrong
114-
// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
115-
// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
116-
// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
117-
// CHECK: end_borrow %[[V4]] : $ARCStrong
118-
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
119-
120-
public func testBlockConstRefStrong() {
121-
blockConstRefStrong({S in });
122-
}

‎test/Interop/Cxx/stdlib/use-std-function.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,12 @@ StdFunctionTestSuite.test("FunctionStringToString init from closure and pass as
6565
expectEqual(std.string("prefixabcabc"), res)
6666
}
6767

68-
StdFunctionTestSuite.test("FunctionStringToStringConstRef init from closure and pass as parameter") {
69-
let res = invokeFunctionTwiceConstRef(.init({ $0 + std.string("abc") }),
70-
std.string("prefix"))
71-
expectEqual(std.string("prefixabcabc"), res)
72-
}
68+
// FIXME: assertion for address-only closure params (rdar://124501345)
69+
//StdFunctionTestSuite.test("FunctionStringToStringConstRef init from closure and pass as parameter") {
70+
// let res = invokeFunctionTwiceConstRef(.init({ $0 + std.string("abc") }),
71+
// std.string("prefix"))
72+
// expectEqual(std.string("prefixabcabc"), res)
73+
//}
7374
#endif
7475

7576
runAllTests()

‎test/Serialization/Inputs/convention_c_function.swift

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎test/Serialization/clang-function-types-convention-c.swift

Lines changed: 0 additions & 21 deletions
This file was deleted.

0 commit comments

Comments
 (0)