Merge pull request #27104 from slavapestov/sil-weak-linking-fix-5.1-08-28

Fix bad interaction between SIL deserialization and weak linking [5.1 08/28]

Author: slavapestov

include/swift/AST/Decl.h

@@ -975,9 +975,31 @@ class alignas(1 << DeclAlignInBits) Decl {
 
   bool isPrivateStdlibDecl(bool treatNonBuiltinProtocolsAsPublic = true) const;
 
-  /// Whether this declaration is weak-imported.
-  bool isWeakImported(ModuleDecl *fromModule,
-                      AvailabilityContext fromContext) const;
+  AvailabilityContext getAvailabilityForLinkage() const;
+
+  /// Whether this declaration or one of its outer contexts has the
+  /// @_weakLinked attribute.
+  bool isAlwaysWeakImported() const;
+
+  /// Whether this declaration is weak-imported from the given module,
+  /// either because of the presence of the @_weakLinked attribute, or
+  /// because of availability.
+  ///
+  /// Note that \p fromModule should either be the "main module" or
+  /// nullptr. (This is because when it is non-null, we query the
+  /// current deployment target, and not the deployment target that
+  /// the module was built with.)
+  ///
+  /// If \p fromModule is the main module, this returns false when the
+  /// declaration is part of the main module, or if the declaration is
+  /// at least as available as the current deployment target.
+  ///
+  /// If \p fromModule is null, we instead return true if the
+  /// declaration is meant to be weak linked with _some_ deployment
+  /// target; that is, the presence of the @_weakLinked attribute or
+  /// any kind of availability is enough, irrespective of the current
+  /// deployment target.
+  bool isWeakImported(ModuleDecl *fromModule) const;
 
   /// Returns true if the nature of this declaration allows overrides.
   /// Note that this does not consider whether it is final or whether

include/swift/AST/DiagnosticsParse.def

@@ -631,6 +631,8 @@ ERROR(expected_sil_function_type, none,
       "sil function expected to have SIL function type", ())
 ERROR(sil_dynamically_replaced_func_not_found,none,
       "dynamically replaced function not found %0", (Identifier))
+ERROR(sil_availability_expected_version,none,
+      "expected version number in 'available' attribute", ())
 
 // SIL Stage
 ERROR(expected_sil_stage_name, none,

include/swift/AST/ProtocolConformance.h

@@ -349,8 +349,7 @@ class RootProtocolConformance : public ProtocolConformance {
   bool isInvalid() const;
 
   /// Whether this conformance is weak-imported.
-  bool isWeakImported(ModuleDecl *fromModule,
-                      AvailabilityContext fromContext) const;
+  bool isWeakImported(ModuleDecl *fromModule) const;
 
   bool hasWitness(ValueDecl *requirement) const;
   Witness getWitness(ValueDecl *requirement, LazyResolver *resolver) const;

include/swift/IRGen/Linking.h

@@ -1102,8 +1102,7 @@ class LinkEntity {
   }
 
   /// Determine whether this entity will be weak-imported.
-  bool isWeakImported(ModuleDecl *module,
-                      AvailabilityContext fromContext) const;
+  bool isWeakImported(ModuleDecl *module) const;
 
   /// Return the source file whose codegen should trigger emission of this
   /// link entity, if one can be identified.
@@ -1173,7 +1172,6 @@ class LinkInfo {
 
   static LinkInfo get(const UniversalLinkageInfo &linkInfo,
                       ModuleDecl *swiftModule,
-                      AvailabilityContext availabilityContext,
                       const LinkEntity &entity,
                       ForDefinition_t forDefinition);
 

include/swift/SIL/SILFunction.h

@@ -18,6 +18,7 @@
 #define SWIFT_SIL_SILFUNCTION_H
 
 #include "swift/AST/ASTNode.h"
+#include "swift/AST/Availability.h"
 #include "swift/AST/ResilienceExpansion.h"
 #include "swift/Basic/ProfileCounter.h"
 #include "swift/SIL/SILBasicBlock.h"
@@ -160,6 +161,27 @@ class SILFunction
 
   Identifier ObjCReplacementFor;
 
+  /// The function's set of semantics attributes.
+  ///
+  /// TODO: Why is this using a std::string? Why don't we use uniqued
+  /// StringRefs?
+  std::vector<std::string> SemanticsAttrSet;
+
+  /// The function's remaining set of specialize attributes.
+  std::vector<SILSpecializeAttr*> SpecializeAttrSet;
+
+  /// Has value if there's a profile for this function
+  /// Contains Function Entry Count
+  ProfileCounter EntryCount;
+
+  /// The availability used to determine if declarations of this function
+  /// should use weak linking.
+  AvailabilityContext Availability;
+
+  /// This is the number of uses of this SILFunction inside the SIL.
+  /// It does not include references from debug scopes.
+  unsigned RefCount = 0;
+
   /// The function's bare attribute. Bare means that the function is SIL-only
   /// and does not require debug info.
   unsigned Bare : 1;
@@ -194,39 +216,16 @@ class SILFunction
   /// would indicate.
   unsigned HasCReferences : 1;
 
-  /// Whether cross-module references to this function should use weak linking.
-  unsigned IsWeakLinked : 1;
+  /// Whether cross-module references to this function should always use
+  /// weak linking.
+  unsigned IsWeakImported : 1;
 
   // Whether the implementation can be dynamically replaced.
   unsigned IsDynamicReplaceable : 1;
 
-  /// If != OptimizationMode::NotSet, the optimization mode specified with an
-  /// function attribute.
-  OptimizationMode OptMode;
-
-  /// This is the number of uses of this SILFunction inside the SIL.
-  /// It does not include references from debug scopes.
-  unsigned RefCount = 0;
-
-  /// The function's set of semantics attributes.
-  ///
-  /// TODO: Why is this using a std::string? Why don't we use uniqued
-  /// StringRefs?
-  llvm::SmallVector<std::string, 1> SemanticsAttrSet;
-
-  /// The function's remaining set of specialize attributes.
-  std::vector<SILSpecializeAttr*> SpecializeAttrSet;
-
-  /// The function's effects attribute.
-  EffectsKind EffectsKindAttr;
-
-  /// Has value if there's a profile for this function
-  /// Contains Function Entry Count
-  ProfileCounter EntryCount;
-
   /// True if this function is inlined at least once. This means that the
   /// debug info keeps a pointer to this function.
-  bool Inlined = false;
+  unsigned Inlined : 1;
 
   /// True if this function is a zombie function. This means that the function
   /// is dead and not referenced from anywhere inside the SIL. But it is kept
@@ -235,28 +234,35 @@ class SILFunction
   /// *) It is a dead method of a class which has higher visibility than the
   ///    method itself. In this case we need to create a vtable stub for it.
   /// *) It is a function referenced by the specialization information.
-  bool Zombie = false;
+  unsigned Zombie : 1;
 
   /// True if this function is in Ownership SSA form and thus must pass
   /// ownership verification.
   ///
   /// This enables the verifier to easily prove that before the Ownership Model
   /// Eliminator runs on a function, we only see a non-semantic-arc world and
   /// after the pass runs, we only see a semantic-arc world.
-  bool HasOwnership = true;
+  unsigned HasOwnership : 1;
 
   /// Set if the function body was deserialized from canonical SIL. This implies
   /// that the function's home module performed SIL diagnostics prior to
   /// serialization.
-  bool WasDeserializedCanonical = false;
+  unsigned WasDeserializedCanonical : 1;
 
   /// True if this is a reabstraction thunk of escaping function type whose
   /// single argument is a potentially non-escaping closure. This is an escape
   /// hatch to allow non-escaping functions to be stored or passed as an
   /// argument with escaping function type. The thunk argument's function type
   /// is not necessarily @noescape. The only relevant aspect of the argument is
   /// that it may have unboxed capture (i.e. @inout_aliasable parameters).
-  bool IsWithoutActuallyEscapingThunk = false;
+  unsigned IsWithoutActuallyEscapingThunk : 1;
+
+  /// If != OptimizationMode::NotSet, the optimization mode specified with an
+  /// function attribute.
+  unsigned OptMode : NumOptimizationModeBits;
+
+  /// The function's effects attribute.
+  unsigned EffectsKindAttr : NumEffectsKindBits;
 
   static void
   validateSubclassScope(SubclassScope scope, IsThunk_t isThunk,
@@ -575,17 +581,27 @@ class SILFunction
   bool hasCReferences() const { return HasCReferences; }
   void setHasCReferences(bool value) { HasCReferences = value; }
 
+  /// Returns the availability context used to determine if the function's
+  /// symbol should be weakly referenced across module boundaries.
+  AvailabilityContext getAvailabilityForLinkage() const {
+    return Availability;
+  }
+
+  void setAvailabilityForLinkage(AvailabilityContext availability) {
+    Availability = availability;
+  }
+
   /// Returns whether this function's symbol must always be weakly referenced
   /// across module boundaries.
-  bool isWeakLinked() const { return IsWeakLinked; }
-  /// Forces IRGen to treat references to this function as weak across module
-  /// boundaries (i.e. if it has external linkage).
-  void setWeakLinked(bool value = true) {
-    assert(!IsWeakLinked && "already set");
-    IsWeakLinked = value;
+  bool isAlwaysWeakImported() const { return IsWeakImported; }
+
+  void setAlwaysWeakImported(bool value) {
+    IsWeakImported = value;
   }
 
-  /// Returs whether this function implementation can be dynamically replaced.
+  bool isWeakImported() const;
+
+  /// Returns whether this function implementation can be dynamically replaced.
   IsDynamicallyReplaceable_t isDynamicallyReplaceable() const {
     return IsDynamicallyReplaceable_t(IsDynamicReplaceable);
   }
@@ -650,13 +666,17 @@ class SILFunction
 
   /// Get this function's optimization mode or OptimizationMode::NotSet if it is
   /// not set for this specific function.
-  OptimizationMode getOptimizationMode() const { return OptMode; }
+  OptimizationMode getOptimizationMode() const {
+    return OptimizationMode(OptMode);
+  }
 
   /// Returns the optimization mode for the function. If no mode is set for the
   /// function, returns the global mode, i.e. the mode of the module's options.
   OptimizationMode getEffectiveOptimizationMode() const;
 
-  void setOptimizationMode(OptimizationMode mode) { OptMode = mode; }
+  void setOptimizationMode(OptimizationMode mode) {
+    OptMode = unsigned(mode);
+  }
 
   /// \returns True if the function is optimizable (i.e. not marked as no-opt),
   ///          or is raw SIL (so that the mandatory passes still run).
@@ -726,16 +746,16 @@ class SILFunction
   void setInlineStrategy(Inline_t inStr) { InlineStrategy = inStr; }
 
   /// \return the function side effects information.
-  EffectsKind getEffectsKind() const { return EffectsKindAttr; }
+  EffectsKind getEffectsKind() const { return EffectsKind(EffectsKindAttr); }
 
   /// \return True if the function is annotated with the @_effects attribute.
   bool hasEffectsKind() const {
-    return EffectsKindAttr != EffectsKind::Unspecified;
+    return EffectsKind(EffectsKindAttr) != EffectsKind::Unspecified;
   }
 
   /// Set the function side effect information.
   void setEffectsKind(EffectsKind E) {
-    EffectsKindAttr = E;
+    EffectsKindAttr = unsigned(E);
   }
 
   /// Get this function's global_init attribute.

include/swift/Serialization/ModuleFormat.h

@@ -52,7 +52,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 = 500; // distinguish implicit raw values for enum cases
+const uint16_t SWIFTMODULE_VERSION_MINOR = 501; // SIL function availability
 
 using DeclIDField = BCFixed<31>;
 
@@ -473,6 +473,44 @@ enum class ImportControl : uint8_t {
 };
 using ImportControlField = BCFixed<2>;
 
+// Encodes a VersionTuple:
+//
+//  Major
+//  Minor
+//  Subminor
+//  HasMinor
+//  HasSubminor
+#define BC_AVAIL_TUPLE\
+    BCVBR<5>,\
+    BCVBR<5>,\
+    BCVBR<4>,\
+    BCFixed<1>,\
+    BCFixed<1>
+
+#define LIST_VER_TUPLE_PIECES(X)\
+  X##_Major, X##_Minor, X##_Subminor, X##_HasMinor, X##_HasSubminor
+#define DEF_VER_TUPLE_PIECES(X) unsigned LIST_VER_TUPLE_PIECES(X)
+#define DECODE_VER_TUPLE(X)\
+  if (X##_HasMinor) {\
+    if (X##_HasSubminor)\
+      X = llvm::VersionTuple(X##_Major, X##_Minor, X##_Subminor);\
+    else\
+      X = llvm::VersionTuple(X##_Major, X##_Minor);\
+    }\
+  else X = llvm::VersionTuple(X##_Major);
+#define ENCODE_VER_TUPLE(X, X_Expr)\
+    unsigned X##_Major = 0, X##_Minor = 0, X##_Subminor = 0,\
+             X##_HasMinor = 0, X##_HasSubminor = 0;\
+    const auto &X##_Val = X_Expr;\
+    if (X##_Val.hasValue()) {\
+      const auto &Y = X##_Val.getValue();\
+      X##_Major = Y.getMajor();\
+      X##_Minor = Y.getMinor().getValueOr(0);\
+      X##_Subminor = Y.getSubminor().getValueOr(0);\
+      X##_HasMinor = Y.getMinor().hasValue();\
+      X##_HasSubminor = Y.getSubminor().hasValue();\
+    }
+
 /// The various types of blocks that can occur within a serialized Swift
 /// module.
 ///
@@ -1596,20 +1634,6 @@ namespace decls_block {
     BCFixed<2>  // optimize value
   >;
 
-  // Encodes a VersionTuple:
-  //
-  //  Major
-  //  Minor
-  //  Subminor
-  //  HasMinor
-  //  HasSubminor
-#define BC_AVAIL_TUPLE\
-    BCVBR<5>,\
-    BCVBR<5>,\
-    BCVBR<4>,\
-    BCFixed<1>,\
-    BCFixed<1>
-
   using AvailableDeclAttrLayout = BCRecordLayout<
     Available_DECL_ATTR,
     BCFixed<1>, // implicit flag
@@ -1625,8 +1649,6 @@ namespace decls_block {
     BCBlob      // platform, followed by message
   >;
 
-#undef BC_AVAIL_TUPLE
-
   using ObjCDeclAttrLayout = BCRecordLayout<
     ObjC_DECL_ATTR,
     BCFixed<1>, // implicit flag