Skip to content

Commit e8784b8

Browse files
Xazax-hunXazax-hun
authored
Merge pull request #83827 from Xazax-hun/const-ref-crash-take-4
[SILGen] Fix the type of closure thunks that are passed const references
2 parents 01ed0c1 + f267492 commit e8784b8

16 files changed

+365
-83
lines changed

‎lib/SIL/IR/SILFunctionType.cpp‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
//
1717
//===----------------------------------------------------------------------===//
1818

19+
#include "swift/AST/Expr.h"
20+
#include "swift/AST/Type.h"
1921
#define DEBUG_TYPE "libsil"
2022

2123
#include "swift/AST/AnyFunctionRef.h"
@@ -4380,10 +4382,12 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
43804382
// The type of the native-to-foreign thunk for a swift closure.
43814383
if (constant.isForeign && constant.hasClosureExpr() &&
43824384
shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
4383-
auto clangType = TC.Context.getClangFunctionType(
4384-
origLoweredInterfaceType->getParams(),
4385-
origLoweredInterfaceType->getResult(),
4386-
FunctionTypeRepresentation::CFunctionPointer);
4385+
auto clangType = extInfoBuilder.getClangTypeInfo().getType();
4386+
if (!clangType)
4387+
clangType = TC.Context.getClangFunctionType(
4388+
origLoweredInterfaceType->getParams(),
4389+
origLoweredInterfaceType->getResult(),
4390+
FunctionTypeRepresentation::CFunctionPointer);
43874391
AbstractionPattern pattern =
43884392
AbstractionPattern(origLoweredInterfaceType, clangType);
43894393
return getSILFunctionTypeForAbstractCFunction(
@@ -4889,15 +4893,21 @@ static AbstractFunctionDecl *getBridgedFunction(SILDeclRef declRef) {
48894893
}
48904894

48914895
static AbstractionPattern
4892-
getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
4893-
CanAnyFunctionType fnType,
4896+
getAbstractionPatternForConstant(TypeConverter &converter, ASTContext &ctx,
4897+
SILDeclRef constant, CanAnyFunctionType fnType,
48944898
unsigned numParameterLists) {
48954899
if (!constant.isForeign)
48964900
return AbstractionPattern(fnType);
48974901

4902+
if (auto *expr = constant.getAbstractClosureExpr()) {
4903+
auto &info = converter.getClosureTypeInfo(expr);
4904+
return info.OrigType;
4905+
}
4906+
48984907
auto bridgedFn = getBridgedFunction(constant);
48994908
if (!bridgedFn)
49004909
return AbstractionPattern(fnType);
4910+
49014911
const clang::Decl *clangDecl = bridgedFn->getClangDecl();
49024912
if (!clangDecl)
49034913
return AbstractionPattern(fnType);
@@ -4940,9 +4950,8 @@ TypeConverter::getLoweredFormalTypes(SILDeclRef constant,
49404950
unsigned numParameterLists = constant.getParameterListCount();
49414951

49424952
// Form an abstraction pattern for bridging purposes.
4943-
AbstractionPattern bridgingFnPattern =
4944-
getAbstractionPatternForConstant(Context, constant, fnType,
4945-
numParameterLists);
4953+
AbstractionPattern bridgingFnPattern = getAbstractionPatternForConstant(
4954+
*this, Context, constant, fnType, numParameterLists);
49464955

49474956
auto extInfo = fnType->getExtInfo();
49484957
SILFunctionTypeRepresentation rep = getDeclRefRepresentation(constant);

‎lib/SILGen/Conversion.h‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
#ifndef SWIFT_LOWERING_CONVERSION_H
1818
#define SWIFT_LOWERING_CONVERSION_H
1919

20-
#include "swift/Basic/Assertions.h"
21-
#include "swift/Basic/ExternalUnion.h"
2220
#include "Initialization.h"
2321
#include "SGFContext.h"
22+
#include "swift/Basic/Assertions.h"
23+
#include "swift/Basic/ExternalUnion.h"
24+
#include "swift/SIL/AbstractionPattern.h"
25+
#include <optional>
2426

2527
namespace swift {
2628
namespace Lowering {
@@ -115,6 +117,7 @@ class Conversion {
115117

116118
struct BridgingStorage {
117119
bool IsExplicit;
120+
AbstractionPattern InputOrigType;
118121
};
119122

120123
/// The types we store for reabstracting contexts. In general, when
@@ -161,11 +164,11 @@ class Conversion {
161164
static_assert(decltype(Types)::union_is_trivially_copyable,
162165
"define the special members if this changes");
163166

164-
Conversion(KindTy kind, CanType sourceType, CanType resultType,
165-
SILType loweredResultTy, bool isExplicit)
167+
Conversion(KindTy kind, AbstractionPattern inputOrigType, CanType sourceType,
168+
CanType resultType, SILType loweredResultTy, bool isExplicit)
166169
: Kind(kind), SourceType(sourceType), ResultType(resultType),
167170
LoweredResultType(loweredResultTy) {
168-
Types.emplaceAggregate<BridgingStorage>(kind, isExplicit);
171+
Types.emplaceAggregate<BridgingStorage>(kind, isExplicit, inputOrigType);
169172
}
170173

171174
Conversion(AbstractionPattern inputOrigType, CanType inputSubstType,
@@ -236,13 +239,19 @@ class Conversion {
236239
outputOrigType, outputSubstType, outputLoweredTy);
237240
}
238241

239-
static Conversion getBridging(KindTy kind, CanType origType,
240-
CanType resultType, SILType loweredResultTy,
241-
bool isExplicit = false) {
242+
static Conversion
243+
getBridging(KindTy kind, CanType origType, CanType resultType,
244+
SILType loweredResultTy,
245+
std::optional<AbstractionPattern> inputOrigType = std::nullopt,
246+
bool isExplicit = false) {
242247
assert(isBridgingKind(kind));
243248
assert((kind != Subtype || isAllowedConversion(origType, resultType)) &&
244249
"disallowed conversion for subtype relationship");
245-
return Conversion(kind, origType, resultType, loweredResultTy, isExplicit);
250+
if (inputOrigType)
251+
return Conversion(kind, *inputOrigType, origType, resultType,
252+
loweredResultTy, isExplicit);
253+
return Conversion(kind, AbstractionPattern(origType), origType, resultType,
254+
loweredResultTy, isExplicit);
246255
}
247256

248257
static Conversion getSubtype(CanType origType, CanType substType,
@@ -290,6 +299,10 @@ class Conversion {
290299
return Types.get<BridgingStorage>(Kind).IsExplicit;
291300
}
292301

302+
AbstractionPattern getBridgingOriginalInputType() const {
303+
return Types.get<BridgingStorage>(Kind).InputOrigType;
304+
}
305+
293306
CanType getSourceType() const {
294307
return SourceType;
295308
}

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4164,10 +4164,10 @@ class ArgEmitter {
41644164
loweredSubstArgType,
41654165
param.getSILStorageInterfaceType());
41664166
case SILFunctionLanguage::C:
4167-
return Conversion::getBridging(Conversion::BridgeToObjC,
4168-
arg.getSubstRValueType(),
4169-
origParamType.getType(),
4170-
param.getSILStorageInterfaceType());
4167+
return Conversion::getBridging(
4168+
Conversion::BridgeToObjC, arg.getSubstRValueType(),
4169+
origParamType.getType(), param.getSILStorageInterfaceType(),
4170+
origParamType);
41714171
}
41724172
llvm_unreachable("bad language");
41734173
}();

‎lib/SILGen/SILGenBridging.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,8 +1315,10 @@ static SILValue emitObjCUnconsumedArgument(SILGenFunction &SGF,
13151315
SILLocation loc,
13161316
SILValue arg) {
13171317
auto &lowering = SGF.getTypeLowering(arg->getType());
1318-
// If address-only, make a +1 copy and operate on that.
1319-
if (lowering.isAddressOnly() && SGF.useLoweredAddresses()) {
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() &&
1321+
SGF.useLoweredAddresses()) {
13201322
auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
13211323
SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
13221324
return tmp;
@@ -1453,6 +1455,11 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
14531455
auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
14541456
native.forwardInto(SGF, loc, buf);
14551457
native = SGF.emitManagedBufferWithCleanup(buf);
1458+
} else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
1459+
native.getType().isAddress() && SGF.useLoweredAddresses()) {
1460+
// Load the value if the argument has an address type and the native
1461+
// function expects the argument to be passed directly.
1462+
native = SGF.emitManagedLoadCopy(loc, native.getValue());
14561463
}
14571464

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

‎lib/SILGen/SILGenConvert.cpp‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,10 +1348,9 @@ Conversion::adjustForInitialOptionalInjection() const {
13481348
case BridgeFromObjC:
13491349
case BridgeResultFromObjC:
13501350
return OptionalInjectionConversion::forInjection(
1351-
getBridging(getKind(), getSourceType().getOptionalObjectType(),
1352-
getResultType(), getLoweredResultType(),
1353-
isBridgingExplicit())
1354-
);
1351+
getBridging(getKind(), getSourceType().getOptionalObjectType(),
1352+
getResultType(), getLoweredResultType(),
1353+
getBridgingOriginalInputType(), isBridgingExplicit()));
13551354
}
13561355
llvm_unreachable("bad kind");
13571356
}
@@ -1373,9 +1372,9 @@ Conversion::adjustForInitialOptionalConversions(CanType newSourceType) const {
13731372
case BridgeToObjC:
13741373
case BridgeFromObjC:
13751374
case BridgeResultFromObjC:
1376-
return Conversion::getBridging(getKind(), newSourceType,
1377-
getResultType(), getLoweredResultType(),
1378-
isBridgingExplicit());
1375+
return Conversion::getBridging(
1376+
getKind(), newSourceType, getResultType(), getLoweredResultType(),
1377+
getBridgingOriginalInputType(), isBridgingExplicit());
13791378
}
13801379
llvm_unreachable("bad kind");
13811380
}
@@ -1394,9 +1393,9 @@ std::optional<Conversion> Conversion::adjustForInitialForceValue() const {
13941393

13951394
case BridgeToObjC: {
13961395
auto sourceOptType = getSourceType().wrapInOptionalType();
1397-
return Conversion::getBridging(ForceAndBridgeToObjC,
1398-
sourceOptType, getResultType(),
1399-
getLoweredResultType(),
1396+
return Conversion::getBridging(ForceAndBridgeToObjC, sourceOptType,
1397+
getResultType(), getLoweredResultType(),
1398+
getBridgingOriginalInputType(),
14001399
isBridgingExplicit());
14011400
}
14021401
}

‎lib/SILGen/SILGenExpr.cpp‎

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "swift/Basic/Defer.h"
4747
#include "swift/Basic/SourceManager.h"
4848
#include "swift/Basic/type_traits.h"
49+
#include "swift/SIL/AbstractionPattern.h"
4950
#include "swift/SIL/Consumption.h"
5051
#include "swift/SIL/DynamicCasts.h"
5152
#include "swift/SIL/SILArgument.h"
@@ -749,10 +750,10 @@ tryEmitAsBridgingConversion(SILGenFunction &SGF, Expr *E, bool isExplicit,
749750
auto subExpr = result.SubExpr;
750751

751752
CanType resultType = E->getType()->getCanonicalType();
752-
Conversion conversion =
753-
Conversion::getBridging(kind, subExpr->getType()->getCanonicalType(),
754-
resultType, SGF.getLoweredType(resultType),
755-
isExplicit);
753+
Conversion conversion = Conversion::getBridging(
754+
kind, subExpr->getType()->getCanonicalType(), resultType,
755+
SGF.getLoweredType(resultType), AbstractionPattern(subExpr->getType()),
756+
isExplicit);
756757

757758
// Only use this special pattern for AnyErasure conversions when we're
758759
// emitting into a peephole.
@@ -1723,11 +1724,21 @@ static ManagedValue emitAnyClosureExpr(SILGenFunction &SGF, Expr *e,
17231724
}
17241725
}
17251726

1726-
static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
1727-
FunctionConversionExpr *e,
1728-
SILType loweredResultTy,
1729-
llvm::function_ref<ManagedValue ()> fnEmitter) {
1730-
SILType loweredDestTy = SGF.getLoweredType(e->getType());
1727+
static ManagedValue
1728+
convertCFunctionSignature(SILGenFunction &SGF, FunctionConversionExpr *e,
1729+
SILType loweredResultTy, SGFContext C,
1730+
llvm::function_ref<ManagedValue()> fnEmitter) {
1731+
SILType loweredDestTy;
1732+
auto destTy = e->getType();
1733+
if (const auto init = C.getAsConversion()) {
1734+
SILType loweredDestOptTy = init->getConversion().getLoweredResultType();
1735+
if (auto objTy = loweredDestOptTy.getOptionalObjectType())
1736+
loweredDestTy = objTy;
1737+
else
1738+
loweredDestTy = loweredDestOptTy;
1739+
} else
1740+
loweredDestTy = SGF.getLoweredType(destTy);
1741+
17311742
ManagedValue result;
17321743

17331744
// We're converting between C function pointer types. They better be
@@ -1762,9 +1773,9 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
17621773
return result;
17631774
}
17641775

1765-
static
1766-
ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
1767-
FunctionConversionExpr *conversionExpr) {
1776+
static ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
1777+
FunctionConversionExpr *conversionExpr,
1778+
SGFContext C) {
17681779
auto expr = conversionExpr->getSubExpr();
17691780

17701781
// Look through base-ignored exprs to get to the function ref.
@@ -1798,20 +1809,33 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
17981809
#endif
17991810
semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr();
18001811
}
1801-
1812+
1813+
const clang::Type *destFnType = nullptr;
1814+
18021815
if (auto declRef = dyn_cast<DeclRefExpr>(semanticExpr)) {
18031816
setLocFromConcreteDeclRef(declRef->getDeclRef());
18041817
} else if (auto memberRef = dyn_cast<MemberRefExpr>(semanticExpr)) {
18051818
setLocFromConcreteDeclRef(memberRef->getMember());
18061819
} else if (isAnyClosureExpr(semanticExpr)) {
1807-
(void) emitAnyClosureExpr(SGF, semanticExpr,
1808-
[&](AbstractClosureExpr *closure) {
1809-
// Emit the closure body.
1810-
SGF.SGM.emitClosure(closure, SGF.getClosureTypeInfo(closure));
1811-
1812-
loc = closure;
1813-
return ManagedValue();
1814-
});
1820+
if (auto init = C.getAsConversion()) {
1821+
auto conv = init->getConversion();
1822+
auto origParamType = conv.getBridgingOriginalInputType();
1823+
if (origParamType.isClangType())
1824+
destFnType = origParamType.getClangType();
1825+
}
1826+
(void)emitAnyClosureExpr(
1827+
SGF, semanticExpr, [&](AbstractClosureExpr *closure) {
1828+
// Emit the closure body.
1829+
auto functionInfo = SGF.getClosureTypeInfo(closure);
1830+
if (destFnType) {
1831+
functionInfo.OrigType =
1832+
AbstractionPattern(functionInfo.OrigType.getType(), destFnType);
1833+
SGF.SGM.Types.withClosureTypeInfo(closure, functionInfo, [] {});
1834+
}
1835+
SGF.SGM.emitClosure(closure, functionInfo);
1836+
loc = closure;
1837+
return ManagedValue::forInContext();
1838+
});
18151839
} else {
18161840
llvm_unreachable("c function pointer converted from a non-concrete decl ref");
18171841
}
@@ -1847,13 +1871,10 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18471871
}
18481872

18491873
return convertCFunctionSignature(
1850-
SGF, conversionExpr,
1851-
constantInfo.getSILType(),
1852-
[&]() -> ManagedValue {
1853-
SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant);
1854-
return ManagedValue::forObjectRValueWithoutOwnership(
1855-
cRef);
1856-
});
1874+
SGF, conversionExpr, constantInfo.getSILType(), C, [&]() -> ManagedValue {
1875+
SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant);
1876+
return ManagedValue::forObjectRValueWithoutOwnership(cRef);
1877+
});
18571878
}
18581879

