Skip to content

[DebugInfo] Fix verifier crash for complex switch #73504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 8, 2024
Merged

[DebugInfo] Fix verifier crash for complex switch #73504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bc672e
[DebugInfo] Deduplicate location and scope in debug variables (NFC)
Snowy1803 May 7, 2024
6136183
[DebugInfo] Fix loss of VarDecl in debug values for salvaged stores
Snowy1803 May 7, 2024
95d2479
[DebugInfo] Fix verifier crash for complex switch
Snowy1803 May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/SIL/SILBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ class SILBuilder {
#else
(void)skipVarDeclAssert;
#endif
// Don't apply location overrides on variables.
if (Var && !Var->Loc)
Var->Loc = Loc;
return insert(AllocStackInst::create(
getSILDebugLocation(Loc, true), elementType, getFunction(),
substituteAnonymousArgs(Name, Var, Loc), dynamic, isLexical,
Expand Down
14 changes: 14 additions & 0 deletions include/swift/SIL/SILInstruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,13 @@ class AllocStackInst final
/// VarDecl.
void setIsFromVarDecl() { sharedUInt8().AllocStackInst.fromVarDecl = true; }

/// Return the SILLocation for the debug variable.
SILLocation getVarLoc() const {
if (hasAuxDebugLocation())
return *getTrailingObjects<SILLocation>();
return getLoc();
}

/// Return the debug variable information attached to this instruction.
std::optional<SILDebugVariable> getVarInfo() const {
// If we used to have debug info attached but our debug info is now
Expand Down Expand Up @@ -5374,6 +5381,13 @@ class DebugValueInst final
/// or null if we don't have one.
VarDecl *getDecl() const;

/// Return the SILLocation for the debug variable.
SILLocation getVarLoc() const {
if (hasAuxDebugLocation())
return *getTrailingObjects<SILLocation>();
return getLoc();
}

/// Return the debug variable information attached to this instruction.
std::optional<SILDebugVariable> getVarInfo() const {
std::optional<SILType> AuxVarType;
Expand Down
26 changes: 22 additions & 4 deletions lib/SIL/IR/SILInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,15 @@ AllocStackInst *AllocStackInst::create(SILDebugLocation Loc,
IsLexical_t isLexical,
IsFromVarDecl_t isFromVarDecl,
UsesMoveableValueDebugInfo_t wasMoved) {
// Don't store the same information twice.
if (Var) {
if (Var->Loc == Loc.getLocation())
Var->Loc = {};
if (Var->Scope == Loc.getScope())
Var->Scope = nullptr;
if (Var->Type == elementType)
Var->Type = {};
}
SmallVector<SILValue, 8> TypeDependentOperands;
collectTypeDependentOperands(TypeDependentOperands, F,
elementType.getASTType());
Expand All @@ -277,6 +286,9 @@ AllocStackInst *AllocStackInst::create(SILDebugLocation Loc,
}

VarDecl *AllocationInst::getDecl() const {
if (auto ASI = dyn_cast<AllocStackInst>(this)) {
return ASI->getVarLoc().getAsASTNode<VarDecl>();
}
return getLoc().getAsASTNode<VarDecl>();
}

Expand Down Expand Up @@ -460,6 +472,13 @@ DebugValueInst *DebugValueInst::create(SILDebugLocation DebugLoc,
SILDebugVariable Var, bool poisonRefs,
UsesMoveableValueDebugInfo_t wasMoved,
bool trace) {
// Don't store the same information twice.
if (Var.Loc == DebugLoc.getLocation())
Var.Loc = {};
if (Var.Scope == DebugLoc.getScope())
Var.Scope = nullptr;
if (Var.Type == Operand->getType().getObjectType())
Var.Type = {};
void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
return ::new (buf)
DebugValueInst(DebugLoc, Operand, Var, poisonRefs, wasMoved, trace);
Expand All @@ -474,9 +493,8 @@ DebugValueInst::createAddr(SILDebugLocation DebugLoc, SILValue Operand,
if (!isa<AllocStackInst>(Operand))
Var.DIExpr.prependElements(
{SILDIExprElement::createOperator(SILDIExprOperator::Dereference)});
void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
return ::new (buf) DebugValueInst(DebugLoc, Operand, Var,
/*poisonRefs=*/false, wasMoved, trace);
return DebugValueInst::create(DebugLoc, Operand, M, Var,
/*poisonRefs=*/false, wasMoved, trace);
}

bool DebugValueInst::exprStartsWithDeref() const {
Expand All @@ -490,7 +508,7 @@ bool DebugValueInst::exprStartsWithDeref() const {
}

VarDecl *DebugValueInst::getDecl() const {
return getLoc().getAsASTNode<VarDecl>();
return getVarLoc().getAsASTNode<VarDecl>();
}

VarDecl *SILDebugVariable::getDecl() const {
Expand Down
45 changes: 45 additions & 0 deletions test/DebugInfo/case-match-vars.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// RUN: %empty-directory(%t)
// RUN: %{python} %utils/split_file.py -o %t %s

// RUN: %target-swift-frontend -module-name a -parse-as-library -emit-ir -g %t/Seq.swift %t/A.swift | %FileCheck %s

// This code used to trigger the verifier.
// https://github.com/apple/swift/issues/73338

// BEGIN Seq.swift
struct A<Element: Equatable>: ExpressibleByArrayLiteral, Equatable {
var base: Element?
init(arrayLiteral elements: Element...) {}
}
struct B<Element: Equatable>: ExpressibleByArrayLiteral, Equatable {
var first: Element?
init(arrayLiteral elements: Element...) {}
}

// BEGIN A.swift
enum E<T: P> {
case a(A<T.ID>)
case b(B<T.ID>)
case c

static func ==(lhs: Self, rhs: Self) -> Bool {
switch (lhs, rhs) {
case (.a([]), .c), (.c, .a([])),
(.b([]), .c), (.c, .b([])):
return true
default:
return false
}
}
}
public protocol P {
associatedtype ID: Equatable
}

// The [] expressions should be available in the debugger

// CHECK: !DILocalVariable(name: "$_0", {{.+}} line: 9
// CHECK: !DILocalVariable(name: "$_1", {{.+}} line: 9
// CHECK: !DILocalVariable(name: "$_2", {{.+}} line: 8
// CHECK: !DILocalVariable(name: "$_3", {{.+}} line: 8

2 changes: 1 addition & 1 deletion test/DebugInfo/copyforward.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ bb0(%0 : $*Optional<Element>, %1 : $*CollectionOfOne<Element>.Iterator):
%3 = alloc_stack [lexical] [var_decl] $Optional<Element>, let, name "result", loc "backward.swift":67:9, scope 2
%4 = struct_element_addr %1 : $*CollectionOfOne<Element>.Iterator, #CollectionOfOne.Iterator._elements, loc "backward.swift":67:18, scope 1

// CHECK: debug_value %{{[0-9]+}} : $*Optional<Element>, let, (name "result", loc "backward.swift":67:9, scope 2), expr op_deref, loc "backward.swift":67:18, scope 2
// CHECK: debug_value %{{[0-9]+}} : $*Optional<Element>, let, (name "result", loc "backward.swift":67:9), expr op_deref, loc "backward.swift":67:18, scope 2

copy_addr %4 to [init] %3 : $*Optional<Element>, loc "backward.swift":67:18, scope 1
%6 = alloc_stack $Optional<Element>, loc "backward.swift":68:17, scope 2
Expand Down
4 changes: 2 additions & 2 deletions test/DebugInfo/debug_info_expression.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bb0:
%3 = struct_element_addr %2 : $*MyStruct, #MyStruct.x, loc "file.swift":9:17, scope 1
// CHECK: %[[FIELD_X:.*]] = getelementptr {{.*}} %[[MY_STRUCT]]
// CHECK-SIL: debug_value %{{[0-9]+}} : $*Builtin.Int64
// CHECK-SIL-SAME: (name "my_struct", loc "file.swift":8:9, scope {{[0-9]+}})
// CHECK-SIL-SAME: (name "my_struct", loc "file.swift":8:9)
// CHECK-SIL-SAME: type $MyStruct, expr op_deref:op_fragment:#MyStruct.x
debug_value %3 : $*Builtin.Int64, var, (name "my_struct", loc "file.swift":8:9, scope 1), type $MyStruct, expr op_deref:op_fragment:#MyStruct.x, loc "file.swift":9:17, scope 1

Expand All @@ -51,7 +51,7 @@ bb0:
// CHECK: %[[MY_STRUCT:.+]] = alloca %{{.*}}MyStruct
// CHECK: llvm.dbg.declare(metadata ptr %[[MY_STRUCT]], metadata ![[VAR_DECL_MD:[0-9]+]]
// CHECK-SIL: alloc_stack $Int, var
// CHECK-SIL-SAME: (name "my_struct", loc "file.swift":15:9, scope {{[0-9]+}})
// CHECK-SIL-SAME: (name "my_struct", loc "file.swift":15:9)
// CHECK-SIL-SAME: type $MyStruct, expr op_fragment:#MyStruct.x
%field_x = alloc_stack $Int, var, (name "my_struct", loc "file.swift":15:9, scope 2), type $MyStruct, expr op_fragment:#MyStruct.x, loc "file.swift":16:17, scope 2
// CHECK: %[[FIELD_X:.+]] = alloca %TSi
Expand Down
8 changes: 4 additions & 4 deletions test/DebugInfo/sroa_mem2reg.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ bb0(%0 : $Int64, %1 : $Int64):
// CHECK-MEM2REG: %[[FIELD_Y:[0-9]+]] = struct_extract %[[STRUCT]] : $MyStruct, #MyStruct.y, loc "sroa.swift":8:21
store %1 to %15 : $*Int64, loc "sroa.swift":10:17, scope 2
// Make sure the struct fields' SSA values are properly connected to the source variables via op_fragment
// CHECK-MEM2REG: debug_value %[[FIELD_X]] : $Int64, var, (name "my_struct", loc "sroa.swift":8:9, scope 2), type $*MyStruct, expr op_fragment:#MyStruct.x
// CHECK-MEM2REG: debug_value %[[FIELD_Y]] : $Int64, var, (name "my_struct", loc "sroa.swift":8:9, scope 2), type $*MyStruct, expr op_fragment:#MyStruct.y
// CHECK-MEM2REG: debug_value %[[FIELD_X]] : $Int64, var, (name "my_struct", loc "sroa.swift":8:9), type $*MyStruct, expr op_fragment:#MyStruct.x
// CHECK-MEM2REG: debug_value %[[FIELD_Y]] : $Int64, var, (name "my_struct", loc "sroa.swift":8:9), type $*MyStruct, expr op_fragment:#MyStruct.y
// CHECK-IR: call void @llvm.dbg.value(metadata i64 %0
// CHECK-IR-SAME: metadata ![[STRUCT_MD:[0-9]+]]
// CHECK-IR-SAME: !DIExpression(DW_OP_LLVM_fragment, 0, 64)
// CHECK-IR: call void @llvm.dbg.value(metadata i64 %1
// CHECK-IR-SAME: metadata ![[STRUCT_MD]]
// CHECK-IR-SAME: !DIExpression(DW_OP_LLVM_fragment, 64, 64)
// Make sure function arguments' SSA values are also properly connected to the source variables
// CHECK-MEM2REG: debug_value %0 : $Int64, var, (name "my_struct", loc "sroa.swift":8:9, scope 2), type $*MyStruct, expr op_fragment:#MyStruct.x
// CHECK-MEM2REG: debug_value %1 : $Int64, var, (name "my_struct", loc "sroa.swift":8:9, scope 2), type $*MyStruct, expr op_fragment:#MyStruct.y
// CHECK-MEM2REG: debug_value %0 : $Int64, var, (name "my_struct", loc "sroa.swift":8:9), type $*MyStruct, expr op_fragment:#MyStruct.x
// CHECK-MEM2REG: debug_value %1 : $Int64, var, (name "my_struct", loc "sroa.swift":8:9), type $*MyStruct, expr op_fragment:#MyStruct.y
// CHECK-IR: call void @llvm.dbg.value(metadata i64 %0, metadata ![[ARG1_MD:[0-9]+]]
// CHECK-IR: call void @llvm.dbg.value(metadata i64 %1, metadata ![[ARG2_MD:[0-9]+]]
dealloc_stack %4 : $*MyStruct, loc "sroa.swift":8:9, scope 2
Expand Down
16 changes: 6 additions & 10 deletions test/SILOptimizer/assemblyvision_remark/cast_remarks.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,11 @@ public func forcedCast3<NS, T>(_ ns: NS) -> T {

public func forcedCast4<NS, T>(_ ns: NS, _ ns2: NS) -> T {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we lose that x was assigned ns2. This is flow sensitive information
// that we might be able to recover. We still emit that a runtime cast
// occurred here, just don't say what the underlying value was.
var x = ns
x = ns2
return x as! T // expected-remark @:12 {{unconditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-9:44 {{of 'ns2'}}
// expected-note @-5:44 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
}

public func condCast<NS, T>(_ ns: NS) -> T? {
Expand Down Expand Up @@ -73,14 +70,11 @@ public func condCast3<NS, T>(_ ns: NS) -> T? {

public func condCast4<NS, T>(_ ns: NS, _ ns2: NS) -> T? {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we lose that x was assigned ns2. This is flow sensitive information
// that we might be able to recover. We still emit that a runtime cast
// occurred here, just don't say what the underlying value was.
var x = ns
x = ns2
return x as? T // expected-remark @:12 {{conditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-9:42 {{of 'ns2'}}
// expected-note @-5:42 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, so this actually fixes another bug?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now that getDecl() returns the correct variable declaration after a store, this pass is able to point back to the x value! After optimizations. the SILValue will be associated with both debug variables, so the pass prints both of them.

}

public func condCast5<NS, T>(_ ns: NS) -> T? {
Expand Down Expand Up @@ -252,6 +246,7 @@ public func forcedCast4(_ ns: Existential1, _ ns2: Existential1) -> Existential2
x = ns2
return x as! Existential2 // expected-remark @:12 {{unconditional runtime cast of value with type 'any Existential1' to 'any Existential2'}}
// expected-note @-5:47 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
}

public func condCast(_ ns: Existential1) -> Existential2? {
Expand Down Expand Up @@ -287,6 +282,7 @@ public func condCast4(_ ns: Existential1, _ ns2: Existential1) -> Existential2?
x = ns2
return x as? Existential2 // expected-remark @:12 {{conditional runtime cast of value with type 'any Existential1' to 'any Existential2'}}
// expected-note @-5:45 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
}

public func condCast5(_ ns: Existential1) -> Existential2? {
Expand Down
38 changes: 12 additions & 26 deletions test/SILOptimizer/assemblyvision_remark/cast_remarks_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,27 @@ public func forcedCast<NS, T>(_ ns: NS) -> T {

public func forcedCast2<NS, T>(_ ns: NS) -> T {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we seem to completely eliminate 'x' here in the debug info. TODO:
// Maybe we can recover this info somehow.
let x = ns
return x as! T // expected-remark @:12 {{unconditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-7:34 {{of 'ns'}}
// expected-note @-4:34 {{of 'ns'}}
// expected-note @-3:7 {{of 'x'}}
}

public func forcedCast3<NS, T>(_ ns: NS) -> T {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we seem to completely eliminate 'x' here in the debug info. TODO:
// Maybe we can recover this info somehow.
var x = ns // expected-warning {{variable 'x' was never mutated}}
return x as! T // expected-remark @:12 {{unconditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-7:34 {{of 'ns'}}
// expected-note @-4:34 {{of 'ns'}}
// expected-note @-3:7 {{of 'x'}}
}

public func forcedCast4<NS, T>(_ ns: NS, _ ns2: NS) -> T {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we lose that x was assigned ns2. This is flow sensitive information
// that we might be able to recover. We still emit that a runtime cast
// occurred here, just don't say what the underlying value was.
var x = ns
x = ns2
return x as! T // expected-remark @:12 {{unconditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-9:44 {{of 'ns2'}}
// expected-note @-5:44 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
}

public func condCast<NS, T>(_ ns: NS) -> T? {
Expand All @@ -56,34 +49,27 @@ public func condCast<NS, T>(_ ns: NS) -> T? {

public func condCast2<NS, T>(_ ns: NS) -> T? {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we seem to completely eliminate 'x' here in the debug info. TODO:
// Maybe we can recover this info somehow.
let x = ns
return x as? T // expected-remark @:12 {{conditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-7:32 {{of 'ns'}}
// expected-note @-4:32 {{of 'ns'}}
// expected-note @-3:7 {{of 'x'}}
}

public func condCast3<NS, T>(_ ns: NS) -> T? {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we seem to completely eliminate 'x' here in the debug info. TODO:
// Maybe we can recover this info somehow.
var x = ns // expected-warning {{variable 'x' was never mutated}}
return x as? T // expected-remark @:12 {{conditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-7:32 {{of 'ns'}}
// expected-note @-4:32 {{of 'ns'}}
// expected-note @-3:7 {{of 'x'}}
}

public func condCast4<NS, T>(_ ns: NS, _ ns2: NS) -> T? {
// Make sure the colon info is right so that the arrow is under the a.
//
// Today, we lose that x was assigned ns2. This is flow sensitive information
// that we might be able to recover. We still emit that a runtime cast
// occurred here, just don't say what the underlying value was.
var x = ns
x = ns2
return x as? T // expected-remark @:12 {{conditional runtime cast of value with type 'NS' to 'T'}}
// expected-note @-9:42 {{of 'ns2'}}
// expected-note @-5:42 {{of 'ns2'}}
// expected-note @-4:7 {{of 'x'}}
}

public func condCast5<NS, T>(_ ns: NS) -> T? {
Expand Down
3 changes: 2 additions & 1 deletion test/SILOptimizer/assemblyvision_remark/chacha.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ public func run_ChaCha(_ N: Int) {
// expected-remark @-1:27 {{release of type '}}
}
} // expected-remark {{release of type '}}

// expected-note @-7 {{of 'plaintext}}
// expected-remark @-2 {{release of type '}}
// expected-note @-16 {{of 'nonce}}
// expected-remark @-4 {{release of type '}}
// expected-note @-19 {{of 'key}}
// expected-remark @-6 {{release of type '}}
// expected-note @-18 {{of 'checkedtext}}