From 99cf35cdce40211f6be088e322af4a597ce4e412 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 7 Nov 2025 16:32:10 -0800 Subject: [PATCH] [SILOptimzer] Fix a crash caused by SILCombine mishandling inlined variables This showed up on and off again on the source-compatibility testsuite project hummingbird. The gist of the problem is that transformations may not rewrite the type of an inlined instance of a variable without also createing a deep copy of the inlined function with a different name (and e.g., a specialization suffix). Otherwise the modified inlined variable will cause an inconsistency when later compiler passes try to create the abstract declaration of that inlined function as there would be conflicting declarations for that variable. Since SILDebugScope isn't yet available in the SwiftCompilerSources this fix just drop these variables, but it would be absolutely possible to preserve them by using the same mechanism that SILCloner uses to create a deep copy of the inlined function scopes. rdar://163167975 --- .../SimplifyAllocStack.swift | 11 +++++ include/swift/SIL/TypeSubstCloner.h | 5 ++- lib/SIL/Verifier/SILVerifier.cpp | 42 +++++++++++++++++-- test/DebugInfo/inlined-generics-basic.swift | 10 ++--- test/SILOptimizer/cast_folding.swift | 28 +++++++------ test/SILOptimizer/simplify_alloc_stack.sil | 22 ++++++++++ test/Serialization/debug-value.swift | 2 +- 7 files changed, 97 insertions(+), 23 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift b/SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift index 5c99b3d8f8d4d..ea6db393de269 100644 --- a/SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift +++ b/SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift @@ -225,6 +225,17 @@ private extension AllocStackInst { source: newAlloc, sourceFormalType: concreteFormalType, destination: ucca.destination, targetFormalType: ucca.targetFormalType) context.erase(instruction: ucca) + case let dv as DebugValueInst: + if dv.location.isInlined { + // We cannot change the type of an inlined instance of a variable + // without renaming the inlined function to get a unique + // specialization suffix (prior art exists in + // SILCloner::remapFunction()). + // For now, just remove affected inlined variables. + use.set(to: Undef.get(type: type, context), context) + } else { + use.set(to: newAlloc, context) + } default: use.set(to: newAlloc, context) } diff --git a/include/swift/SIL/TypeSubstCloner.h b/include/swift/SIL/TypeSubstCloner.h index d7573487791d9..fb7e5ac679570 100644 --- a/include/swift/SIL/TypeSubstCloner.h +++ b/include/swift/SIL/TypeSubstCloner.h @@ -423,14 +423,15 @@ class TypeSubstCloner : public SILClonerWithScopes { // and types. We check if the function is called with non-identity substitutions // to decide whether it's necessary to clone a unique copy of the function // declaration with the substitutions applied for the debug info. - if (SubsMap.isIdentity()) + if (SubsMap.isIdentity() && + !SubsMap.getRecursiveProperties().hasTypeParameter()) return ParentFunction; // Note that mapReplacementTypesOutOfContext() can't do anything for // opened existentials, and since archetypes can't be mangled, ignore // this case for now. if (SubsMap.getRecursiveProperties().hasLocalArchetype()) - return ParentFunction; + SubsMap = {}; // Clone the function with the substituted type for the debug info. Mangle::GenericSpecializationMangler Mangler(M.getASTContext(), ParentFunction, diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 1f6267063d6cb..fb869d064eebc 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -1658,12 +1658,48 @@ class SILVerifier : public SILVerifierBase { require(lhs == rhs || (lhs.isAddress() && lhs.getObjectType() == rhs) || (DebugVarTy.isAddress() && lhs == rhs.getObjectType()) || - // When cloning SIL (e.g. in LoopUnroll) local archetypes are uniqued // and therefore distinct in cloned instructions. (lhs.hasLocalArchetype() && rhs.hasLocalArchetype()), - "Two variables with different type but same scope!"); - } + "Two variables with different type but same scope"); + } + + // Check that all inlined function arguments agree on their types. +#ifdef EXPENSIVE_VERIFIER_CHECKS + if (unsigned ArgNo = varInfo->ArgNo) + if (varInfo->Scope) + if (SILFunction *Fn = varInfo->Scope->getInlinedFunction()) { + using ArgMap = llvm::StringMap>; + static ArgMap DebugArgs; + llvm::StringRef Key = Fn->getName(); + if (!Key.empty()) { + auto [It, Inserted] = DebugArgs.insert({Key, {}}); + auto &CachedArgs = It->second; + if (Inserted || (!Inserted && (CachedArgs.size() < ArgNo)) || + (!Inserted && !CachedArgs[ArgNo - 1])) { + if (CachedArgs.size() < ArgNo) + CachedArgs.resize(ArgNo); + CachedArgs[ArgNo - 1] = DebugVarTy; + } else { + SILType CachedArg = CachedArgs[ArgNo - 1]; + auto lhs = CachedArg.removingMoveOnlyWrapper(); + auto rhs = DebugVarTy.removingMoveOnlyWrapper(); + if (lhs != rhs) { + llvm::errs() << "***** " << varInfo->Name << "\n"; + lhs.dump(); + rhs.dump(); + Fn->dump(); + } + require( + lhs == rhs || + (lhs.isAddress() && lhs.getObjectType() == rhs) || + (DebugVarTy.isAddress() && lhs == rhs.getObjectType()) || + (lhs.hasLocalArchetype() && rhs.hasLocalArchetype()), + "Conflicting types for function argument!"); + } + } + } +#endif // Check debug info expression if (const auto &DIExpr = varInfo->DIExpr) { diff --git a/test/DebugInfo/inlined-generics-basic.swift b/test/DebugInfo/inlined-generics-basic.swift index eb75b73c1a11b..c387d75ff94f1 100644 --- a/test/DebugInfo/inlined-generics-basic.swift +++ b/test/DebugInfo/inlined-generics-basic.swift @@ -102,7 +102,7 @@ public class C { // IR-DAG: ![[GRS_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GRS_T:[0-9]+]], {{.*}}type: ![[LET_TUPLE:[0-9]+]] // IR-DAG: ![[SP_GRS_T]] = {{.*}}linkageName: "$s1A1gyyxlFx_qd__t_Ti5" // IR-DAG: ![[GRS_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GRS_U:[0-9]+]], {{.*}}type: ![[LET_TUPLE:[0-9]+]] -// IR-DAG: ![[SP_GRS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_qd__t_Ti5" +// IR-DAG: ![[SP_GRS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5x_qd__t_Ti5" // IR-DAG: ![[LET_TUPLE]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[TUPLE:[0-9]+]]) // IR-DAG: ![[TUPLE]] = {{.*}}DW_TAG_structure_type, name: "$sx_qd__tD" // IR-DAG: ![[S]] = !DILocalVariable(name: "s", {{.*}} type: ![[LET_TAU_1_0:[0-9]+]] @@ -110,7 +110,7 @@ public class C { // IR-DAG: ![[GS_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GS_T:[0-9]+]], {{.*}} type: ![[LET_TAU_1_0]]) // IR-DAG: ![[SP_GS_T]] = {{.*}}linkageName: "$s1A1gyyxlFqd___Ti5" // IR-DAG: ![[GS_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GS_U:[0-9]+]], {{.*}} type: ![[LET_TAU_1_0]]) -// IR-DAG: ![[SP_GS_U]] = {{.*}}linkageName: "$s1A1hyyxlFqd___Ti5" +// IR-DAG: ![[SP_GS_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5qd___Ti5" // Debug info for this variable is removed. See the note above the call to g(r). // ![[GR_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GR_T:[0-9]+]], {{.*}}type: ![[LET_TAU_0_0]]) @@ -118,12 +118,12 @@ public class C { // ![[SP_GR_T]] = {{.*}}linkageName: "$s1A1gyyxlF" // IR-DAG: ![[GR_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GR_U:[0-9]+]], {{.*}}type: ![[LET_TAU_0_0]]) -// IR-DAG: ![[SP_GR_U]] = {{.*}}linkageName: "$s1A1hyyxlF" +// IR-DAG: ![[SP_GR_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5x_Ti5" // IR-DAG: ![[GI_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GI_G:[0-9]+]], {{.*}}type: ![[LET_INT]]) // IR-DAG: ![[SP_GI_G]] = {{.*}}linkageName: "$s1A1gyyxlFSi_Tg5" // IR-DAG: ![[GI_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GI_U:[0-9]+]], {{.*}}type: ![[LET_INT]]) -// IR-DAG: ![[SP_GI_U]] = {{.*}}linkageName: "$s1A1hyyxlFSi_TG5" +// IR-DAG: ![[SP_GI_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5Si_TG5" // IR-DAG: ![[GB_T]] = !DILocalVariable(name: "t", {{.*}} scope: ![[SP_GB_G:[0-9]+]], {{.*}}type: ![[LET_BOOL]]) // IR-DAG: ![[SP_GB_G]] = {{.*}}linkageName: "$s1A1gyyxlFSb_Tg5" // IR-DAG: ![[GB_U]] = !DILocalVariable(name: "u", {{.*}} scope: ![[SP_GB_U:[0-9]+]], {{.*}}type: ![[LET_BOOL]]) -// IR-DAG: ![[SP_GB_U]] = {{.*}}linkageName: "$s1A1hyyxlFSb_TG5" +// IR-DAG: ![[SP_GB_U]] = {{.*}}linkageName: "$s1A1hyyxlFx_Ti5Sb_TG5" diff --git a/test/SILOptimizer/cast_folding.swift b/test/SILOptimizer/cast_folding.swift index 5d42fba771e89..92559512d7dad 100644 --- a/test/SILOptimizer/cast_folding.swift +++ b/test/SILOptimizer/cast_folding.swift @@ -415,9 +415,10 @@ func test13_2() -> Bool { // CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test13_3SbyF : $@convention(thin) () -> Bool // CHECK: bb0 -// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1 -// CHECK-NEXT: %1 = struct $Bool -// CHECK-NEXT: return %1 +// CHECK-NEXT: debug_value +// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1 +// CHECK-NEXT: %2 = struct $Bool +// CHECK-NEXT: return %2 @inline(never) func test13_3() -> Bool { return cast13(A() as P) @@ -456,9 +457,10 @@ func test15_1() -> Bool { // CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test15_2SbyF : $@convention(thin) () -> Bool // CHECK: bb0 -// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1 -// CHECK-NEXT: %1 = struct $Bool -// CHECK-NEXT: return %1 +// CHECK-NEXT: debug_value +// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1 +// CHECK-NEXT: %2 = struct $Bool +// CHECK-NEXT: return %2 @inline(never) func test15_2() -> Bool { return cast15(A() as P) @@ -476,9 +478,10 @@ func test16_1() -> Bool { // CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test16_2SbyF : $@convention(thin) () -> Bool // CHECK: bb0 -// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1 -// CHECK-NEXT: %1 = struct $Bool -// CHECK-NEXT: return %1 +// CHECK-NEXT: debug_value +// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1 +// CHECK-NEXT: %2 = struct $Bool +// CHECK-NEXT: return %2 @inline(never) func test16_2() -> Bool { return cast16(A() as P) @@ -566,9 +569,10 @@ func test21_1() -> Bool { // CHECK-LABEL: sil hidden [noinline] @$s12cast_folding8test21_2SbyF : $@convention(thin) () -> Bool // CHECK: bb0 -// CHECK-NEXT: %0 = integer_literal $Builtin.Int1, -1 -// CHECK-NEXT: %1 = struct $Bool -// CHECK-NEXT: return %1 +// CHECK-NEXT: debug_value +// CHECK-NEXT: %1 = integer_literal $Builtin.Int1, -1 +// CHECK-NEXT: %2 = struct $Bool +// CHECK-NEXT: return %2 @inline(never) func test21_2() -> Bool { return cast21(A() as P) diff --git a/test/SILOptimizer/simplify_alloc_stack.sil b/test/SILOptimizer/simplify_alloc_stack.sil index d1623d23cef60..62771ed32acab 100644 --- a/test/SILOptimizer/simplify_alloc_stack.sil +++ b/test/SILOptimizer/simplify_alloc_stack.sil @@ -312,6 +312,28 @@ bb0(%0 : $*T, %1 : @owned $T): return %r } +// CHECK-LABEL: sil [ossa] @replace_existential_with_concrete_type3 : +// CHECK: [[S:%.*]] = alloc_stack $T +// CHECK-NOT: init_existential_addr +// CHECK-NOT: open_existential_addr +// CHECK: debug_value undef : $*any P, let, name "value1" +// CHECK: destroy_addr [[S]] +// CHECK: } // end sil function 'replace_existential_with_concrete_type3' +sil_scope 1 { loc "a.swift":1:1 parent @replace_existential_with_concrete_type3 : $@convention(thin) (@owned T) -> () } +sil_scope 2 { loc "inlined.swift":1:1 parent @$inlined : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () inlined_at 1 } +sil [ossa] @replace_existential_with_concrete_type3 : $@convention(thin) (@owned T) -> () { +bb0(%0 : @owned $T): + %5 = alloc_stack $any P + debug_value %5, let, name "value1", loc "inlined.swift":1:1, scope 2 + %6 = init_existential_addr %5, $T + store %0 to [init] %6 + %8 = open_existential_addr mutable_access %5 to $*@opened("83DE9694-7315-11E8-955C-ACDE48001122", P) Self + destroy_addr %8 + dealloc_stack %5 + %r = tuple () + return %r +} + // CHECK-LABEL: sil [ossa] @replace_existential_with_archetype : // CHECK: [[S:%.*]] = alloc_stack $@opened("82105EE0-DCB0-11E5-865D-C8E0EB309913", any P) Self // CHECK-NOT: init_existential_addr diff --git a/test/Serialization/debug-value.swift b/test/Serialization/debug-value.swift index 41e5a76bfc823..f81c6ce5a1cca 100644 --- a/test/Serialization/debug-value.swift +++ b/test/Serialization/debug-value.swift @@ -35,7 +35,7 @@ public func fooCaller(_ x: T, _ y : T) -> T { // BEGIN Main.swift import MyModule // sil_scope should refer to the specialized version of foo -//CHECK: sil_scope {{.*}} { loc "{{.*}}MyModule.swift":13:6 parent @$s8MyModule3fooyxx_xts18AdditiveArithmeticRzlFSi_TG5 {{.*}} inlined_at {{.*}} } +//CHECK: sil_scope {{.*}} { loc "{{.*}}MyModule.swift":13:6 parent @$s8MyModule3fooyxx_xts18AdditiveArithmeticRzlFx_Ti5Si_TG5 {{.*}} inlined_at {{.*}} } let _ = fooCaller(1, 2) func test() {