From da4e72cf2db1dbe931a774e5e720d34754462978 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Fri, 5 Dec 2025 16:55:41 -0800 Subject: [PATCH] SIL verifier: The `atInstruction`/`atArgument` parameter to `require` should not be optional. The underlying C++ code expects a non-null `Instruction*` or `SILArgument*` pointer, and most of the contextual information in a verifier error is derived from these arguments, so it doesn't really make sense for the Swift level interface to present these arguments as optional. --- .../Sources/SIL/Utilities/Verifier.swift | 47 ++++++++++++++----- include/swift/SIL/SILBridging.h | 6 +-- lib/SIL/Utils/SILBridging.cpp | 8 ++-- 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/SwiftCompilerSources/Sources/SIL/Utilities/Verifier.swift b/SwiftCompilerSources/Sources/SIL/Utilities/Verifier.swift index 3cf75f37333..e35d91f696e 100644 --- a/SwiftCompilerSources/Sources/SIL/Utilities/Verifier.swift +++ b/SwiftCompilerSources/Sources/SIL/Utilities/Verifier.swift @@ -18,7 +18,7 @@ private protocol VerifiableInstruction : Instruction { func verify(_ context: VerifierContext) } -private func require(_ condition: Bool, _ message: @autoclosure () -> String, atInstruction: Instruction? = nil) { +private func require(_ condition: Bool, _ message: @autoclosure () -> String, atInstruction: Instruction) { if !condition { let msg = message() msg._withBridgedStringRef { stringRef in @@ -27,6 +27,15 @@ private func require(_ condition: Bool, _ message: @autoclosure () -> String, at } } +private func require(_ condition: Bool, _ message: @autoclosure () -> String, atArgument: Argument) { + if !condition { + let msg = message() + msg._withBridgedStringRef { stringRef in + BridgedVerifier.verifierError(stringRef, atArgument.bridged) + } + } +} + struct VerifierContext: Context { let _bridged: BridgedContext } @@ -53,16 +62,21 @@ extension Function { private extension Instruction { func checkForwardingConformance() { if bridged.shouldBeForwarding() { - require(self is ForwardingInstruction, "instruction \(self)\nshould conform to ForwardingInstruction") + require(self is ForwardingInstruction, + "instruction \(self)\nshould conform to ForwardingInstruction", + atInstruction: self) } else { - require(!(self is ForwardingInstruction), "instruction \(self)\nshould not conform to ForwardingInstruction") + require(!(self is ForwardingInstruction), + "instruction \(self)\nshould not conform to ForwardingInstruction", + atInstruction: self) } } func checkGuaranteedResults() { for result in results where result.ownership == .guaranteed { require(BeginBorrowValue(result) != nil || self is ForwardingInstruction || result.isGuaranteedApplyResult, - "\(result) must either be a BeginBorrowValue or a ForwardingInstruction") + "\(result) must either be a BeginBorrowValue or a ForwardingInstruction", + atInstruction: self) } } } @@ -89,14 +103,17 @@ private extension Phi { var forwardingBorrowedFromFound = false for use in value.uses { require(use.instruction is BorrowedFromInst, - "guaranteed phi: \(self)\n has non borrowed-from use: \(use)") + "guaranteed phi: \(self)\n has non borrowed-from use: \(use)", + atArgument: self.value) if use.index == 0 { - require(!forwardingBorrowedFromFound, "phi \(self) has multiple forwarding borrowed-from uses") + require(!forwardingBorrowedFromFound, "phi \(self) has multiple forwarding borrowed-from uses", + atArgument: self.value) forwardingBorrowedFromFound = true } } require(forwardingBorrowedFromFound, - "missing forwarding borrowed-from user of guaranteed phi \(self)") + "missing forwarding borrowed-from user of guaranteed phi \(self)", + atArgument: self.value) } } @@ -104,7 +121,9 @@ extension BorrowedFromInst : VerifiableInstruction { func verify(_ context: VerifierContext) { for ev in enclosingValues { - require(ev.isValidEnclosingValueInBorrowedFrom, "invalid enclosing value in borrowed-from: \(ev)") + require(ev.isValidEnclosingValueInBorrowedFrom, + "invalid enclosing value in borrowed-from: \(ev)", + atInstruction: self) } var computedEVs = Stack(context) @@ -120,7 +139,8 @@ extension BorrowedFromInst : VerifiableInstruction { for computedEV in computedEVs { require(existingEVs.contains(computedEV), - "\(computedEV)\n missing in enclosing values of \(self)") + "\(computedEV)\n missing in enclosing values of \(self)", + atInstruction: self) } } } @@ -189,10 +209,12 @@ extension BeginAccessInst : VerifiableInstruction { extension VectorBaseAddrInst : VerifiableInstruction { func verify(_ context: VerifierContext) { require(vector.type.isBuiltinFixedArray, - "vector operand of vector_element_addr must be a Builtin.FixedArray") + "vector operand of vector_element_addr must be a Builtin.FixedArray", + atInstruction: self) require(type == vector.type.builtinFixedArrayElementType(in: parentFunction, maximallyAbstracted: true).addressType, - "result of vector_element_addr has wrong type") + "result of vector_element_addr has wrong type", + atInstruction: self) } } @@ -235,7 +257,8 @@ private struct MutatingUsesWalker : AddressDefUseWalker { if let bf = phi.borrowedFrom { linearLiveranges.pushIfNotVisited(bf) } else { - require(false, "missing borrowed-from for \(phi.value)") + require(false, "missing borrowed-from for \(phi.value)", + atArgument: phi.value) } } } diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index e2b729ab239..f87ed97d382 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -1629,11 +1629,11 @@ struct BridgedVerifier { static void registerVerifier(VerifyFunctionFn verifyFunctionFn); static void verifierError(BridgedStringRef message, - OptionalBridgedInstruction atInstruction); + BridgedInstruction atInstruction); static void verifierError(BridgedStringRef message, - OptionalBridgedArgument atArgument); + BridgedArgument atArgument); static void verifierError(BridgedStringRef message, - OptionalBridgedValue atValue); + BridgedValue atValue); }; struct BridgedUtilities { diff --git a/lib/SIL/Utils/SILBridging.cpp b/lib/SIL/Utils/SILBridging.cpp index e798f91d4a7..258718ec9f0 100644 --- a/lib/SIL/Utils/SILBridging.cpp +++ b/lib/SIL/Utils/SILBridging.cpp @@ -918,19 +918,19 @@ void BridgedVerifier::runSwiftFunctionVerification(SILFunction * _Nonnull f, SIL } void BridgedVerifier::verifierError(BridgedStringRef message, - OptionalBridgedInstruction atInstruction) { + BridgedInstruction atInstruction) { verificationFailure(message.unbridged(), atInstruction.unbridged(), /*extraContext=*/nullptr); } void BridgedVerifier::verifierError(BridgedStringRef message, - OptionalBridgedArgument atArgument) { - verificationFailure(message.unbridged(), atArgument.unbridged(), + BridgedArgument atArgument) { + verificationFailure(message.unbridged(), atArgument.getArgument(), /*extraContext=*/nullptr); } void BridgedVerifier::verifierError(BridgedStringRef message, - OptionalBridgedValue atValue) { + BridgedValue atValue) { verificationFailure(message.unbridged(), SILValue(atValue.getSILValue()), /*extraContext=*/nullptr); }