Serialization: Protect readNormalProtocolConformanceXRef

Failures in `readNormalProtocolConformanceXRef` are usually caused by a
dependency change without the required rebuild of its dependents.
Display a proper error instead of crashing when encountering such an
issue during normal compilation. Recover silently when we can afford to,
during indexing or in LLDB.

Author: xymus

include/swift/AST/DiagnosticsSema.def

@@ -1037,6 +1037,12 @@ NOTE(modularization_issue_worked_around,none,
      "-experimental-force-workaround-broken-modules",
      ())
 
+ERROR(modularization_issue_conformance_xref_error,none,
+      "Conformance of %0 to %1 not found in referenced module %2",
+      (DeclName, DeclName, DeclName))
+NOTE(modularization_issue_conformance_xref_note,none,
+     "Breaks conformances of '%0' to %1",
+     (StringRef, DeclName))
 ERROR(reserved_member_name,none,
       "type member must not be named %0, since it would conflict with the"
       " 'foo.%1' expression", (const ValueDecl *, StringRef))

lib/Serialization/Deserialization.cpp

@@ -179,6 +179,8 @@ const char ModularizationError::ID = '\0';
 void ModularizationError::anchor() {}
 const char InvalidEnumValueError::ID = '\0';
 void InvalidEnumValueError::anchor() {}
+const char ConformanceXRefError::ID = '\0';
+void ConformanceXRefError::anchor() {}
 
 /// Skips a single record in the bitstream.
 ///
@@ -404,6 +406,16 @@ ModuleFile::diagnoseModularizationError(llvm::Error error,
   return outError;
 }
 
