Skip to content

Commit 9e43f49

Browse files
eecksteineeckstein
committed
GenericSpecializer: use an alternative mangling if the function has re-abstracted resilient type parameters.
If the specialized function has a re-abstracted (= converted from indirect to direct) resilient argument or return types, use an alternative mangling: "TB" instead of "Tg". Resilient parameters/returns can be converted from indirect to direct if the specialization is created within the type's resilience domain, i.e. in its module (where the type is loadable). In this case we need to generate a different mangled name for the specialized function to distinguish it from specializations in other modules, which cannot re-abstract this resilient type. This fixes a miscompile resulting from ODR-linking specializations from different modules, which in fact have different function signatures. https://bugs.swift.org/browse/SR-13900 rdar://71914016
1 parent 4db201c commit 9e43f49

10 files changed

+159
-57
lines changed

‎include/swift/AST/TypeExpansionContext.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ class TypeExpansionContext {
8989
other.expansion == this->expansion;
9090
}
9191

92+
bool operator!=(const TypeExpansionContext &other) const {
93+
return !operator==(other);
94+
}
95+
9296
bool operator<(const TypeExpansionContext other) const {
9397
assert(other.inContext != this->inContext ||
9498
other.isContextWholeModule == this->isContextWholeModule);

‎include/swift/SIL/GenericSpecializationMangler.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ class GenericSpecializationMangler : public SpecializationMangler {
8989
Serialized, F) {}
9090

9191
std::string mangleNotReabstracted(SubstitutionMap subs);
92-
93-
std::string mangleReabstracted(SubstitutionMap subs);
92+
93+
std::string mangleReabstracted(SubstitutionMap subs, bool alternativeMangling);
9494

9595
std::string mangleForDebugInfo(GenericSignature sig, SubstitutionMap subs,
9696
bool forInlining);

‎include/swift/SILOptimizer/Utils/Generics.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,19 @@ class ReabstractionInfo {
6161
/// argument has a trivial type.
6262
SmallBitVector TrivialArgs;
6363

64+
/// Set to true if the function has a re-abstracted (= converted from
65+
/// indirect to direct) resilient argument or return type. This can happen if
66+
/// the function is compiled within the type's resilience domain, i.e. in
67+
/// its module (where the type is loadable).
68+
/// In this case we need to generate a different mangled name for the
69+
/// function to distinguish it from functions in other modules, which cannot
70+
/// re-abstract this resilient type.
71+
/// Fortunately, a flag is sufficient to describe this: either a function has
72+
/// re-abstracted resilient types or not. It cannot happen that two
73+
/// functions have two different subsets of re-abstracted resilient parameter
74+
/// types.
75+
bool hasConvertedResilientParams = false;
76+
6477
/// If set, indirect to direct conversions should be performed by the generic
6578
/// specializer.
6679
bool ConvertIndirectToDirect;
@@ -128,6 +141,12 @@ class ReabstractionInfo {
128141
// Is the generated specialization going to be serialized?
129142
IsSerialized_t Serialized;
130143

144+
enum TypeCategory {
145+
NotLoadable,
146+
Loadable,
147+
LoadableAndTrivial
148+
};
149+
131150
unsigned param2ArgIndex(unsigned ParamIdx) const {
132151
return ParamIdx + NumFormalIndirectResults;
133152
}
@@ -138,6 +157,15 @@ class ReabstractionInfo {
138157
bool HasUnboundGenericParams);
139158

140159
void createSubstitutedAndSpecializedTypes();
160+
161+
TypeCategory getReturnTypeCategory(const SILResultInfo &RI,
162+
const SILFunctionConventions &substConv,
163+
TypeExpansionContext typeExpansion);
164+
165+
TypeCategory getParamTypeCategory(const SILParameterInfo &PI,
166+
const SILFunctionConventions &substConv,
167+
TypeExpansionContext typeExpansion);
168+
141169
bool prepareAndCheck(ApplySite Apply, SILFunction *Callee,
142170
SubstitutionMap ParamSubs,
143171
OptRemark::Emitter *ORE = nullptr);
@@ -175,6 +203,12 @@ class ReabstractionInfo {
175203
IsSerialized_t isSerialized() const {
176204
return Serialized;
177205
}
206+
207+
/// Returns true if the specialized function needs an alternative mangling.
208+
/// See hasConvertedResilientParams.
209+
bool needAlternativeMangling() const {
210+
return hasConvertedResilientParams;
211+
}
178212

179213
TypeExpansionContext getResilienceExpansion() const {
180214
auto resilience = (Serialized ? ResilienceExpansion::Minimal

‎lib/SIL/Utils/GenericSpecializationMangler.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,13 @@ mangleNotReabstracted(SubstitutionMap subs) {
103103
}
104104

105105
std::string GenericSpecializationMangler::
106-
mangleReabstracted(SubstitutionMap subs) {
106+
mangleReabstracted(SubstitutionMap subs, bool alternativeMangling) {
107107
beginMangling();
108108
appendSubstitutions(getGenericSignature(), subs);
109-
appendSpecializationOperator("Tg");
109+
110+
// See ReabstractionInfo::hasConvertedResilientParams for why and when to use
111+
// the alternative mangling.
112+
appendSpecializationOperator(alternativeMangling ? "TB" : "Tg");
110113
return finalize();
111114
}
112115

‎lib/SILOptimizer/IPO/UsePrespecialized.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ bool UsePrespecialized::replaceByPrespecialized(SILFunction &F) {
106106
// are serialized without bodies. Thus use IsNotSerialized here.
107107
Mangle::GenericSpecializationMangler NewGenericMangler(ReferencedF,
108108
IsNotSerialized);
109-
std::string ClonedName = NewGenericMangler.mangleReabstracted(Subs);
109+
std::string ClonedName = NewGenericMangler.mangleReabstracted(Subs,
110+
ReInfo.needAlternativeMangling());
110111

111112
SILFunction *NewF = nullptr;
112113
// If we already have this specialization, reuse it.

‎lib/SILOptimizer/Utils/Generics.cpp

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,9 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
683683
TrivialArgs.resize(NumArgs);
684684

685685
SILFunctionConventions substConv(SubstitutedType, M);
686+
TypeExpansionContext resilienceExp = getResilienceExpansion();
687+
TypeExpansionContext minimalExp(ResilienceExpansion::Minimal,
688+
TargetModule, isWholeModule);
686689

687690
if (SubstitutedType->getNumDirectFormalResults() == 0) {
688691
// The original function has no direct result yet. Try to convert the first
@@ -693,18 +696,15 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
693696
for (SILResultInfo RI : SubstitutedType->getIndirectFormalResults()) {
694697
assert(RI.isFormalIndirect());
695698

696-
auto ResultTy = substConv.getSILType(RI, getResilienceExpansion());
697-
ResultTy = Callee->mapTypeIntoContext(ResultTy);
698-
auto &TL = M.Types.getTypeLowering(ResultTy,
699-
getResilienceExpansion());
700-
701-
if (TL.isLoadable() &&
702-
!RI.getReturnValueType(M, SubstitutedType, getResilienceExpansion())
703-
->isVoid() &&
704-
shouldExpand(M, ResultTy)) {
699+
TypeCategory tc = getReturnTypeCategory(RI, substConv, resilienceExp);
700+
if (tc != NotLoadable) {
705701
Conversions.set(IdxForResult);
706-
if (TL.isTrivial())
702+
if (tc == LoadableAndTrivial)
707703
TrivialArgs.set(IdxForResult);
704+
if (resilienceExp != minimalExp &&
705+
getReturnTypeCategory(RI, substConv, minimalExp) == NotLoadable) {
706+
hasConvertedResilientParams = true;
707+
}
708708
break;
709709
}
710710
++IdxForResult;
@@ -717,21 +717,20 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
717717
auto IdxToInsert = IdxForParam;
718718
++IdxForParam;
719719

720-
auto ParamTy = substConv.getSILType(PI, getResilienceExpansion());
721-
ParamTy = Callee->mapTypeIntoContext(ParamTy);
722-
auto &TL = M.Types.getTypeLowering(ParamTy,
723-
getResilienceExpansion());
724-
725-
if (!TL.isLoadable()) {
720+
TypeCategory tc = getParamTypeCategory(PI, substConv, resilienceExp);
721+
if (tc == NotLoadable)
726722
continue;
727-
}
728723

729724
switch (PI.getConvention()) {
730725
case ParameterConvention::Indirect_In:
731726
case ParameterConvention::Indirect_In_Guaranteed:
732727
Conversions.set(IdxToInsert);
733-
if (TL.isTrivial())
728+
if (tc == LoadableAndTrivial)
734729
TrivialArgs.set(IdxToInsert);
730+
if (resilienceExp != minimalExp &&
731+
getParamTypeCategory(PI, substConv, minimalExp) == NotLoadable) {
732+
hasConvertedResilientParams = true;
733+
}
735734
break;
736735
case ParameterConvention::Indirect_In_Constant:
737736
case ParameterConvention::Indirect_Inout:
@@ -749,6 +748,43 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
749748
SpecializedType = createSpecializedType(SubstitutedType, M);
750749
}
751750

751+
ReabstractionInfo::TypeCategory ReabstractionInfo::
752+
getReturnTypeCategory(const SILResultInfo &RI,
753+
const SILFunctionConventions &substConv,
754+
TypeExpansionContext typeExpansion) {
755+
auto &M = Callee->getModule();
756+
auto ResultTy = substConv.getSILType(RI, typeExpansion);
757+
ResultTy = Callee->mapTypeIntoContext(ResultTy);
758+
auto &TL = M.Types.getTypeLowering(ResultTy, typeExpansion);
759+
760+
if (!TL.isLoadable())
761+
return NotLoadable;
762+
763+
if (RI.getReturnValueType(M, SubstitutedType, typeExpansion)
764+
->isVoid())
765+
return NotLoadable;
766+
767+
if (!shouldExpand(M, ResultTy))
768+
return NotLoadable;
769+
770+
return TL.isTrivial() ? LoadableAndTrivial : Loadable;
771+
}
772+
773+
ReabstractionInfo::TypeCategory ReabstractionInfo::
774+
getParamTypeCategory(const SILParameterInfo &PI,
775+
const SILFunctionConventions &substConv,
776+
TypeExpansionContext typeExpansion) {
777+
auto &M = Callee->getModule();
778+
auto ParamTy = substConv.getSILType(PI, typeExpansion);
779+
ParamTy = Callee->mapTypeIntoContext(ParamTy);
780+
auto &TL = M.Types.getTypeLowering(ParamTy, typeExpansion);
781+
782+
if (!TL.isLoadable())
783+
return NotLoadable;
784+
785+
return TL.isTrivial() ? LoadableAndTrivial : Loadable;
786+
}
787+
752788
/// Create a new substituted type with the updated signature.
753789
CanSILFunctionType
754790
ReabstractionInfo::createSubstitutedType(SILFunction *OrigF,
@@ -1822,7 +1858,8 @@ GenericFuncSpecializer::GenericFuncSpecializer(
18221858
if (ReInfo.isPrespecialized()) {
18231859
ClonedName = Mangler.manglePrespecialized(ParamSubs);
18241860
} else {
1825-
ClonedName = Mangler.mangleReabstracted(ParamSubs);
1861+
ClonedName = Mangler.mangleReabstracted(ParamSubs,
1862+
ReInfo.needAlternativeMangling());
18261863
}
18271864
}
18281865
LLVM_DEBUG(llvm::dbgs() << " Specialized function " << ClonedName << '\n');
@@ -2467,7 +2504,7 @@ usePrespecialized(SILOptFunctionBuilder &funcBuilder, ApplySite apply,
24672504
Mangle::GenericSpecializationMangler mangler(refF, reInfo.isSerialized());
24682505
std::string name = reInfo.isPrespecialized() ?
24692506
mangler.manglePrespecialized(subs) :
2470-
mangler.mangleReabstracted(subs);
2507+
mangler.mangleReabstracted(subs, reInfo.needAlternativeMangling());
24712508

24722509
prespecializedReInfo = reInfo;
24732510
return lookupOrCreatePrespecialization(funcBuilder, refF, name, reInfo);

‎test/SILOptimizer/Inputs/specialization_and_resilience_module.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
public struct Mystruct {
2+
var x: Int
3+
4+
public init(_ x: Int) { self.x = x }
5+
}
6+
7+
@inline(never)
8+
@inlinable
9+
public func testParam<T>(_ t: T) {
10+
print(t)
11+
}
12+
13+
@inline(never)
14+
@inlinable
15+
public func testReturn<T>(_ a: [T]) -> T {
16+
return a[0]
17+
}
18+
19+
public func otherFunc() {
20+
testParam(Mystruct(27))
21+
print(testReturn([Mystruct(28)]))
22+
}
23+

‎test/SILOptimizer/specialization_and_resilience.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1-
// RUN: %target-swift-frontend -parse-as-library -O -module-name=test %s -enable-library-evolution -emit-sil | %FileCheck %s
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -wmo -O -enable-library-evolution %S/Inputs/specialization_and_resilience_module.swift -DMODULE -parse-as-library -emit-module -emit-module-path=%t/Test.swiftmodule -module-name=Test -c -o %t/module.o
3+
// RUN: %target-build-swift -wmo -O %s -I%t -module-name=Main -c -o %t/main.o
4+
// RUN: %target-build-swift %t/main.o %t/module.o -o %t/a.out
5+
// RUN: %target-codesign %t/a.out
6+
// RUN: %target-run %t/a.out | %FileCheck %s
27

3-
public enum En {
4-
case A
5-
case B
6-
}
8+
// REQUIRES: executable_test
79

8-
@inlinable
9-
@inline(never)
10-
func genfunc<T>(_ t: T) -> T {
11-
return t
12-
}
10+
import Test
11+
12+
// CHECK: Mystruct(x: 100)
13+
testParam(Mystruct(100))
14+
// CHECK: Mystruct(x: 101)
15+
print(testReturn([Mystruct(101)]))
16+
// CHECK: Mystruct(x: 27)
17+
// CHECK: Mystruct(x: 28)
18+
otherFunc()
1319

14-
// CHECK-LABEL: sil @$s4test11callGenFuncyyF : $@convention(thin) () -> () {
15-
// CHECK: = function_ref @$s4test7genfuncyxxlFAA2EnO_Tg5 : $@convention(thin) (En) -> @out En
16-
// CHECK: } // end sil function '$s4test11callGenFuncyyF'
17-
public func callGenFunc() {
18-
_ = genfunc(En.A)
19-
}

‎test/SILOptimizer/specialize_default_witness_resilience.sil

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ public struct ConformingStruct : ResilientProtocol {
1515
public func defaultB()
1616
}
1717

18-
// CHECK-LABEL: sil shared @$s8defaultA4main16ConformingStructV_Tg5
18+
// CHECK-LABEL: sil shared @$s8defaultA4main16ConformingStructV_TB5
1919
// CHECK: bb0(%0 : $ConformingStruct):
2020
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $ConformingStruct
2121
// CHECK-NEXT: store %0 to [[TMP]] : $*ConformingStruct
22-
// CHECK: [[FN:%.*]] = function_ref @$s8defaultB4main16ConformingStructV_Tg5
22+
// CHECK: [[FN:%.*]] = function_ref @$s8defaultB4main16ConformingStructV_TB5
2323
// CHECK-NEXT: [[LOAD:%.*]] = load [[TMP]] : $*ConformingStruct
2424
// CHECK-NEXT: [[RESULT:%.*]] = apply [[FN]]([[LOAD]])
2525
// CHECK-NEXT: dealloc_stack [[TMP]] : $*ConformingStruct
@@ -32,7 +32,7 @@ bb0(%0 : $*Self):
3232
return %result : $()
3333
}
3434

35-
// CHECK-LABEL: sil shared @$s8defaultB4main16ConformingStructV_Tg5
35+
// CHECK-LABEL: sil shared @$s8defaultB4main16ConformingStructV_TB5
3636
// CHECK: bb0(%0 : $ConformingStruct):
3737
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $ConformingStruct
3838
// CHECK-NEXT: store %0 to [[TMP]] : $*ConformingStruct
@@ -48,7 +48,7 @@ bb0(%0 : $*Self):
4848

4949
// CHECK-LABEL: sil hidden @test_specialize_default_witness_method
5050
// CHECK: bb0(%0 : $*ConformingStruct):
51-
// CHECK: [[FN:%.*]] = function_ref @$s8defaultA4main16ConformingStructV_Tg5
51+
// CHECK: [[FN:%.*]] = function_ref @$s8defaultA4main16ConformingStructV_TB5
5252
// CHECK-NEXT: [[VALUE:%.*]] = load %0 : $*ConformingStruct
5353
// CHECK-NEXT: [[RESULT:%.*]] = apply [[FN]]([[VALUE]])
5454
// CHECK-NEXT: return [[RESULT]]

0 commit comments

Comments
 (0)