Merge pull request #28193 from harlanhaskins/truncation-station

[TBDGen] Match TAPI's truncation behavior for dylib versions

Author: Harlan Haskins

include/swift/AST/DiagnosticsFrontend.def

@@ -232,6 +232,15 @@ ERROR(error_formatting_invalid_range,none,
 WARNING(stats_disabled,none,
         "compiler was not built with support for collecting statistics", ())
 
+WARNING(tbd_warn_truncating_version,none,
+        "truncating %select{current|compatibility}0 version '%1' in TBD file "
+        "to fit in 32-bit space used by old mach-o format",
+        (unsigned, StringRef))
+
+ERROR(tbd_err_invalid_version,none,
+      "invalid dynamic library %select{current|compatibility}0 version '%1'",
+      (unsigned, StringRef))
+
 WARNING(tbd_only_supported_in_whole_module,none,
         "TBD generation is only supported when the whole module can be seen",
         ())

include/swift/TBDGen/TBDGen.h

@@ -40,12 +40,12 @@ struct TBDGenOptions {
   std::string ModuleLinkName;
 
   /// The current project version to use in the generated TBD file. Defaults
-  /// to None.
-  llvm::Optional<version::Version> CurrentVersion = None;
+  /// to empty string if not provided.
+  std::string CurrentVersion;
 
   /// The dylib compatibility-version to use in the generated TBD file. Defaults
-  /// to None.
-  llvm::Optional<version::Version> CompatibilityVersion = None;
+  /// to empty string if not provided.
+  std::string CompatibilityVersion;
 };
 
 void enumeratePublicSymbols(FileUnit *module, llvm::StringSet<> &symbols,

lib/Frontend/CompilerInvocation.cpp

@@ -961,16 +961,11 @@ static bool ParseTBDGenArgs(TBDGenOptions &Opts, ArgList &Args,
   Opts.IsInstallAPI = Args.hasArg(OPT_tbd_is_installapi);
 
   if (const Arg *A = Args.getLastArg(OPT_tbd_compatibility_version)) {
-    if (auto vers = version::Version::parseVersionString(
-          A->getValue(), SourceLoc(), &Diags)) {
-      Opts.CompatibilityVersion = *vers;
-    }
+    Opts.CompatibilityVersion = A->getValue();
   }
+
   if (const Arg *A = Args.getLastArg(OPT_tbd_current_version)) {
-    if (auto vers = version::Version::parseVersionString(
-          A->getValue(), SourceLoc(), &Diags)) {
-      Opts.CurrentVersion = *vers;
-    }
+    Opts.CurrentVersion = A->getValue();
   }
   return false;
 }

lib/TBDGen/TBDGen.cpp

@@ -19,6 +19,7 @@
 #include "swift/AST/Availability.h"
 #include "swift/AST/ASTMangler.h"
 #include "swift/AST/ASTVisitor.h"
+#include "swift/AST/DiagnosticsFrontend.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/PropertyWrappers.h"
@@ -593,16 +594,44 @@ void TBDGenVisitor::addFirstFileSymbols() {
   }
 }
 
-/// Converts a version tuple into a packed version, ignoring components beyond
-/// major, minor, and subminor.
-static llvm::MachO::PackedVersion
-convertToPacked(const version::Version &version) {
-  // FIXME: Warn if version is greater than 3 components?
-  unsigned major = 0, minor = 0, subminor = 0;
-  if (version.size() > 0) major = version[0];
-  if (version.size() > 1) minor = version[1];
-  if (version.size() > 2) subminor = version[2];
-  return llvm::MachO::PackedVersion(major, minor, subminor);
+/// The kind of version being parsed, used for diagnostics.
+/// Note: Must match the order in DiagnosticsFrontend.def
+enum DylibVersionKind_t: unsigned {
+  CurrentVersion,
+  CompatibilityVersion
+};
+
+/// Converts a version string into a packed version, truncating each component
+/// if necessary to fit all 3 into a 32-bit packed structure.
+///
+/// For example, the version '1219.37.11' will be packed as
+///
+///  Major (1,219)       Minor (37) Patch (11)
+/// ┌───────────────────┬──────────┬──────────┐
+/// │ 00001100 11000011 │ 00100101 │ 00001011 │
+/// └───────────────────┴──────────┴──────────┘
+///
+/// If an individual component is greater than the highest number that can be
+/// represented in its alloted space, it will be truncated to the maximum value
+/// that fits in the alloted space, which matches the behavior of the linker.
+static Optional<llvm::MachO::PackedVersion>
+parsePackedVersion(DylibVersionKind_t kind, StringRef versionString,
+                   ASTContext &ctx) {
+  if (versionString.empty())
+    return None;
+
+  llvm::MachO::PackedVersion version;
+  auto result = version.parse64(versionString);
+  if (!result.first) {
+    ctx.Diags.diagnose(SourceLoc(), diag::tbd_err_invalid_version,
+                       (unsigned)kind, versionString);
+    return None;
+  }
+  if (result.second) {
+    ctx.Diags.diagnose(SourceLoc(), diag::tbd_warn_truncating_version,
+                       (unsigned)kind, versionString);
+  }
+  return version;
 }
 
 static bool isApplicationExtensionSafe(const LangOptions &LangOpts) {
@@ -627,16 +656,20 @@ static void enumeratePublicSymbolsAndWrite(ModuleDecl *M, FileUnit *singleFile,
   file.setApplicationExtensionSafe(
     isApplicationExtensionSafe(M->getASTContext().LangOpts));
   file.setInstallName(opts.InstallName);
-  if (auto currentVersion = opts.CurrentVersion) {
-    file.setCurrentVersion(convertToPacked(*currentVersion));
-  }
-  if (auto compatibilityVersion = opts.CompatibilityVersion) {
-    file.setCompatibilityVersion(convertToPacked(*compatibilityVersion));
-  }
   file.setTwoLevelNamespace();
   file.setSwiftABIVersion(irgen::getSwiftABIVersion());
   file.setInstallAPI(opts.IsInstallAPI);
 
+  if (auto packed = parsePackedVersion(CurrentVersion,
+                                       opts.CurrentVersion, ctx)) {
+    file.setCurrentVersion(*packed);
+  }
+
+  if (auto packed = parsePackedVersion(CompatibilityVersion,
+                                       opts.CompatibilityVersion, ctx)) {
+    file.setCompatibilityVersion(*packed);
+  }
+
   llvm::MachO::Target target(triple);
   file.addTarget(target);
 

test/TBD/dylib-version-truncation.swift

@@ -0,0 +1,27 @@
+// REQUIRES: VENDOR=apple
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-current-version 2.0 | %FileCheck %s --check-prefix TWOPOINTZERO
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-current-version 2 | %FileCheck %s --check-prefix TWOPOINTZERO
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-current-version 20.10 | %FileCheck %s --check-prefix TWENTYPOINTTEN
+
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-compatibility-version 2.0 | %FileCheck %s --check-prefix TWOPOINTZEROCOMPAT
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-compatibility-version 2 | %FileCheck %s --check-prefix TWOPOINTZEROCOMPAT
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-compatibility-version 20.10 | %FileCheck %s --check-prefix TWENTYPOINTTENCOMPAT
+
+// Make sure we correctly truncate a value over 255
+
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-current-version 20.300 2>&1 | %FileCheck %s --check-prefix TWENTYPOINTTHREEHUNDRED
+// RUN: %target-swift-frontend -typecheck %s -emit-tbd-path - -tbd-compatibility-version 20.300 2>&1 | %FileCheck %s --check-prefix TWENTYPOINTTHREEHUNDREDCOMPAT
+
+// TWOPOINTZERO: current-version: 2
+// TWENTYPOINTTEN: current-version: 20.10
+
+// TWOPOINTZEROCOMPAT: compatibility-version: 2
+// TWENTYPOINTTENCOMPAT: compatibility-version: 20.10
+
+// TWENTYPOINTTHREEHUNDRED: warning: truncating current version '20.300' in TBD file to fit in 32-bit space used by old mach-o format
+// TWENTYPOINTTHREEHUNDRED: current-version: 20.255
+
+// TWENTYPOINTTHREEHUNDREDCOMPAT: warning: truncating compatibility version '20.300' in TBD file to fit in 32-bit space used by old mach-o format
+// TWENTYPOINTTHREEHUNDREDCOMPAT: compatibility-version: 20.255

test/TBD/dylib-version.swift

@@ -14,4 +14,4 @@
 // CURRENT: current-version: 2
 // COMPAT: compatibility-version: 2
 
-// BOGUS: version component contains non-numeric characters
+// BOGUS: invalid dynamic library compatibility version 'not_a_version_string'