Skip to content

Commit 747e318

Browse files
jrose-applejrose-apple
authored
Merge pull request #17564 from jrose-apple/recovery-is-a-SILly-idea
Start recovering from missing types in SIL deserialization rdar://problem/40899824
2 parents bf22d4c + f1b347c commit 747e318

File tree

12 files changed

+245
-58
lines changed

12 files changed

+245
-58
lines changed

‎lib/Serialization/Deserialization.cpp

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,6 @@ void TypeError::anchor() {}
129129
const char ExtensionError::ID = '\0';
130130
void ExtensionError::anchor() {}
131131

132-
LLVM_NODISCARD
133-
static std::unique_ptr<llvm::ErrorInfoBase> takeErrorInfo(llvm::Error error) {
134-
std::unique_ptr<llvm::ErrorInfoBase> result;
135-
llvm::handleAllErrors(std::move(error),
136-
[&](std::unique_ptr<llvm::ErrorInfoBase> info) {
137-
result = std::move(info);
138-
});
139-
return result;
140-
}
141-
142-
143132
/// Skips a single record in the bitstream.
144133
///
145134
/// Returns true if the next entry is a record of type \p recordKind.
@@ -4756,27 +4745,42 @@ Expected<Type> ModuleFile::getTypeChecked(TypeID TID) {
47564745
}
47574746

