Merge pull request #62886 from xymus/deser-safety-infra

[Serialization] Prepare the infrastructure for the deserialization safety feature

Author: xymus

include/swift/Basic/LangOptions.h

@@ -357,11 +357,15 @@ namespace swift {
     bool EnableTargetOSChecking = true;
 
     /// Whether to attempt to recover from missing cross-references and other
-    /// errors when deserializing from a Swift module.
+    /// errors when deserializing from a binary swiftmodule file.
     ///
-    /// This is a staging flag; eventually it will be removed.
+    /// This flag should only be used in testing.
     bool EnableDeserializationRecovery = true;
 
+    /// Enable early skipping deserialization of decls that are marked as
+    /// unsafe to read.
+    bool EnableDeserializationSafety = false;
+
     /// Whether to enable the new operator decl and precedencegroup lookup
     /// behavior. This is a staging flag, and will be removed in the future.
     bool EnableNewOperatorLookup = false;

include/swift/Option/FrontendOptions.td

@@ -564,6 +564,13 @@ def disable_deserialization_recovery :
   Flag<["-"], "disable-deserialization-recovery">,
   HelpText<"Don't attempt to recover from missing xrefs (etc) in swiftmodules">;
 
+def enable_deserialization_safety :
+  Flag<["-"], "enable-deserialization-safety">,
+  HelpText<"Avoid reading potentially unsafe decls in swiftmodules">;
+def disable_deserialization_safety :
+  Flag<["-"], "disable-deserialization-safety">,
+  HelpText<"Don't avoid reading potentially unsafe decls in swiftmodules">;
+
 def enable_experimental_string_processing :
   Flag<["-"], "enable-experimental-string-processing">,
   HelpText<"Enable experimental string processing">;

lib/Frontend/CompilerInvocation.cpp

@@ -500,6 +500,12 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
       = A->getOption().matches(OPT_enable_deserialization_recovery);
   }
 
+  if (auto A = Args.getLastArg(OPT_enable_deserialization_safety,
+                               OPT_disable_deserialization_safety)) {
+    Opts.EnableDeserializationSafety
+      = A->getOption().matches(OPT_enable_deserialization_safety);
+  }
+
   // Whether '/.../' regex literals are enabled. This implies experimental
   // string processing.
   if (Args.hasArg(OPT_enable_bare_slash_regex)) {

lib/Frontend/Frontend.cpp

@@ -274,6 +274,11 @@ bool CompilerInstance::setUpASTContextIfNeeded() {
 
   registerIRGenSILTransforms(*Context);
 
+  if (Invocation.getFrontendOptions().RequestedAction ==
+        FrontendOptions::ActionType::MergeModules ||
+      Invocation.getLangOptions().DebuggerSupport)
+    Invocation.getLangOptions().EnableDeserializationSafety = false;
+
   if (setUpModuleLoaders())
     return true;
 

lib/Serialization/DeclTypeRecordNodes.def

@@ -182,6 +182,8 @@ OTHER(LOCAL_DISCRIMINATOR, 127)
 OTHER(PRIVATE_DISCRIMINATOR, 128)
 OTHER(FILENAME_FOR_PRIVATE, 129)
 
+OTHER(DESERIALIZATION_SAFETY, 130)
+
 // 140 is unused; was ABSTRACT_PROTOCOL_CONFORMANCE
 OTHER(NORMAL_PROTOCOL_CONFORMANCE, 141)
 OTHER(SPECIALIZED_PROTOCOL_CONFORMANCE, 142)

lib/Serialization/Deserialization.cpp

@@ -155,6 +155,8 @@ const char DeclAttributesDidNotMatch::ID = '\0';
 void DeclAttributesDidNotMatch::anchor() {}
 const char InvalidRecordKindError::ID = '\0';
 void InvalidRecordKindError::anchor() {}
+const char UnsafeDeserializationError::ID = '\0';
+void UnsafeDeserializationError::anchor() {}
 
 /// Skips a single record in the bitstream.
 ///
@@ -4750,6 +4752,14 @@ ModuleFile::getDeclChecked(
       IDC->setDeclID(DID);
     }
   }
+
+  LLVM_DEBUG(
+    if (auto *VD = dyn_cast_or_null<ValueDecl>(declOrOffset.get())) {
+      llvm::dbgs() << "Deserialized: '";
+      llvm::dbgs() << VD->getName();
+      llvm::dbgs() << "'\n";
+    });
+
   return declOrOffset;
 }
 
@@ -5403,6 +5413,17 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
       IdentifierID filenameID;
       decls_block::FilenameForPrivateLayout::readRecord(scratch, filenameID);
       filenameForPrivate = MF.getIdentifierText(filenameID);
+    } else if (recordID == decls_block::DESERIALIZATION_SAFETY) {
+      IdentifierID declID;
+      decls_block::DeserializationSafetyLayout::readRecord(scratch, declID);
+
+      if (MF.getResilienceStrategy() == ResilienceStrategy::Resilient &&
+          MF.getContext().LangOpts.EnableDeserializationSafety) {
+        auto name = MF.getIdentifier(declID);
+        LLVM_DEBUG(llvm::dbgs() << "Skipping unsafe deserialization: '"
+                                << name << "'\n");
+        return llvm::make_error<UnsafeDeserializationError>(name);
+      }
     } else {
       return llvm::Error::success();
     }

lib/Serialization/DeserializationErrors.h

@@ -494,6 +494,27 @@ class InvalidRecordKindError :
   }
 };
 
