Skip to content

Commit 11356ac

Browse files
Merge pull request #85397 from adrian-prantl/163167975
[SILOptimzer] Fix a crash caused by SILCombine mishandling inlined variables
2 parents bc9df74 + 99cf35c commit 11356ac

File tree

7 files changed

+97
-23
lines changed

7 files changed

+97
-23
lines changed

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,17 @@ private extension AllocStackInst {
225225
source: newAlloc, sourceFormalType: concreteFormalType,
226226
destination: ucca.destination, targetFormalType: ucca.targetFormalType)
227227
context.erase(instruction: ucca)
228+
case let dv as DebugValueInst:
229+
if dv.location.isInlined {
230+
// We cannot change the type of an inlined instance of a variable
231+
// without renaming the inlined function to get a unique
232+
// specialization suffix (prior art exists in
233+
// SILCloner::remapFunction()).
234+
// For now, just remove affected inlined variables.
235+
use.set(to: Undef.get(type: type, context), context)
236+
} else {
237+
use.set(to: newAlloc, context)
238+
}
228239
default:
229240
use.set(to: newAlloc, context)
230241
}

include/swift/SIL/TypeSubstCloner.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,14 +423,15 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
423423
// and types. We check if the function is called with non-identity substitutions
424424
// to decide whether it's necessary to clone a unique copy of the function
425425
// declaration with the substitutions applied for the debug info.
426-
if (SubsMap.isIdentity())
426+
if (SubsMap.isIdentity() &&
427+
!SubsMap.getRecursiveProperties().hasTypeParameter())
427428
return ParentFunction;
428429

429430
// Note that mapReplacementTypesOutOfContext() can't do anything for
430431
// opened existentials, and since archetypes can't be mangled, ignore
431432
// this case for now.
432433
if (SubsMap.getRecursiveProperties().hasLocalArchetype())
433-
return ParentFunction;
434+
SubsMap = {};
434435

435436
// Clone the function with the substituted type for the debug info.
436437
Mangle::GenericSpecializationMangler Mangler(M.getASTContext(), ParentFunction,

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1658,12 +1658,48 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
16581658
require(lhs == rhs ||
16591659
(lhs.isAddress() && lhs.getObjectType() == rhs) ||
16601660
(DebugVarTy.isAddress() && lhs == rhs.getObjectType()) ||
1661-
16621661
// When cloning SIL (e.g. in LoopUnroll) local archetypes are uniqued
16631662
// and therefore distinct in cloned instructions.
16641663
(lhs.hasLocalArchetype() && rhs.hasLocalArchetype()),
1665-
"Two variables with different type but same scope!");
1666-
}
1664+
"Two variables with different type but same scope");
1665+
}
1666+
1667+
// Check that all inlined function arguments agree on their types.
1668+
#ifdef EXPENSIVE_VERIFIER_CHECKS
1669+
if (unsigned ArgNo = varInfo->ArgNo)
1670+
if (varInfo->Scope)
1671+
if (SILFunction *Fn = varInfo->Scope->getInlinedFunction()) {
1672+
using ArgMap = llvm::StringMap<llvm::SmallVector<SILType, 8>>;
1673+
static ArgMap DebugArgs;
1674+
llvm::StringRef Key = Fn->getName();
1675+
if (!Key.empty()) {
1676+
auto [It, Inserted] = DebugArgs.insert({Key, {}});
1677+
auto &CachedArgs = It->second;
1678+
if (Inserted || (!Inserted && (CachedArgs.size() < ArgNo)) ||
1679+
(!Inserted && !CachedArgs[ArgNo - 1])) {
1680+
if (CachedArgs.size() < ArgNo)
1681+
CachedArgs.resize(ArgNo);
1682+
CachedArgs[ArgNo - 1] = DebugVarTy;
1683+
} else {
1684+
SILType CachedArg = CachedArgs[ArgNo - 1];
1685+
auto lhs = CachedArg.removingMoveOnlyWrapper();
1686+
auto rhs = DebugVarTy.removingMoveOnlyWrapper();
1687+
if (lhs != rhs) {
1688+
llvm::errs() << "***** " << varInfo->Name << "\n";
1689+
lhs.dump();
1690+
rhs.dump();
1691+
Fn->dump();
1692+
}
1693+
require(
1694+
lhs == rhs ||
1695+
(lhs.isAddress() && lhs.getObjectType() == rhs) ||
1696+
(DebugVarTy.isAddress() && lhs == rhs.getObjectType()) ||
1697+
(lhs.hasLocalArchetype() && rhs.hasLocalArchetype()),
1698+
"Conflicting types for function argument!");
1699+
}
1700+
}
1701+
}
1702+
#endif
16671703

