Skip to content

Commit 55963c0

Browse files
eecksteineeckstein
committed
SILVerifier: don't crash on verifier errors in the function header
The thing with `optional<VerifierErrorEmitterGuard>` didn't work as expected. This resulted in a null-pointer de-reference when printing a verifier error.
1 parent 94a0cb5 commit 55963c0

File tree

2 files changed

+89
-79
lines changed

2 files changed

+89
-79
lines changed

‎lib/SIL/Verifier/SILVerifier.cpp‎

Lines changed: 80 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -7311,95 +7311,96 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
73117311
void visitSILFunction(SILFunction *F) {
73127312
PrettyStackTraceSILFunction stackTrace("verifying", F);
73137313

7314-
std::optional<VerifierErrorEmitterGuard> functionTypeErrorGuard{{this, F}};
7315-
CanSILFunctionType FTy = F->getLoweredFunctionType();
7316-
verifySILFunctionType(FTy);
7317-
verifyParentFunctionSILFunctionType(FTy);
7318-
73197314
SILModule &mod = F->getModule();
7320-
bool embedded = mod.getASTContext().LangOpts.hasFeature(Feature::Embedded);
7321-
7322-
require(!F->isAnySerialized() || !mod.isSerialized() || mod.isParsedAsSerializedSIL(),
7323-
"cannot have a serialized function after the module has been serialized");
7315+
{
7316+
VerifierErrorEmitterGuard functionTypeErrorGuard(this, F);
7317+
CanSILFunctionType FTy = F->getLoweredFunctionType();
7318+
verifySILFunctionType(FTy);
7319+
verifyParentFunctionSILFunctionType(FTy);
7320+
7321+
bool embedded = mod.getASTContext().LangOpts.hasFeature(Feature::Embedded);
7322+
7323+
require(!F->isAnySerialized() || !mod.isSerialized() || mod.isParsedAsSerializedSIL(),
7324+
"cannot have a serialized function after the module has been serialized");
7325+
7326+
switch (F->getLinkage()) {
7327+
case SILLinkage::Public:
7328+
case SILLinkage::Package:
7329+
case SILLinkage::Shared:
7330+
require(F->isDefinition() || F->hasForeignBody(),
7331+
"public/package/shared function must have a body");
7332+
break;
7333+
case SILLinkage::PublicNonABI:
7334+
case SILLinkage::PackageNonABI:
7335+
require(F->isDefinition(),
7336+
"alwaysEmitIntoClient function must have a body");
7337+
require(F->isAnySerialized() || mod.isSerialized(),
7338+
"alwaysEmitIntoClient function must be serialized");
7339+
break;
7340+
case SILLinkage::Hidden:
7341+
case SILLinkage::Private:
7342+
require(F->isDefinition() || F->hasForeignBody(),
7343+
"internal/private function must have a body");
7344+
require(!F->isAnySerialized() || embedded,
7345+
"internal/private function cannot be serialized or serializable");
7346+
break;
7347+
case SILLinkage::PublicExternal:
7348+
require(F->isExternalDeclaration() ||
7349+
F->isAnySerialized() ||
7350+
mod.isSerialized(),
7351+
"public-external function definition must be serialized");
7352+
break;
7353+
case SILLinkage::PackageExternal:
7354+
require(F->isExternalDeclaration() ||
7355+
F->isAnySerialized() ||
7356+
mod.isSerialized(),
7357+
"package-external function definition must be serialized");
7358+
break;
7359+
case SILLinkage::HiddenExternal:
7360+
require(F->isExternalDeclaration() || embedded,
7361+
"hidden-external function cannot have a body");
7362+
break;
7363+
}
73247364

7325-
switch (F->getLinkage()) {
7326-
case SILLinkage::Public:
7327-
case SILLinkage::Package:
7328-
case SILLinkage::Shared:
7329-
require(F->isDefinition() || F->hasForeignBody(),
7330-
"public/package/shared function must have a body");
7331-
break;
7332-
case SILLinkage::PublicNonABI:
7333-
case SILLinkage::PackageNonABI:
7334-
require(F->isDefinition(),
7335-
"alwaysEmitIntoClient function must have a body");
7336-
require(F->isAnySerialized() || mod.isSerialized(),
7337-
"alwaysEmitIntoClient function must be serialized");
7338-
break;
7339-
case SILLinkage::Hidden:
7340-
case SILLinkage::Private:
7341-
require(F->isDefinition() || F->hasForeignBody(),
7342-
"internal/private function must have a body");
7343-
require(!F->isAnySerialized() || embedded,
7344-
"internal/private function cannot be serialized or serializable");
7345-
break;
7346-
case SILLinkage::PublicExternal:
7347-
require(F->isExternalDeclaration() ||
7348-
F->isAnySerialized() ||
7349-
mod.isSerialized(),
7350-
"public-external function definition must be serialized");
7351-
break;
7352-
case SILLinkage::PackageExternal:
7353-
require(F->isExternalDeclaration() ||
7354-
F->isAnySerialized() ||
7355-
mod.isSerialized(),
7356-
"package-external function definition must be serialized");
7357-
break;
7358-
case SILLinkage::HiddenExternal:
7359-
require(F->isExternalDeclaration() || embedded,
7360-
"hidden-external function cannot have a body");
7361-
break;
7362-
}
7365+
// Don't verify functions that were skipped. We are likely to see them in
7366+
// FunctionBodySkipping::NonInlinableWithoutTypes mode.
7367+
auto Ctx = F->getDeclContext();
7368+
if (Ctx) {
7369+
if (auto AFD = dyn_cast<AbstractFunctionDecl>(Ctx)) {
7370+
if (AFD->isBodySkipped())
7371+
return;
7372+
}
7373+
}
73637374

7364-
// Don't verify functions that were skipped. We are likely to see them in
7365-
// FunctionBodySkipping::NonInlinableWithoutTypes mode.
7366-
auto Ctx = F->getDeclContext();
7367-
if (Ctx) {
7368-
if (auto AFD = dyn_cast<AbstractFunctionDecl>(Ctx)) {
7369-
if (AFD->isBodySkipped())
7375+
if (F->isExternalDeclaration()) {
7376+
if (F->hasForeignBody())
73707377
return;
7371-
}
7372-
}
73737378

7374-
if (F->isExternalDeclaration()) {
7375-
if (F->hasForeignBody())
7379+
require(F->isAvailableExternally(),
7380+
"external declaration of internal SILFunction not allowed");
7381+
// If F is an external declaration, there is nothing further to do,
7382+
// return.
73767383
return;
7384+
}
73777385

7378-
require(F->isAvailableExternally(),
7379-
"external declaration of internal SILFunction not allowed");
7380-
// If F is an external declaration, there is nothing further to do,
7381-
// return.
7382-
return;
7383-
}
7384-
7385-
require(!FTy->hasErasedIsolation(),
7386-
"function declarations cannot have erased isolation");
7386+
require(!FTy->hasErasedIsolation(),
7387+
"function declarations cannot have erased isolation");
73877388

7388-
assert(!F->hasForeignBody());
7389+
assert(!F->hasForeignBody());
73897390

7390-
// Make sure that our SILFunction only has context generic params if our
7391-
// SILFunctionType is non-polymorphic.
7392-
if (F->getGenericEnvironment() &&
7393-
!F->getGenericEnvironment()->getGenericSignature()
7394-
->areAllParamsConcrete()) {
7395-
require(FTy->isPolymorphic(),
7396-
"non-generic function definitions cannot have a "
7397-
"generic environment");
7398-
} else {
7399-
require(!FTy->isPolymorphic(),
7400-
"generic function definition must have a generic environment");
7391+
// Make sure that our SILFunction only has context generic params if our
7392+
// SILFunctionType is non-polymorphic.
7393+
if (F->getGenericEnvironment() &&
7394+
!F->getGenericEnvironment()->getGenericSignature()
7395+
->areAllParamsConcrete()) {
7396+
require(FTy->isPolymorphic(),
7397+
"non-generic function definitions cannot have a "
7398+
"generic environment");
7399+
} else {
7400+
require(!FTy->isPolymorphic(),
7401+
"generic function definition must have a generic environment");
7402+
}
74017403
}
7402-
functionTypeErrorGuard.reset();
74037404

74047405
// Before verifying the body of the function, validate the SILUndef map to
74057406
// make sure that all SILUndef in the function's map point at the function

‎test/SIL/verifier_failures.sil‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,12 @@ bb2:
138138
%7 = end_apply %4 as $()
139139
unwind
140140
}
141+
142+
// CHECK-LABEL: Begin Error in function private_without_body
143+
// CHECK: SIL verification failed: internal/private function must have a body
144+
// CHECK: End Error in function private_without_body
145+
// CHECK-LABEL: Begin Error in function private_without_body
146+
// CHECK: SIL verification failed: external declaration of internal SILFunction not allowed
147+
// CHECK: End Error in function private_without_body
148+
sil private @private_without_body : $@convention(thin) () -> ()
149+

0 commit comments

Comments
 (0)