Merge pull request swiftlang#85397 from adrian-prantl/163167975

[SILOptimzer] Fix a crash caused by SILCombine mishandling inlined variables

Author: adrian-prantl

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)
       }

include/swift/SIL/TypeSubstCloner.h

@@ -423,14 +423,15 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
     // 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,

lib/SIL/Verifier/SILVerifier.cpp

@@ -1658,12 +1658,48 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       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<llvm::SmallVector<SILType, 8>>;
+          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) {

test/DebugInfo/inlined-generics-basic.swift

@@ -102,28 +102,28 @@ public class C<R> {
 // 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]+]]
 // IR-DAG: ![[LET_TAU_1_0]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[TAU_1_0]])
 // 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]])
 // S has the same generic parameter numbering s T and U.
 //   ![[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"

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)

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

test/Serialization/debug-value.swift

@@ -35,7 +35,7 @@ public func fooCaller<T: AdditiveArithmetic>(_ 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() {