[Serialization] Fix deserializing opaque types for computed properties and subscripts

A client shouldn't know about the underlying type of an opaque type
unless it can see the body of the naming decl. Attempting to read it can
lead to accessing a hidden dependency and a compiler crash.

This was protected by a check specific to function decls but var decls
and subscripts were not handled. To support them we have to move this
logic to the writer side where we have access to the full
AbstractStorageDecl and write in the swifmodule whether the underlying
type should be visible outside of the module.

rdar://117607906

Author: xymus

include/swift/AST/Decl.h

@@ -3325,6 +3325,9 @@ class OpaqueTypeDecl final :
     };
   }
 
+  /// Should the underlying type be visible to clients outside of the module?
+  bool exportUnderlyingType() const;
+
   /// The substitutions that map the generic parameters of the opaque type to
   /// the unique underlying types, when that information is known.
   llvm::Optional<SubstitutionMap> getUniqueUnderlyingTypeSubstitutions() const {

lib/AST/Decl.cpp

@@ -9548,6 +9548,25 @@ GenericTypeParamDecl *OpaqueTypeDecl::getExplicitGenericParam(
   return genericParamType->getDecl();
 }
 
+bool OpaqueTypeDecl::exportUnderlyingType() const {
+  auto mod = getDeclContext()->getParentModule();
+  if (mod->getResilienceStrategy() != ResilienceStrategy::Resilient)
+    return true;
+
+  ValueDecl *namingDecl = getNamingDecl();
+  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl))
+    return AFD->getResilienceExpansion() == ResilienceExpansion::Minimal;
+
+  if (auto *ASD = dyn_cast<AbstractStorageDecl>(namingDecl)) {
+    for (auto *accessor : ASD->getAllAccessors())
+      if (accessor->getResilienceExpansion() == ResilienceExpansion::Minimal)
+        return true;
+    return false;
+  }
+
+  llvm_unreachable("The naming decl is expected to be either an AFD or ASD");
+}
+
 llvm::Optional<unsigned>
 OpaqueTypeDecl::getAnonymousOpaqueParamOrdinal(TypeRepr *repr) const {
   assert(NamingDeclAndHasOpaqueReturnTypeRepr.getInt() &&

lib/Serialization/Deserialization.cpp

@@ -4221,11 +4221,13 @@ class DeclDeserializer {
     GenericSignatureID genericSigID;
     SubstitutionMapID underlyingTypeSubsID;
     uint8_t rawAccessLevel;
+    bool exportUnderlyingType;
     decls_block::OpaqueTypeLayout::readRecord(scratch, contextID,
                                               namingDeclID, interfaceSigID,
                                               interfaceTypeID, genericSigID,
                                               underlyingTypeSubsID,
-                                              rawAccessLevel);
+                                              rawAccessLevel,
+                                              exportUnderlyingType);
 
     auto declContext = MF.getDeclContext(contextID);
     auto interfaceSigOrErr = MF.getGenericSignatureChecked(interfaceSigID);
@@ -4261,16 +4263,25 @@ class DeclDeserializer {
     else
       opaqueDecl->setGenericSignature(GenericSignature());
 
-    auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl);
-    if (MF.getResilienceStrategy() == ResilienceStrategy::Resilient &&
-        !MF.FileContext->getParentModule()->isMainModule() &&
-        AFD && AFD->getResilienceExpansion() != ResilienceExpansion::Minimal) {
+    if (!MF.FileContext->getParentModule()->isMainModule() &&
+        !exportUnderlyingType) {
       // Do not try to read the underlying type information if the function
       // is not inlinable in clients. This reflects the swiftinterface behavior
       // in where clients are only aware of the underlying type when the body
       // of the function is public.
+      LLVM_DEBUG(
+        llvm::dbgs() << "Ignoring underlying information for opaque type of '";
+        llvm::dbgs() << namingDecl->getName();
+        llvm::dbgs() << "'\n";
+        );
 
     } else if (underlyingTypeSubsID) {
+      LLVM_DEBUG(
+        llvm::dbgs() << "Loading underlying information for opaque type of '";
+        llvm::dbgs() << namingDecl->getName();
+        llvm::dbgs() << "'\n";
+        );
+
       auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
       if (!subMapOrError) {
         // If the underlying type references internal details, ignore it.

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 = 816; // throw_addr
+const uint16_t SWIFTMODULE_VERSION_MINOR = 817; // Opaque type export details
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1641,7 +1641,8 @@ namespace decls_block {
     TypeIDField, // interface type for opaque type
     GenericSignatureIDField, // generic environment
     SubstitutionMapIDField, // optional substitution map for underlying type
-    AccessLevelField // access level
+    AccessLevelField, // access level
+    BCFixed<1> // export underlying type details
     // trailed by generic parameters
     // trailed by conditional substitutions
   >;

lib/Serialization/Serialization.cpp

@@ -4490,11 +4490,14 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     }
     uint8_t rawAccessLevel =
         getRawStableAccessLevel(opaqueDecl->getFormalAccess());
+    bool exportDetails = opaqueDecl->exportUnderlyingType();
+
     unsigned abbrCode = S.DeclTypeAbbrCodes[OpaqueTypeLayout::Code];
     OpaqueTypeLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
                                  contextID.getOpaqueValue(), namingDeclID,
                                  interfaceSigID, interfaceTypeID, genericSigID,
-                                 underlyingSubsID, rawAccessLevel);
+                                 underlyingSubsID, rawAccessLevel,
+                                 exportDetails);
     writeGenericParams(opaqueDecl->getGenericParams());
 
     // Serialize all of the conditionally available substitutions expect the

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

@@ -8,9 +8,9 @@
 // RUN:   -enable-library-evolution -swift-version 5 \
 // RUN:   -emit-module-path %t/HiddenLib.swiftmodule
 // RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
-// RUN:   -enable-library-evolution \
+// RUN:   -swift-version 5 -enable-library-evolution \
 // RUN:   -emit-module-path %t/Lib.swiftmodule \
-// RUN:   -emit-module-interface-path %t/Lib.swiftinterface
+// RUN:   -emit-module-interface-path %t/Lib.swiftinterface -verify
 
 /// Build clients, with and without safety.
 // RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
@@ -107,12 +107,58 @@ public struct EV : V {
 
 @available(SwiftStdlib 5.1, *)
 public extension V {
+  private func referencedPrivateFunc(v: some V) -> some V { return v }
+
+  /// Hidden underlying types.
+// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivate()'
   @VB
   func opaqueReferencingPrivate() -> some V {
     referencedPrivateFunc(v: EV())
   }
 
-  private func referencedPrivateFunc(v: some V) -> some V { return v }
+// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVar'
+  var opaqueReferencingPrivateVar: some V {
+    referencedPrivateFunc(v: EV())
+  }
+
+// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVarPattern'
+  var opaqueReferencingPrivateVarPattern: some V {
+    get {
+      referencedPrivateFunc(v: EV())
+    }
+  }
+
+// NEEDED: Ignoring underlying information for opaque type of 'subscript(_:)'
+  subscript(v: some V) -> some V {
+    referencedPrivateFunc(v: v)
+  }
+
+  /// Visible underlying types.
+// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueFunc()'
+  @inlinable
+  func inlinableOpaqueFunc() -> some V { EV() }
+
+// NEEDED: Loading underlying information for opaque type of 'backdeployedOpaqueFunc()'
+  @backDeployed(before: SwiftStdlib 5.1) // expected-warning 4 {{'@backDeployed' is unsupported on a instance method with a 'some' return type}}
+  func backdeployedOpaqueFunc() -> some V { EV() }
+
+// NEEDED: Loading underlying information for opaque type of 'aeicOpaqueFunc()'
+  @_alwaysEmitIntoClient
+  func aeicOpaqueFunc() -> some V { EV() }
+
+// NEEDED: Loading underlying information for opaque type of 'transparentOpaqueFunc()'
+  @_transparent
+  func transparentOpaqueFunc() -> some V { EV() }
+
+// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueVar'
+  @inlinable
+  var inlinableOpaqueVar: some V { EV() }
+
+// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueVarPattern'
+  var inlinableOpaqueVarPattern: some V {
+      @inlinable
+      get { EV() }
+  }
 }
 
 //--- Client.swift
@@ -127,4 +173,15 @@ x.notAMember() // expected-error {{value of type 'PublicStruct' has no member 'n
 if #available(SwiftStdlib 5.1, *) {
   let v = EV()
   let _ = v.opaqueReferencingPrivate()
+  let _ = v.opaqueReferencingPrivateVar
+  let _ = v.opaqueReferencingPrivateVarPattern
+  let _ = v[v]
+
+  let _ = v.inlinableOpaqueFunc()
+  let _ = v.backdeployedOpaqueFunc()
+  let _ = v.aeicOpaqueFunc()
+  let _ = v.transparentOpaqueFunc()
+
+  let _ = v.inlinableOpaqueVar
+  let _ = v.inlinableOpaqueVarPattern
 }