16681704
// Check debug info expression
16691705
if (const auto &DIExpr = varInfo->DIExpr) {

test/DebugInfo/inlined-generics-basic.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,28 +102,28 @@ public class C<R> {
102102
// IR-DAG: ![[GRS_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GRS_T:[0-9]+]], {{.*}}type: ![[LET_TUPLE:[0-9]+]]
103103
// IR-DAG: ![[SP_GRS_T]] = {{.*}}linkageName: "$s1A1gyyxlFx_qd__t_Ti5"
104104
// IR-DAG: ![[GRS_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GRS_U:[0-9]+]], {{.*}}type: ![[LET_TUPLE:[0-9]+]]
105-
// IR-DAG: ![[SP_GRS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_qd__t_Ti5"
105+
// IR-DAG: ![[SP_GRS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5x_qd__t_Ti5"
106106
// IR-DAG: ![[LET_TUPLE]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[TUPLE:[0-9]+]])
107107
// IR-DAG: ![[TUPLE]] = {{.*}}DW_TAG_structure_type, name: "$sx_qd__tD"
108108
// IR-DAG: ![[S]] = !DILocalVariable(name: "s", {{.*}} type: ![[LET_TAU_1_0:[0-9]+]]
109109
// IR-DAG: ![[LET_TAU_1_0]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[TAU_1_0]])
110110
// IR-DAG: ![[GS_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GS_T:[0-9]+]], {{.*}} type: ![[LET_TAU_1_0]])
111111
// IR-DAG: ![[SP_GS_T]] = {{.*}}linkageName: "$s1A1gyyxlFqd___Ti5"
112112
// IR-DAG: ![[GS_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GS_U:[0-9]+]], {{.*}} type: ![[LET_TAU_1_0]])
113-
// IR-DAG: ![[SP_GS_U]] = {{.*}}linkageName: "$s1A1hyyxlFqd___Ti5"
113+
// IR-DAG: ![[SP_GS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5qd___Ti5"
114114

115115
// Debug info for this variable is removed. See the note above the call to g(r).
116116
// ![[GR_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GR_T:[0-9]+]], {{.*}}type: ![[LET_TAU_0_0]])
117117
// S has the same generic parameter numbering s T and U.
118118
// ![[SP_GR_T]] = {{.*}}linkageName: "$s1A1gyyxlF"
119119

120120
// IR-DAG: ![[GR_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GR_U:[0-9]+]], {{.*}}type: ![[LET_TAU_0_0]])
121-
// IR-DAG: ![[SP_GR_U]] = {{.*}}linkageName: "$s1A1hyyxlF"
121+
// IR-DAG: ![[SP_GR_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5x_Ti5"
122122
// IR-DAG: ![[GI_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GI_G:[0-9]+]], {{.*}}type: ![[LET_INT]])
123123
// IR-DAG: ![[SP_GI_G]] = {{.*}}linkageName: "$s1A1gyyxlFSi_Tg5"
124124
// IR-DAG: ![[GI_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GI_U:[0-9]+]], {{.*}}type: ![[LET_INT]])
125-
// IR-DAG: ![[SP_GI_U]] = {{.*}}linkageName: "$s1A1hyyxlFSi_TG5"
125+
// IR-DAG: ![[SP_GI_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5Si_TG5"
126126
// IR-DAG: ![[GB_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GB_G:[0-9]+]], {{.*}}type: ![[LET_BOOL]])
127127
// IR-DAG: ![[SP_GB_G]] = {{.*}}linkageName: "$s1A1gyyxlFSb_Tg5"
128128
// IR-DAG: ![[GB_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GB_U:[0-9]+]], {{.*}}type: ![[LET_BOOL]])
129-
// IR-DAG: ![[SP_GB_U]] = {{.*}}linkageName: "$s1A1hyyxlFSb_TG5"
129+
// IR-DAG: ![[SP_GB_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5Sb_TG5"

test/SILOptimizer/cast_folding.swift

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,10 @@ func test13_2() -> Bool {
415415

416416
// CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test13_3SbyF : $@convention(thin) () -> Bool
417417
// CHECK: bb0
418-
// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1
419-
// CHECK-NEXT: %1 = struct $Bool
420-
// CHECK-NEXT: return %1
418+
// CHECK-NEXT: debug_value
419+
// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1
420+
// CHECK-NEXT: %2 = struct $Bool
421+
// CHECK-NEXT: return %2
421422
@inline(never)
422423
func test13_3() -> Bool {
423424
return cast13(A() as P)
@@ -456,9 +457,10 @@ func test15_1() -> Bool {
456457

457458
// CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test15_2SbyF : $@convention(thin) () -> Bool
458459
// CHECK: bb0
459-
// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1
460-
// CHECK-NEXT: %1 = struct $Bool
461-
// CHECK-NEXT: return %1
460+
// CHECK-NEXT: debug_value
461+
// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1
462+
// CHECK-NEXT: %2 = struct $Bool
463+
// CHECK-NEXT: return %2
462464
@inline(never)
463465
func test15_2() -> Bool {
464466
return cast15(A() as P)
@@ -476,9 +478,10 @@ func test16_1() -> Bool {
476478

477479
// CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test16_2SbyF : $@convention(thin) () -> Bool
478480
// CHECK: bb0
479-
// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1
480-
// CHECK-NEXT: %1 = struct $Bool
481-
// CHECK-NEXT: return %1
481+
// CHECK-NEXT: debug_value
482+
// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1
483+
// CHECK-NEXT: %2 = struct $Bool
484+
// CHECK-NEXT: return %2
482485
@inline(never)
483486
func test16_2() -> Bool {
484487
return cast16(A() as P)
@@ -566,9 +569,10 @@ func test21_1() -> Bool {
566569

567570
// CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test21_2SbyF : $@convention(thin) () -> Bool
568571
// CHECK: bb0
569-
// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1
570-
// CHECK-NEXT: %1 = struct $Bool
571-
// CHECK-NEXT: return %1
572+
// CHECK-NEXT: debug_value
573+
// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1
574+
// CHECK-NEXT: %2 = struct $Bool
575+
// CHECK-NEXT: return %2
572576
@inline(never)
573577
func test21_2() -> Bool {
574578
return cast21(A() as P)

test/SILOptimizer/simplify_alloc_stack.sil

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,28 @@ bb0(%0 : $*T, %1 : @owned $T):
312312
return %r
313313
}
314314

315+
// CHECK-LABEL: sil [ossa] @replace_existential_with_concrete_type3 :
316+
// CHECK: [[S:%.*]] = alloc_stack $T
317+
// CHECK-NOT: init_existential_addr
318+
// CHECK-NOT: open_existential_addr
319+
// CHECK: debug_value undef : $*any P, let, name "value1"
320+
// CHECK: destroy_addr [[S]]
321+
// CHECK: } // end sil function 'replace_existential_with_concrete_type3'
322+
sil_scope 1 { loc "a.swift":1:1 parent @replace_existential_with_concrete_type3 : $@convention(thin) (@owned T) -> () }
323+
sil_scope 2 { loc "inlined.swift":1:1 parent @$inlined : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () inlined_at 1 }
324+
sil [ossa] @replace_existential_with_concrete_type3 : $@convention(thin) (@owned T) -> () {
325+
bb0(%0 : @owned $T):
326+
%5 = alloc_stack $any P
327+
debug_value %5, let, name "value1", loc "inlined.swift":1:1, scope 2
328+
%6 = init_existential_addr %5, $T
329+
store %0 to [init] %6
330+
%8 = open_existential_addr mutable_access %5 to $*@opened("83DE9694-7315-11E8-955C-ACDE48001122", P) Self
331+
destroy_addr %8
332+
dealloc_stack %5
333+
%r = tuple ()
334+
return %r
335+
}
336+
315337
// CHECK-LABEL: sil [ossa] @replace_existential_with_archetype :
316338
// CHECK: [[S:%.*]] = alloc_stack $@opened("82105EE0-DCB0-11E5-865D-C8E0EB309913", any P) Self
317339
// CHECK-NOT: init_existential_addr

test/Serialization/debug-value.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public func fooCaller<T: AdditiveArithmetic>(_ x: T, _ y : T) -> T {
3535
// BEGIN Main.swift
3636
import MyModule
3737
// sil_scope should refer to the specialized version of foo
38-
//CHECK: sil_scope {{.*}} { loc "{{.*}}MyModule.swift":13:6 parent @$s8MyModule3fooyxx_xts18AdditiveArithmeticRzlFSi_TG5 {{.*}} inlined_at {{.*}} }
38+
//CHECK: sil_scope {{.*}} { loc "{{.*}}MyModule.swift":13:6 parent @$s8MyModule3fooyxx_xts18AdditiveArithmeticRzlFx_Ti5Si_TG5 {{.*}} inlined_at {{.*}} }
3939
let _ = fooCaller(1, 2)
4040

4141
func test() {

0 commit comments

Comments
 (0)