18591880
// Change the representation without changing the signature or
@@ -2110,7 +2131,7 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21102131
FunctionTypeRepresentation::CFunctionPointer) {
21112132
// A "conversion" of a DeclRef a C function pointer is done by referencing
21122133
// the thunk (or original C function) with the C calling convention.
2113-
result = emitCFunctionPointer(SGF, e);
2134+
result = emitCFunctionPointer(SGF, e, C);
21142135
} else {
21152136
// Ok, we're converting a C function pointer value to another C function
21162137
// pointer.
@@ -2120,10 +2141,9 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21202141

21212142
// Possibly bitcast the C function pointer to account for ABI-compatible
21222143
// parameter and result type conversions
2123-
result = convertCFunctionSignature(SGF, e, result.getType(),
2124-
[&]() -> ManagedValue {
2125-
return result;
2126-
});
2144+
result =
2145+
convertCFunctionSignature(SGF, e, result.getType(), C,
2146+
[&]() -> ManagedValue { return result; });
21272147
}
21282148
return RValue(SGF, e, result);
21292149
}

‎lib/SILGen/SILGenProlog.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1670,7 +1670,8 @@ uint16_t SILGenFunction::emitBasicProlog(
16701670
emitIndirectResultParameters(*this, resultType, origResultType, DC);
16711671

16721672
std::optional<AbstractionPattern> origErrorType;
1673-
if (origClosureType && !origClosureType->isTypeParameterOrOpaqueArchetype()) {
1673+
if (origClosureType && !origClosureType->isTypeParameterOrOpaqueArchetype() &&
1674+
!origClosureType->isClangType()) {
16741675
origErrorType = origClosureType->getFunctionThrownErrorType();
16751676
if (origErrorType && !errorType)
16761677
errorType = origErrorType->getEffectiveThrownErrorType();

‎lib/Serialization/Deserialization.cpp‎

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

85258525
llvm::Expected<const clang::Type *>
85268526
ModuleFile::getClangType(ClangTypeID TID) {
8527-
if (!getContext().LangOpts.UseClangFunctionTypes)
8528-
return nullptr;
8529-
85308527
if (TID == 0)
85318528
return nullptr;
85328529

0 commit comments

Comments
 (0)