47584747
auto processParameter = [&](TypeID typeID, uint64_t rawConvention)
4759-
-> Optional<SILParameterInfo> {
4748+
-> llvm::Expected<SILParameterInfo> {
47604749
auto convention = getActualParameterConvention(rawConvention);
4761-
auto type = getType(typeID);
4762-
if (!convention || !type) return None;
4763-
return SILParameterInfo(type->getCanonicalType(), *convention);
4750+
if (!convention) {
4751+
error();
4752+
llvm_unreachable("an error is a fatal exit at this point");
4753+
}
4754+
auto type = getTypeChecked(typeID);
4755+
if (!type)
4756+
return type.takeError();
4757+
return SILParameterInfo(type.get()->getCanonicalType(), *convention);
47644758
};
47654759

47664760
auto processYield = [&](TypeID typeID, uint64_t rawConvention)
4767-
-> Optional<SILYieldInfo> {
4761+
-> llvm::Expected<SILYieldInfo> {
47684762
auto convention = getActualParameterConvention(rawConvention);
4769-
auto type = getType(typeID);
4770-
if (!convention || !type) return None;
4771-
return SILYieldInfo(type->getCanonicalType(), *convention);
4763+
if (!convention) {
4764+
error();
4765+
llvm_unreachable("an error is a fatal exit at this point");
4766+
}
4767+
auto type = getTypeChecked(typeID);
4768+
if (!type)
4769+
return type.takeError();
4770+
return SILYieldInfo(type.get()->getCanonicalType(), *convention);
47724771
};
47734772

47744773
auto processResult = [&](TypeID typeID, uint64_t rawConvention)
4775-
-> Optional<SILResultInfo> {
4774+
-> llvm::Expected<SILResultInfo> {
47764775
auto convention = getActualResultConvention(rawConvention);
4777-
auto type = getType(typeID);
4778-
if (!convention || !type) return None;
4779-
return SILResultInfo(type->getCanonicalType(), *convention);
4776+
if (!convention) {
4777+
error();
4778+
llvm_unreachable("an error is a fatal exit at this point");
4779+
}
4780+
auto type = getTypeChecked(typeID);
4781+
if (!type)
4782+
return type.takeError();
4783+
return SILResultInfo(type.get()->getCanonicalType(), *convention);
47804784
};
47814785

47824786
// Bounds check. FIXME: overflow
@@ -4795,11 +4799,9 @@ Expected<Type> ModuleFile::getTypeChecked(TypeID TID) {
47954799
auto typeID = variableData[nextVariableDataIndex++];
47964800
auto rawConvention = variableData[nextVariableDataIndex++];
47974801
auto param = processParameter(typeID, rawConvention);
4798-
if (!param) {
4799-
error();
4800-
return nullptr;
4801-
}
4802-
allParams.push_back(*param);
4802+
if (!param)
4803+
return param.takeError();
4804+
allParams.push_back(param.get());
48034805
}
48044806

48054807
// Process the yields.
@@ -4809,11 +4811,9 @@ Expected<Type> ModuleFile::getTypeChecked(TypeID TID) {
48094811
auto typeID = variableData[nextVariableDataIndex++];
48104812
auto rawConvention = variableData[nextVariableDataIndex++];
48114813
auto yield = processYield(typeID, rawConvention);
4812-
if (!yield) {
4813-
error();
4814-
return nullptr;
4815-
}
4816-
allYields.push_back(*yield);
4814+
if (!yield)
4815+
return yield.takeError();
4816+
allYields.push_back(yield.get());
48174817
}
48184818

48194819
// Process the results.
@@ -4823,23 +4823,20 @@ Expected<Type> ModuleFile::getTypeChecked(TypeID TID) {
48234823
auto typeID = variableData[nextVariableDataIndex++];
48244824
auto rawConvention = variableData[nextVariableDataIndex++];
48254825
auto result = processResult(typeID, rawConvention);
4826-
if (!result) {
4827-
error();
4828-
return nullptr;
4829-
}
4830-
allResults.push_back(*result);
4826+
if (!result)
4827+
return result.takeError();
4828+
allResults.push_back(result.get());
48314829
}
48324830

48334831
// Process the error result.
48344832
Optional<SILResultInfo> errorResult;
48354833
if (hasErrorResult) {
48364834
auto typeID = variableData[nextVariableDataIndex++];
48374835
auto rawConvention = variableData[nextVariableDataIndex++];
4838-
errorResult = processResult(typeID, rawConvention);
4839-
if (!errorResult) {
4840-
error();
4841-
return nullptr;
4842-
}
4836+
auto maybeErrorResult = processResult(typeID, rawConvention);
4837+
if (!maybeErrorResult)
4838+
return maybeErrorResult.takeError();
4839+
errorResult = maybeErrorResult.get();
48434840
}
48444841

48454842
Optional<ProtocolConformanceRef> witnessMethodConformance;

‎lib/Serialization/DeserializationErrors.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,41 @@ class ExtensionError : public llvm::ErrorInfo<ExtensionError> {
355355
}
356356
};
357357

358+
class SILEntityError : public llvm::ErrorInfo<SILEntityError> {
359+
friend ErrorInfo;
360+
static const char ID;
361+
void anchor() override;
362+
363+
std::unique_ptr<ErrorInfoBase> underlyingReason;
364+
StringRef name;
365+
public:
366+
SILEntityError(StringRef name, std::unique_ptr<ErrorInfoBase> reason)
367+
: underlyingReason(std::move(reason)), name(name) {}
368+
369+
void log(raw_ostream &OS) const override {
370+
OS << "could not deserialize SIL entity '" << name << "'";
371+
if (underlyingReason) {
372+
OS << ": ";
373+
underlyingReason->log(OS);
374+
}
375+
}
376+
377+
std::error_code convertToErrorCode() const override {
378+
return llvm::inconvertibleErrorCode();
379+
}
380+
};
381+
382+
LLVM_NODISCARD
383+
static inline std::unique_ptr<llvm::ErrorInfoBase>
384+
takeErrorInfo(llvm::Error error) {
385+
std::unique_ptr<llvm::ErrorInfoBase> result;
386+
llvm::handleAllErrors(std::move(error),
387+
[&](std::unique_ptr<llvm::ErrorInfoBase> info) {
388+
result = std::move(info);
389+
});
390+
return result;
391+
}
392+
358393
class PrettyStackTraceModuleFile : public llvm::PrettyStackTraceEntry {
359394
const char *Action;
360395
const ModuleFile &MF;

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 78 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,16 @@
1212

1313
#define DEBUG_TYPE "deserialize"
1414
#include "DeserializeSIL.h"
15+
16+
#include "DeserializationErrors.h"
17+
#include "SILFormat.h"
18+
1519
#include "swift/Basic/Defer.h"
1620
#include "swift/Basic/PrettyStackTrace.h"
1721
#include "swift/AST/GenericSignature.h"
1822
#include "swift/AST/ProtocolConformance.h"
1923
#include "swift/AST/PrettyStackTrace.h"
2024
#include "swift/Serialization/ModuleFile.h"
21-
#include "SILFormat.h"
2225
#include "swift/SIL/SILArgument.h"
2326
#include "swift/SIL/SILBuilder.h"
2427
#include "swift/SIL/SILDebugScope.h"
@@ -38,6 +41,9 @@ using namespace swift::serialization;
3841
using namespace swift::serialization::sil_block;
3942
using namespace llvm::support;
4043

44+
const char SILEntityError::ID = '\0';
45+
void SILEntityError::anchor() {}
46+
4147
STATISTIC(NumDeserializedFunc, "Number of deserialized SIL functions");
4248

4349
static Optional<StringLiteralInst::Encoding>
@@ -339,7 +345,14 @@ SILFunction *SILDeserializer::getFuncForReference(StringRef name,
339345
// Otherwise, look for a function with this name in the module.
340346
auto iter = FuncTable->find(name);
341347
if (iter != FuncTable->end()) {
342-
fn = readSILFunction(*iter, nullptr, name, /*declarationOnly*/ true);
348+
auto maybeFn = readSILFunctionChecked(*iter, nullptr, name,
349+
/*declarationOnly*/ true);
350+
if (maybeFn) {
351+
fn = maybeFn.get();
352+
} else {
353+
// Ignore the failure; we'll synthesize a bogus function instead.
354+
llvm::consumeError(maybeFn.takeError());
355+
}
343356
}
344357
}
345358

@@ -362,7 +375,15 @@ SILFunction *SILDeserializer::getFuncForReference(StringRef name) {
362375
if (iter == FuncTable->end())
363376
return nullptr;
364377

365-
return readSILFunction(*iter, nullptr, name, /*declarationOnly*/ true);
378+
auto maybeFn = readSILFunctionChecked(*iter, nullptr, name,
379+
/*declarationOnly*/ true);
380+
if (!maybeFn) {
381+
// Ignore the failure and just pretend the function doesn't exist
382+
llvm::consumeError(maybeFn.takeError());
383+
return nullptr;
384+
}
385+
386+
return maybeFn.get();
366387
}
367388

368389
/// Helper function to find a SILGlobalVariable given its name. It first checks
@@ -386,6 +407,19 @@ SILFunction *SILDeserializer::readSILFunction(DeclID FID,
386407
StringRef name,
387408
bool declarationOnly,
388409
bool errorIfEmptyBody) {
410+
llvm::Expected<SILFunction *> deserialized =
411+
readSILFunctionChecked(FID, existingFn, name, declarationOnly,
412+
errorIfEmptyBody);
413+
if (!deserialized) {
414+
MF->fatal(deserialized.takeError());
415+
}
416+
return deserialized.get();
417+
}
418+
419+
llvm::Expected<SILFunction *>
420+
SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
421+
StringRef name, bool declarationOnly,
422+
bool errorIfEmptyBody) {
389423
// We can't deserialize function bodies after IRGen lowering passes have
390424
// happened since other definitions in the module will no longer be in
391425
// canonical SIL form.
@@ -444,7 +478,16 @@ SILFunction *SILDeserializer::readSILFunction(DeclID FID,
444478
MF->error();
445479
return nullptr;
446480
}
447-
auto ty = getSILType(MF->getType(funcTyID), SILValueCategory::Object);
481+
auto astType = MF->getTypeChecked(funcTyID);
482+
if (!astType) {
483+
if (!existingFn || errorIfEmptyBody) {
484+
return llvm::make_error<SILEntityError>(
485+
name, takeErrorInfo(astType.takeError()));
486+
}
487+
llvm::consumeError(astType.takeError());
488+
return existingFn;
489+
}
490+
auto ty = getSILType(astType.get(), SILValueCategory::Object);
448491
if (!ty.is<SILFunctionType>()) {
449492
DEBUG(llvm::dbgs() << "not a function type for SILFunction\n");
450493
MF->error();
@@ -2396,14 +2439,21 @@ SILFunction *SILDeserializer::lookupSILFunction(SILFunction *InFunc) {
23962439
if (iter == FuncTable->end())
23972440
return nullptr;
23982441

2399-
auto Func = readSILFunction(*iter, InFunc, name, /*declarationOnly*/ false);
2400-
if (Func) {
2442+
auto maybeFunc = readSILFunctionChecked(*iter, InFunc, name,
2443+
/*declarationOnly*/ false);
2444+
if (!maybeFunc) {
2445+
// Ignore the error; treat it as if we didn't have a definition.
2446+
llvm::consumeError(maybeFunc.takeError());
2447+
return nullptr;
2448+
}
2449+
2450+
if (maybeFunc.get()) {
24012451
DEBUG(llvm::dbgs() << "Deserialize SIL:\n";
2402-
Func->dump());
2403-
assert(InFunc->getName() == Func->getName());
2452+
maybeFunc.get()->dump());
2453+
assert(InFunc->getName() == maybeFunc.get()->getName());
24042454
}
24052455

2406-
return Func;
2456+
return maybeFunc.get();
24072457
}
24082458

24092459
/// Check for existence of a function with a given name and required linkage.
@@ -2480,11 +2530,20 @@ SILFunction *SILDeserializer::lookupSILFunction(StringRef name,
24802530
if (iter == FuncTable->end())
24812531
return nullptr;
24822532

2483-
auto Func = readSILFunction(*iter, nullptr, name, declarationOnly);
2484-
if (Func)
2533+
auto maybeFunc = readSILFunctionChecked(*iter, nullptr, name,
2534+
declarationOnly);
2535+
2536+
if (!maybeFunc) {
2537+
// Ignore the error; treat it as if we didn't have a definition.
2538+
llvm::consumeError(maybeFunc.takeError());
2539+
return nullptr;
2540+
}
2541+
2542+
if (maybeFunc.get()) {
24852543
DEBUG(llvm::dbgs() << "Deserialize SIL:\n";
2486-
Func->dump());
2487-
return Func;
2544+
maybeFunc.get()->dump());
2545+
}
2546+
return maybeFunc.get();
24882547
}
24892548

24902549
SILGlobalVariable *SILDeserializer::readGlobalVar(StringRef Name) {
@@ -2580,8 +2639,12 @@ void SILDeserializer::getAllSILFunctions() {
25802639
auto DI = FuncTable->find(*KI);
25812640
assert(DI != FuncTable->end() && "There should never be a key without data.");
25822641

2583-
readSILFunction(*DI, nullptr, *KI, false,
2584-
false/*errorIfEmptyBody*/);
2642+
auto maybeFunc = readSILFunctionChecked(*DI, nullptr, *KI, false,
2643+
false/*errorIfEmptyBody*/);
2644+
if (!maybeFunc) {
2645+
// Ignore the error; treat it as if we didn't have a definition.
2646+
llvm::consumeError(maybeFunc.takeError());
2647+
}
25852648
}
25862649
}
25872650

‎lib/Serialization/DeserializeSIL.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ namespace swift {
8181
SILFunction *readSILFunction(serialization::DeclID, SILFunction *InFunc,
8282
StringRef Name, bool declarationOnly,
8383
bool errorIfEmptyBody = true);
84+
/// Read a SIL function.
85+
llvm::Expected<SILFunction *>
86+
readSILFunctionChecked(serialization::DeclID, SILFunction *InFunc,
87+
StringRef Name, bool declarationOnly,
88+
bool errorIfEmptyBody = true);
89+
8490
/// Read a SIL basic block within a given SIL function.
8591
SILBasicBlock *readSILBasicBlock(SILFunction *Fn,
8692
SILBasicBlock *Prev,

‎test/SIL/Serialization/Recovery/Inputs/bad-modules/Types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// struct SoonToBeMissing {
2+
// int value;
3+
// };

‎test/SIL/Serialization/Recovery/Inputs/bad-modules/module.modulemap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module Types { header "Types.h" }

‎test/SIL/Serialization/Recovery/Inputs/good-modules/Types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
struct SoonToBeMissing {
2+
int value;
3+
};

‎test/SIL/Serialization/Recovery/Inputs/good-modules/module.modulemap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module Types { header "Types.h" }

‎test/SIL/Serialization/Recovery/function.sil

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -parse-sil %s -emit-sib -o %t/Library.sib -module-name Library -I %S/Inputs/good-modules -parse-stdlib
3+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/good-modules | %FileCheck %s
4+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules | %FileCheck -check-prefix=CHECK-RECOVERY %s
5+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules | %FileCheck -check-prefix=CHECK-RECOVERY-NEGATIVE %s
6+
7+
// CHECK-LABEL: sil_stage raw
8+
// CHECK-RECOVERY-LABEL: sil_stage raw
9+
10+
sil_stage raw
11+
import Types
12+
13+
// CHECK-LABEL: sil @missingParam : $@convention(thin) (SoonToBeMissing) -> () {
14+
// CHECK-RECOVERY-NEGATIVE-NOT: sil @missingParam
15+
sil @missingParam : $@convention(thin) (SoonToBeMissing) -> () {
16+
entry(%arg: $SoonToBeMissing):
17+
%9999 = tuple()
18+
return %9999 : $()
19+
}
20+
21+
// CHECK-LABEL: sil @missingResult : $@convention(thin) () -> SoonToBeMissing {
22+
// CHECK-RECOVERY-NEGATIVE-NOT: sil @missingResult
23+
sil @missingResult : $@convention(thin) () -> (SoonToBeMissing) {
24+
entry:
25+
unreachable
26+
}

‎validation-test/Serialization/Inputs/custom-modules/module.modulemap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module rdar40899824Helper { header "rdar40899824Helper.h" }

0 commit comments

Comments
 (0)