Skip to content

Commit ae7e301

Browse files
gottesmmgottesmm
authored
Merge pull request #37468 from gottesmm/sil-debug-info-more-verification
[sil][debug-info] Refactor SILInstruction SILLocation verification out of SILVerifier onto a helper/use to verify in SILBuilder
2 parents 7da537c + e88134b commit ae7e301

File tree

7 files changed

+113
-58
lines changed

7 files changed

+113
-58
lines changed

‎include/swift/SIL/SILBuilder.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2622,6 +2622,11 @@ class SILBuilder {
26222622
C.notifyInserted(TheInst);
26232623

26242624
#ifndef NDEBUG
2625+
// If we are inserting into a specific function (rather than a block for a
2626+
// global_addr), verify that our instruction/the associated location are in
2627+
// sync. We don't care if an instruction is used in global_addr.
2628+
if (F)
2629+
TheInst->verifyDebugInfo();
26252630
TheInst->verifyOperandOwnership();
26262631
#endif
26272632
}

‎include/swift/SIL/SILDebugScope.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
9595
};
9696

9797
/// Determine whether an instruction may not have a SILDebugScope.
98-
bool maybeScopeless(SILInstruction &I);
98+
bool maybeScopeless(const SILInstruction &inst);
9999

100100
/// Knows how to make a deep copy of a debug scope.
101101
class ScopeCloner {

‎include/swift/SIL/SILInstruction.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,10 @@ class SILInstruction : public llvm::ilist_node<SILInstruction> {
733733
/// with this instruction.
734734
void verifyOperandOwnership() const;
735735

736+
/// Verify that this instruction and its associated debug information follow
737+
/// all SIL debug info invariants.
738+
void verifyDebugInfo() const;
739+
736740
/// Get the number of created SILInstructions.
737741
static int getNumCreatedInstructions() {
738742
return NumCreatedInstructions;

‎lib/SIL/IR/SILDebugScope.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ SILFunction *SILDebugScope::getParentFunction() const {
5757
}
5858

5959
/// Determine whether an instruction may not have a SILDebugScope.
60-
bool swift::maybeScopeless(SILInstruction &I) {
61-
if (I.getFunction()->isBare())
60+
bool swift::maybeScopeless(const SILInstruction &inst) {
61+
if (inst.getFunction()->isBare())
6262
return true;
63-
return !isa<DebugValueInst>(I) && !isa<DebugValueAddrInst>(I);
63+
return !isa<DebugValueInst>(inst) && !isa<DebugValueAddrInst>(inst);
6464
}

‎lib/SIL/Verifier/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
target_sources(swiftSIL PRIVATE
2+
DebugInfoVerifier.cpp
23
LoadBorrowImmutabilityChecker.cpp
34
LinearLifetimeChecker.cpp
45
MemoryLifetimeVerifier.cpp

‎lib/SIL/Verifier/DebugInfoVerifier.cpp‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//===--- DebugInfoVerifier.cpp --------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
///
13+
/// \file
14+
///
15+
/// Utility verifier code for validating debug info.
16+
///
17+
//===----------------------------------------------------------------------===//
18+
19+
#include "swift/SIL/SILDebugScope.h"
20+
#include "swift/SIL/SILInstruction.h"
21+
22+
using namespace swift;
23+
24+
//===----------------------------------------------------------------------===//
25+
// MARK: Verify SILInstruction Debug Info
26+
//===----------------------------------------------------------------------===//
27+
28+
void SILInstruction::verifyDebugInfo() const {
29+
auto require = [&](bool reqt, StringRef message) {
30+
if (!reqt) {
31+
llvm::errs() << message << "\n";
32+
assert(false && "invoking standard assertion failure");
33+
}
34+
};
35+
36+
// Check the location kind.
37+
SILLocation loc = getLoc();
38+
SILLocation::LocationKind locKind = loc.getKind();
39+
SILInstructionKind instKind = getKind();
40+
41+
// Regular locations are allowed on all instructions.
42+
if (locKind == SILLocation::RegularKind)
43+
return;
44+
45+
if (locKind == SILLocation::ReturnKind ||
46+
locKind == SILLocation::ImplicitReturnKind)
47+
require(
48+
instKind == SILInstructionKind::BranchInst ||
49+
instKind == SILInstructionKind::ReturnInst ||
50+
instKind == SILInstructionKind::UnreachableInst,
51+
"return locations are only allowed on branch and return instructions");
52+
53+
if (locKind == SILLocation::ArtificialUnreachableKind)
54+
require(
55+
instKind == SILInstructionKind::UnreachableInst,
56+
"artificial locations are only allowed on Unreachable instructions");
57+
}

‎lib/SIL/Verifier/SILVerifier.cpp‎

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,67 +1204,55 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
12041204
}
12051205
}
12061206

1207-
void checkInstructionsSILLocation(SILInstruction *I) {
1208-
// Check the debug scope.
1209-
auto *DS = I->getDebugScope();
1210-
if (DS && !maybeScopeless(*I))
1211-
require(DS, "instruction has a location, but no scope");
1207+
void checkInstructionsSILLocation(SILInstruction *inst) {
1208+
// First verify structural debug info information.
1209+
inst->verifyDebugInfo();
12121210

1213-
require(!DS || DS->getParentFunction() == I->getFunction(),
1211+
// Check the debug scope.
1212+
auto *debugScope = inst->getDebugScope();
1213+
if (debugScope && !maybeScopeless(*inst))
1214+
require(debugScope, "instruction has a location, but no scope");
1215+
require(!debugScope ||
1216+
debugScope->getParentFunction() == inst->getFunction(),
12141217
"debug scope of instruction belongs to a different function");
12151218

1216-
// Check the location kind.
1217-
SILLocation L = I->getLoc();
1218-
SILLocation::LocationKind LocKind = L.getKind();
1219-
SILInstructionKind InstKind = I->getKind();
1220-
1221-
// Check that there is at most one debug variable defined
1222-
// for each argument slot. This catches SIL transformations
1223-
// that accidentally remove inline information (stored in the SILDebugScope)
1224-
// from debug-variable-carrying instructions.
1225-
if (!DS->InlinedCallSite) {
1226-
Optional<SILDebugVariable> VarInfo;
1227-
if (auto *DI = dyn_cast<AllocStackInst>(I))
1228-
VarInfo = DI->getVarInfo();
1229-
else if (auto *DI = dyn_cast<AllocBoxInst>(I))
1230-
VarInfo = DI->getVarInfo();
1231-
else if (auto *DI = dyn_cast<DebugValueInst>(I))
1232-
VarInfo = DI->getVarInfo();
1233-
else if (auto *DI = dyn_cast<DebugValueAddrInst>(I))
1234-
VarInfo = DI->getVarInfo();
1235-
1236-
if (VarInfo)
1237-
if (unsigned ArgNo = VarInfo->ArgNo) {
1238-
// It is a function argument.
1239-
if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty() && !VarInfo->Name.empty()) {
1240-
require(
1241-
DebugVars[ArgNo] == VarInfo->Name,
1219+
// Check that there is at most one debug variable defined for each argument
1220+
// slot if our debug scope is not an inlined call site.
1221+
//
1222+
// This catches SIL transformations that accidentally remove inline
1223+
// information (stored in the SILDebugScope) from debug-variable-carrying
1224+
// instructions.
1225+
if (debugScope->InlinedCallSite)
1226+
return;
1227+
1228+
Optional<SILDebugVariable> varInfo;
1229+
if (auto *di = dyn_cast<AllocStackInst>(inst))
1230+
varInfo = di->getVarInfo();
1231+
else if (auto *di = dyn_cast<AllocBoxInst>(inst))
1232+
varInfo = di->getVarInfo();
1233+
else if (auto *di = dyn_cast<DebugValueInst>(inst))
1234+
varInfo = di->getVarInfo();
1235+
else if (auto *di = dyn_cast<DebugValueAddrInst>(inst))
1236+
varInfo = di->getVarInfo();
1237+
1238+
if (!varInfo)
1239+
return;
1240+
1241+
if (unsigned argNum = varInfo->ArgNo) {
1242+
// It is a function argument.
1243+
if (argNum < DebugVars.size() && !DebugVars[argNum].empty() &&
1244+
!varInfo->Name.empty()) {
1245+
require(DebugVars[argNum] == varInfo->Name,
12421246
"Scope contains conflicting debug variables for one function "
12431247
"argument");
1244-
} else {
1245-
// Reserve enough space.
1246-
while (DebugVars.size() <= ArgNo) {
1247-
DebugVars.push_back(StringRef());
1248-
}
1249-
}
1250-
DebugVars[ArgNo] = VarInfo->Name;
1248+
} else {
1249+
// Reserve enough space.
1250+
while (DebugVars.size() <= argNum) {
1251+
DebugVars.push_back(StringRef());
1252+
}
12511253
}
1254+
DebugVars[argNum] = varInfo->Name;
12521255
}
1253-
1254-
// Regular locations are allowed on all instructions.
1255-
if (LocKind == SILLocation::RegularKind)
1256-
return;
1257-
1258-
if (LocKind == SILLocation::ReturnKind ||
1259-
LocKind == SILLocation::ImplicitReturnKind)
1260-
require(InstKind == SILInstructionKind::BranchInst ||
1261-
InstKind == SILInstructionKind::ReturnInst ||
1262-
InstKind == SILInstructionKind::UnreachableInst,
1263-
"return locations are only allowed on branch and return instructions");
1264-
1265-
if (LocKind == SILLocation::ArtificialUnreachableKind)
1266-
require(InstKind == SILInstructionKind::UnreachableInst,
1267-
"artificial locations are only allowed on Unreachable instructions");
12681256
}
12691257

12701258
/// Check that the types of this value producer are all legal in the function

0 commit comments

Comments
 (0)