+// Decl was not deserialized because it's an internal detail maked as unsafe
+// at serialization.
+class UnsafeDeserializationError : public llvm::ErrorInfo<UnsafeDeserializationError, DeclDeserializationError> {
+  friend ErrorInfo;
+  static const char ID;
+  void anchor() override;
+
+public:
+  UnsafeDeserializationError(Identifier name) {
+    this->name = name;
+  }
+
+  void log(raw_ostream &OS) const override {
+    OS << "Decl '" << name << "' is unsafe to deserialize";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
 LLVM_NODISCARD
 static inline std::unique_ptr<llvm::ErrorInfoBase>
 takeErrorInfo(llvm::Error error) {

lib/Serialization/ModuleFormat.h

@@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 731; // macro definition
+const uint16_t SWIFTMODULE_VERSION_MINOR = 732; // deserialization safety
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1809,6 +1809,11 @@ namespace decls_block {
     IdentifierIDField  // the file name, as an identifier
   >;
 
+  using DeserializationSafetyLayout = BCRecordLayout<
+    DESERIALIZATION_SAFETY,
+    IdentifierIDField // name to debug access to unsafe decl
+  >;
+
   using NormalProtocolConformanceLayout = BCRecordLayout<
     NORMAL_PROTOCOL_CONFORMANCE,
     DeclIDField, // the protocol

lib/Serialization/Serialization.cpp

@@ -5336,6 +5336,7 @@ void Serializer::writeAllDeclsAndTypes() {
   registerDeclTypeAbbr<LocalDiscriminatorLayout>();
   registerDeclTypeAbbr<PrivateDiscriminatorLayout>();
   registerDeclTypeAbbr<FilenameForPrivateLayout>();
+  registerDeclTypeAbbr<DeserializationSafetyLayout>();
   registerDeclTypeAbbr<MembersLayout>();
   registerDeclTypeAbbr<XRefLayout>();
 

test/Serialization/Safety/skip-internal-details.swift

@@ -0,0 +1,70 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// REQUIRES: asserts
+
+// RUN: %target-swift-frontend -emit-module %t/HiddenLib.swift \
+// RUN:   -enable-library-evolution \
+// RUN:   -emit-module-path %t/HiddenLib.swiftmodule
+
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
+// RUN:   -enable-library-evolution \
+// RUN:   -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -emit-module-interface-path %t/Lib.swiftinterface
+
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -verify -Xllvm -debug-only=Serialization 2>&1 \
+// RUN:   | %FileCheck --check-prefixes=NEEDED,UNSAFE %s
+
+// RUN: rm %t/Lib.swiftmodule
+
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -verify -Xllvm -debug-only=Serialization 2>&1 \
+// RUN:   | %FileCheck --check-prefixes=NEEDED,CLEAN %s
+
+/// Decls part of the API needed by the client.
+// NEEDED-NOT: Deserialized: 'refToIOI()'
+// NEEDED: Deserialized: 'PublicStruct'
+// NEEDED: Deserialized: 'publicFunc()'
+
+/// Internal details dangerous to load.
+// UNSAFE: Deserialized: 'internalFunc()'
+// UNSAFE: Deserialized: 'privateFunc()'
+// UNSAFE: Deserialized: 'fileprivateFunc()'
+
+/// Decls removed by rebuilding from the swiftinterface.
+// CLEAN-NOT: Deserialized: 'internalFunc()'
+// CLEAN-NOT: Deserialized: 'privateFunc()'
+// CLEAN-NOT: Deserialized: 'fileprivateFunc()'
+
+//--- HiddenLib.swift
+
+public struct HiddenStruct {
+    public init() {}
+}
+
+//--- Lib.swift
+
+@_implementationOnly import HiddenLib
+
+public class PublicStruct {
+    public init() {}
+
+    public func publicFunc() {}
+    internal func internalFunc() {}
+    private func privateFunc() {}
+    fileprivate func fileprivateFunc() {}
+
+    internal func refToIOI() -> HiddenStruct {
+        return HiddenStruct();
+    }
+}
+
+//--- Client.swift
+
+import Lib
+
+var x = PublicStruct()
+
+// Trigger a typo correction that reads all members.
+x.notAMember() // expected-error {{value of type 'PublicStruct' has no member 'notAMember'}}

tools/lldb-moduleimport-test/lldb-moduleimport-test.cpp

@@ -63,6 +63,7 @@ static bool validateModule(
     llvm::outs() << "error: loadFromSerializedAST() failed\n";
     return false;
   }
+  CI.getLangOptions().EnableDeserializationSafety = false;
 
   if (Verbose) {
     if (!info.shortVersion.empty())

tools/sil-opt/SILOpt.cpp

@@ -562,6 +562,7 @@ int main(int argc, char **argv) {
   Invocation.getLangOptions().DisableAvailabilityChecking = true;
   Invocation.getLangOptions().EnableAccessControl = false;
   Invocation.getLangOptions().EnableObjCAttrRequiresFoundation = false;
+  Invocation.getLangOptions().EnableDeserializationSafety = false;
   if (auto overrideKind = getASTOverrideKind()) {
     Invocation.getLangOptions().ASTVerifierOverride = *overrideKind;
   }

tools/swift-ide-test/swift-ide-test.cpp

@@ -3002,6 +3002,10 @@ static int doPrintModules(const CompilerInvocation &InitInvok,
                           bool SynthesizeExtensions) {
   CompilerInvocation Invocation(InitInvok);
 
+  // Read everything from loaded modules, including internal details to
+  // test the behavior around non-public access levels.
+  Invocation.getLangOptions().EnableDeserializationSafety = false;
+
   CompilerInstance CI;
   // Display diagnostics to stderr.
   PrintingDiagnosticConsumer PrintDiags;