+void
+ConformanceXRefError::diagnose(const ModuleFile *MF,
+                               DiagnosticBehavior limit) const {
+  auto &diags = MF->getContext().Diags;
+  diags.diagnose(MF->getSourceLoc(),
+                 diag::modularization_issue_conformance_xref_error,
+                 name, protoName, expectedModule->getName())
+    .limitBehavior(limit);
+}
+
 llvm::Error ModuleFile::diagnoseFatal(llvm::Error error) const {
 
   auto &ctx = getContext();
@@ -860,7 +872,8 @@ ProtocolConformanceDeserializer::readSpecializedProtocolConformance(
     return subMapOrError.takeError();
   auto subMap = subMapOrError.get();
 
-  auto genericConformance = MF.getConformance(conformanceID);
+  ProtocolConformanceRef genericConformance;
+  UNWRAP(MF.getConformanceChecked(conformanceID), genericConformance);
 
   PrettyStackTraceDecl traceTo("... to", genericConformance.getRequirement());
   ++NumNormalProtocolConformancesLoaded;
@@ -892,8 +905,8 @@ ProtocolConformanceDeserializer::readInheritedProtocolConformance(
   PrettyStackTraceType trace(ctx, "reading inherited conformance for",
                              conformingType);
 
-  ProtocolConformanceRef inheritedConformance =
-    MF.getConformance(conformanceID);
+  ProtocolConformanceRef inheritedConformance;
+  UNWRAP(MF.getConformanceChecked(conformanceID), inheritedConformance);
   PrettyStackTraceDecl traceTo("... to",
                                inheritedConformance.getRequirement());
 
@@ -943,14 +956,16 @@ ProtocolConformanceDeserializer::readNormalProtocolConformanceXRef(
   ProtocolConformanceXrefLayout::readRecord(scratch, protoID, nominalID,
                                             moduleID);
 
-  auto maybeNominal = MF.getDeclChecked(nominalID);
-  if (!maybeNominal)
-    return maybeNominal.takeError();
-
-  auto nominal = cast<NominalTypeDecl>(maybeNominal.get());
+  Decl *maybeNominal;
+  UNWRAP(MF.getDeclChecked(nominalID), maybeNominal);
+  auto nominal = cast<NominalTypeDecl>(maybeNominal);
   PrettyStackTraceDecl trace("cross-referencing conformance for", nominal);
-  auto proto = cast<ProtocolDecl>(MF.getDecl(protoID));
+
+  Decl *maybeProto;
+  UNWRAP(MF.getDeclChecked(protoID), maybeProto);
+  auto proto = cast<ProtocolDecl>(maybeProto);
   PrettyStackTraceDecl traceTo("... to", proto);
+
   auto module = MF.getModule(moduleID);
 
   // FIXME: If the module hasn't been loaded, we probably don't want to fall
@@ -971,16 +986,26 @@ ProtocolConformanceDeserializer::readNormalProtocolConformanceXRef(
   // TODO: Sink Sendable derivation into the conformance lookup table
   if (proto->isSpecificProtocol(KnownProtocolKind::Sendable)) {
     auto conformanceRef = lookupConformance(nominal->getDeclaredInterfaceType(), proto);
-    if (!conformanceRef.isConcrete())
-      abort();
-    return conformanceRef.getConcrete();
+    if (conformanceRef.isConcrete())
+      return conformanceRef.getConcrete();
   } else {
     SmallVector<ProtocolConformance *, 2> conformances;
     nominal->lookupConformance(proto, conformances);
-    if (conformances.empty())
-      abort();
-    return conformances.front();
+    if (!conformances.empty())
+      return conformances.front();
   }
+
+  auto error = llvm::make_error<ConformanceXRefError>(
+                 nominal->getName(), proto->getName(), module);
+
+  if (!MF.enableExtendedDeserializationRecovery()) {
+    error = llvm::handleErrors(std::move(error),
+      [&](const ConformanceXRefError &error) -> llvm::Error {
+        error.diagnose(&MF);
+        return llvm::make_error<ConformanceXRefError>(std::move(error));
+      });
+  }
+  return error;
 }
 
 Expected<ProtocolConformance *>
@@ -7863,7 +7888,7 @@ Expected<Type> DESERIALIZE_TYPE(SIL_FUNCTION_TYPE)(
   ProtocolConformanceRef witnessMethodConformance;
   if (*representation == swift::SILFunctionTypeRepresentation::WitnessMethod) {
     auto conformanceID = variableData[nextVariableDataIndex++];
-    witnessMethodConformance = MF.getConformance(conformanceID);
+    UNWRAP(MF.getConformanceChecked(conformanceID), witnessMethodConformance);
   }
 
   GenericSignature invocationSig =
@@ -8643,21 +8668,37 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
       scratch, protoID, contextID, typeCount, valueCount, conformanceCount,
       rawOptions, rawIDs);
 
+  const ProtocolDecl *proto = conformance->getProtocol();
+
   // Read requirement signature conformances.
   SmallVector<ProtocolConformanceRef, 4> reqConformances;
   for (auto conformanceID : rawIDs.slice(0, conformanceCount)) {
     auto maybeConformance = getConformanceChecked(conformanceID);
     if (maybeConformance) {
       reqConformances.push_back(maybeConformance.get());
     } else if (getContext().LangOpts.EnableDeserializationRecovery) {
-      diagnoseAndConsumeError(maybeConformance.takeError());
+      llvm::Error error = maybeConformance.takeError();
+      if (error.isA<ConformanceXRefError>() &&
+          !enableExtendedDeserializationRecovery()) {
+
+        std::string typeStr = conformance->getType()->getString();
+        auto &diags = getContext().Diags;
+        diags.diagnose(getSourceLoc(),
+                       diag::modularization_issue_conformance_xref_note,
+                       typeStr, proto->getName());
+
+        consumeError(std::move(error));
+        conformance->setInvalid();
+        return;
+      }
+
+      diagnoseAndConsumeError(std::move(error));
       reqConformances.push_back(ProtocolConformanceRef::forInvalid());
     } else {
       fatal(maybeConformance.takeError());
     }
   }
 
-  const ProtocolDecl *proto = conformance->getProtocol();
   if (proto->isObjC() && getContext().LangOpts.EnableDeserializationRecovery) {
     // Don't crash if inherited protocols are added or removed.
     // This is limited to Objective-C protocols because we know their only

lib/Serialization/DeserializationErrors.h

@@ -677,6 +677,36 @@ class InvalidEnumValueError
   }
 };
 
+// Cross-reference to a conformance was not found where it was expected.
+class ConformanceXRefError : public llvm::ErrorInfo<ConformanceXRefError,
+                                                    DeclDeserializationError> {
+  friend ErrorInfo;
+  static const char ID;
+  void anchor() override;
+
+  DeclName protoName;
+  const ModuleDecl *expectedModule;
+
+public:
+  ConformanceXRefError(Identifier name, Identifier protoName,
+                       const ModuleDecl *expectedModule):
+                     protoName(protoName), expectedModule(expectedModule) {
+    this->name = name;
+  }
+
+  void diagnose(const ModuleFile *MF,
+                DiagnosticBehavior limit = DiagnosticBehavior::Fatal) const;
+
+  void log(raw_ostream &OS) const override {
+    OS << "Conformance of '" << name << "' to '" << protoName
+       << "' not found in module '" << expectedModule->getName() << "'";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
 class PrettyStackTraceModuleFile : public llvm::PrettyStackTraceEntry {
   const char *Action;
   const ModuleFile &MF;

test/Serialization/Recovery/conformance-xref.swift

@@ -0,0 +1,82 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+/// Reference build: Build libs and client for a normal build.
+// RUN: %target-swift-frontend -emit-module %t/ChangingLib.swift -I %t \
+// RUN:   -emit-module-path %t/ChangingLib.swiftmodule
+// RUN: %target-swift-frontend -emit-module %t/MiddleLib.swift -I %t \
+// RUN:   -emit-module-path %t/MiddleLib.swiftmodule
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t
+
+/// Case 1: Remove a conformance from one lib, leave the other as stale and
+/// rebuild client. Error on the missing conformance.
+// RUN: %target-swift-frontend -emit-module %t/ChangingLib.swift -I %t \
+// RUN:   -emit-module-path %t/ChangingLib.swiftmodule \
+// RUN:   -D DROP_CONFORMANCE
+// RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   2> %t/errors.log
+// RUN: %FileCheck %s --input-file=%t/errors.log \
+// RUN:    --check-prefix CHECK-REMARK-CONFORMANCE
+
+/// No errors when extended recovery is enabled.
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -experimental-allow-module-with-compiler-errors -verify
+
+
+//--- ChangingLib.swift
+
+public protocol SimpleProto {
+  func successor() -> Self
+}
+
+#if DROP_CONFORMANCE
+// CHECK-REMARK-CONFORMANCE: MiddleLib.swiftmodule:1:1: error: Conformance of 'Counter' to 'SimpleProto' not found in referenced module 'ChangingLib'
+// CHECK-REMARK-CONFORMANCE: note: Breaks conformances of 'OneToAThousand' to 'ProtoUser'
+#else
+extension Counter: SimpleProto {}
+#endif
+
+public protocol ProtoUser {
+  associatedtype Impl: SimpleProto
+
+  var start: Impl { get }
+
+  subscript(_: Impl) -> Element { get }
+}
+
+public struct Counter<T> {
+  public var value = 0
+
+  public init(value: Int) { self.value = value }
+
+  public func successor() -> Counter {
+    return Counter(value: value + 1)
+  }
+}
+
+//--- MiddleLib.swift
+
+import ChangingLib
+
+// Instantiate Counter<Int>, relying on Counter's adoption of SimpleProto.
+public struct OneToAThousand : ProtoUser {
+  public typealias Impl = Counter<Int>
+
+  public var start: Impl {
+    return Impl(value: 1)
+  }
+
+  public subscript(i: Impl) -> Int {
+    return i.value
+  }
+
+  public init() {}
+}
+
+//--- Client.swift
+
+import MiddleLib
+
+var test = OneToAThousand()
+var s = test.start
+print(test[s])