Merge pull request swiftlang#65954 from tshortli/swiftinterface-inlinable-function-availability-miscompile

tshortli · web-flow · commit 17d121f7d1f1 · 2023-05-30T18:10:59.000-07:00
Frontend: Don't append `-target-min-inlining-target target` to implicit module builds
diff --git a/include/swift/Frontend/ModuleInterfaceLoader.h b/include/swift/Frontend/ModuleInterfaceLoader.h
@@ -532,6 +532,18 @@ class ModuleInterfaceLoader : public SerializedModuleLoaderBase {
       DependencyTracker *tracker = nullptr);
 };
 
+struct SwiftInterfaceInfo {
+  /// The compiler arguments that were encoded in the swiftinterface.
+  SmallVector<const char *, 64> Arguments;
+
+  /// The string following `swift-compiler-version:` in the swiftinterface.
+  std::string CompilerVersion;
+
+  /// The tools version of the compiler (e.g. 5.8) that emitted the
+  /// swiftinterface. This is extracted from the `CompilerVersion` string.
+  llvm::Optional<version::Version> CompilerToolsVersion;
+};
+
 struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
 private:
   SourceManager &SM;
@@ -558,8 +570,7 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
                                      bool suppressRemarks,
                                      RequireOSSAModules_t requireOSSAModules);
   bool extractSwiftInterfaceVersionAndArgs(CompilerInvocation &subInvocation,
-                                           SmallVectorImpl<const char *> &SubArgs,
-                                           std::string &CompilerVersion,
+                                           SwiftInterfaceInfo &interfaceInfo,
                                            StringRef interfacePath,
                                            SourceLoc diagnosticLoc);
 public:
diff --git a/include/swift/Frontend/ModuleInterfaceSupport.h b/include/swift/Frontend/ModuleInterfaceSupport.h
@@ -75,9 +75,27 @@ struct ModuleInterfaceOptions {
 extern version::Version InterfaceFormatVersion;
 std::string getSwiftInterfaceCompilerVersionForCurrentCompiler(ASTContext &ctx);
 
+/// A regex that matches lines like this:
+///
+///     // swift-interface-format-version: 1.0
+///
+/// and extracts "1.0".
 llvm::Regex getSwiftInterfaceFormatVersionRegex();
+
+/// A regex that matches lines like this:
+///
+///     // swift-compiler-version: Apple Swift version 5.8 (swiftlang-5.8.0.117.59)
+///
+/// and extracts "Apple Swift version 5.8 (swiftlang-5.8.0.117.59)".
 llvm::Regex getSwiftInterfaceCompilerVersionRegex();
 
+/// A regex that matches strings like this:
+///
+///     Apple Swift version 5.8
+///
+/// and extracts "5.8".
+llvm::Regex getSwiftInterfaceCompilerToolsVersionRegex();
+
 /// Emit a stable module interface for \p M, which can be used by a client
 /// source file to import this module, subject to options given by \p Opts.
 ///
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -1330,10 +1330,12 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(
                                         SearchPathOpts.CandidateCompiledModules);
 }
 
-static bool readSwiftInterfaceVersionAndArgs(
-    SourceManager &SM, DiagnosticEngine &Diags, llvm::StringSaver &ArgSaver,
-    SmallVectorImpl<const char *> &SubArgs, std::string &CompilerVersion,
-    StringRef interfacePath, SourceLoc diagnosticLoc) {
+static bool readSwiftInterfaceVersionAndArgs(SourceManager &SM,
+                                             DiagnosticEngine &Diags,
+                                             llvm::StringSaver &ArgSaver,
+                                             SwiftInterfaceInfo &interfaceInfo,
+                                             StringRef interfacePath,
+                                             SourceLoc diagnosticLoc) {
   llvm::vfs::FileSystem &fs = *SM.getFileSystem();
   auto FileOrError = swift::vfs::getFileOrSTDIN(fs, interfacePath);
   if (!FileOrError) {
@@ -1346,7 +1348,7 @@ static bool readSwiftInterfaceVersionAndArgs(
   auto SB = FileOrError.get()->getBuffer();
   auto VersRe = getSwiftInterfaceFormatVersionRegex();
   auto CompRe = getSwiftInterfaceCompilerVersionRegex();
-  SmallVector<StringRef, 1> VersMatches, CompMatches;
+  SmallVector<StringRef, 2> VersMatches, CompMatches;
 
   if (!VersRe.match(SB, &VersMatches)) {
     InterfaceSubContextDelegateImpl::diagnose(
@@ -1355,7 +1357,8 @@ static bool readSwiftInterfaceVersionAndArgs(
     return true;
   }
 
-  if (extractCompilerFlagsFromInterface(interfacePath, SB, ArgSaver, SubArgs)) {
+  if (extractCompilerFlagsFromInterface(interfacePath, SB, ArgSaver,
+                                        interfaceInfo.Arguments)) {
     InterfaceSubContextDelegateImpl::diagnose(
         interfacePath, diagnosticLoc, SM, &Diags,
         diag::error_extracting_version_from_module_interface);
@@ -1375,10 +1378,20 @@ static bool readSwiftInterfaceVersionAndArgs(
 
   if (CompRe.match(SB, &CompMatches)) {
     assert(CompMatches.size() == 2);
-    CompilerVersion = ArgSaver.save(CompMatches[1]).str();
+    interfaceInfo.CompilerVersion = ArgSaver.save(CompMatches[1]).str();
+
+    // For now, successfully parsing the tools version out of the interface is
+    // optional.
+    auto ToolsVersRe = getSwiftInterfaceCompilerToolsVersionRegex();
+    SmallVector<StringRef, 2> VendorToolsVersMatches;
+    if (ToolsVersRe.match(interfaceInfo.CompilerVersion,
+                          &VendorToolsVersMatches)) {
+      interfaceInfo.CompilerToolsVersion = VersionParser::parseVersionString(
+          VendorToolsVersMatches[1], SourceLoc(), nullptr);
+    }
   } else {
     // Don't diagnose; handwritten module interfaces don't include this field.
-    CompilerVersion = "(unspecified, file possibly handwritten)";
+    interfaceInfo.CompilerVersion = "(unspecified, file possibly handwritten)";
   }
 
   // For now: we support anything with the same "major version" and assume
@@ -1425,23 +1438,18 @@ bool ModuleInterfaceLoader::buildExplicitSwiftModuleFromSwiftInterface(
   // Read out the compiler version.
   llvm::BumpPtrAllocator alloc;
   llvm::StringSaver ArgSaver(alloc);
-  std::string CompilerVersion;
-  SmallVector<const char *, 64> InterfaceArgs;
-  readSwiftInterfaceVersionAndArgs(Instance.getSourceMgr(),
-                                   Instance.getDiags(),
-                                   ArgSaver,
-                                   InterfaceArgs,
-                                   CompilerVersion,
-                                   interfacePath,
+  SwiftInterfaceInfo InterfaceInfo;
+  readSwiftInterfaceVersionAndArgs(Instance.getSourceMgr(), Instance.getDiags(),
+                                   ArgSaver, InterfaceInfo, interfacePath,
                                    SourceLoc());
-  
+
   auto Builder = ExplicitModuleInterfaceBuilder(
       Instance, &Instance.getDiags(), Instance.getSourceMgr(),
       moduleCachePath, backupInterfaceDir, prebuiltCachePath,
       ABIDescriptorPath, {});
   auto error = Builder.buildSwiftModuleFromInterface(
       interfacePath, outputPath, ShouldSerializeDeps, /*ModuleBuffer*/nullptr,
-      CompiledCandidates, CompilerVersion);
+      CompiledCandidates, InterfaceInfo.CompilerVersion);
   if (!error)
     return false;
   else
@@ -1567,18 +1575,14 @@ void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface(
 }
 
 bool InterfaceSubContextDelegateImpl::extractSwiftInterfaceVersionAndArgs(
-    CompilerInvocation &subInvocation,
-    SmallVectorImpl<const char *> &SubArgs,
-    std::string &CompilerVersion,
-    StringRef interfacePath,
-    SourceLoc diagnosticLoc) {
-  if (readSwiftInterfaceVersionAndArgs(SM, *Diags, ArgSaver, SubArgs,
-                                       CompilerVersion, interfacePath,
-                                       diagnosticLoc))
+    CompilerInvocation &subInvocation, SwiftInterfaceInfo &interfaceInfo,
+    StringRef interfacePath, SourceLoc diagnosticLoc) {
+  if (readSwiftInterfaceVersionAndArgs(SM, *Diags, ArgSaver, interfaceInfo,
+                                       interfacePath, diagnosticLoc))
     return true;
 
   SmallString<32> ExpectedModuleName = subInvocation.getModuleName();
-  if (subInvocation.parseArgs(SubArgs, *Diags)) {
+  if (subInvocation.parseArgs(interfaceInfo.Arguments, *Diags)) {
     return true;
   }
 
@@ -1845,24 +1849,28 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
     .setMainAndSupplementaryOutputs(outputFiles, ModuleOutputPaths);
 
   SmallVector<const char *, 64> SubArgs;
-
-  // If the interface was emitted by a compiler that didn't print
-  // `-target-min-inlining-version` into it, default to using the version from
-  // the target triple, emulating previous behavior.
-  SubArgs.push_back("-target-min-inlining-version");
-  SubArgs.push_back("target");
-
-  std::string CompilerVersion;
+  SwiftInterfaceInfo interfaceInfo;
   // Extract compiler arguments from the interface file and use them to configure
   // the compiler invocation.
-  if (extractSwiftInterfaceVersionAndArgs(subInvocation,
-                                          SubArgs,
-                                          CompilerVersion,
-                                          interfacePath,
-                                          diagLoc)) {
+  if (extractSwiftInterfaceVersionAndArgs(subInvocation, interfaceInfo,
+                                          interfacePath, diagLoc)) {
     return std::make_error_code(std::errc::not_supported);
   }
 
+  // Prior to Swift 5.9, swiftinterfaces were always built (accidentally) with
+  // `-target-min-inlining-version target` prepended to the argument list. To
+  // preserve compatibility we must continue to prepend those flags to the
+  // invocation when the interface was generated by an older compiler.
+  if (auto toolsVersion = interfaceInfo.CompilerToolsVersion) {
+    if (toolsVersion < version::Version{5, 9}) {
+      SubArgs.push_back("-target-min-inlining-version");
+      SubArgs.push_back("target");
+    }
+  }
+
+  SubArgs.insert(SubArgs.end(), interfaceInfo.Arguments.begin(),
+                 interfaceInfo.Arguments.end());
+
   // Insert arguments collected from the interface file.
   BuildArgs.insert(BuildArgs.end(), SubArgs.begin(), SubArgs.end());
   if (subInvocation.parseArgs(SubArgs, *Diags)) {
@@ -1891,7 +1899,7 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
   CompilerInstance subInstance;
   SubCompilerInstanceInfo info;
   info.Instance = &subInstance;
-  info.CompilerVersion = CompilerVersion;
+  info.CompilerVersion = interfaceInfo.CompilerVersion;
 
   subInstance.getSourceMgr().setFileSystem(SM.getFileSystem());
 
diff --git a/lib/Frontend/ModuleInterfaceSupport.cpp b/lib/Frontend/ModuleInterfaceSupport.cpp
@@ -122,6 +122,10 @@ llvm::Regex swift::getSwiftInterfaceCompilerVersionRegex() {
                      ": (.+)$", llvm::Regex::Newline);
 }
 
+llvm::Regex swift::getSwiftInterfaceCompilerToolsVersionRegex() {
+  return llvm::Regex("Swift version ([0-9\\.]+)", llvm::Regex::Newline);
+}
+
 // MARK(https://github.com/apple/swift/issues/43510): Module name shadowing warnings
 //
 // When swiftc emits a module interface, it qualifies most types with their
diff --git a/test/Interpreter/implicit-module-build-overload-availability-inlinable-function.swift b/test/Interpreter/implicit-module-build-overload-availability-inlinable-function.swift
@@ -0,0 +1,56 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/NonAPI)
+// RUN: %empty-directory(%t/API)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-emit-module-interface(%t/NonAPI/Library.swiftinterface) %t/Library.swift -module-name Library -target %target-swift-abi-5.8-triple
+// RUN: %target-swift-emit-module-interface(%t/API/Library.swiftinterface) %t/Library.swift -module-name Library -target %target-swift-abi-5.8-triple -library-level api
+
+// Build Client.swift against the Library.swiftinterface without
+// `-library-level api`. Since the deployment target of the library is
+// SwiftStdlib 5.8, the newer overload that returns a String should be selected
+// by overload resolution during the implicit module build.
+
+// RUN: %target-build-swift %t/Client.swift -o %t/NonAPI/client -I %t/NonAPI/
+// RUN: %target-codesign %t/NonAPI/client
+// RUN: %target-run %t/NonAPI/client | %FileCheck %s --check-prefix=CHECK-NON-API
+
+// Build Client.swift against the Library.swiftinterface with
+// `-library-level api`. Since the deployment target of the client that will
+// get a copy of `fragileFuncUsingOverload()` is earlier than SwiftStdlib 5.8,
+// the older overload returning an Int should be selected during the implicit
+// module build even though the library targets SwiftStdlib 5.8.
+
+// RUN: %target-build-swift %t/Client.swift -o %t/API/client -I %t/API/
+// RUN: %target-codesign %t/API/client
+// RUN: %target-run %t/API/client | %FileCheck %s --check-prefix=CHECK-API
+
+// REQUIRES: executable_test
+// REQUIRES: OS=macosx || OS=ios || OS=tvos || OS=watchos
+
+//--- Library.swift
+
+@_disfavoredOverload
+@_alwaysEmitIntoClient
+public func overloadedFunc() -> Int {
+  return 1234
+}
+
+@available(SwiftStdlib 5.8, *)
+@_alwaysEmitIntoClient
+public func overloadedFunc() -> String {
+  return "String"
+}
+
+@_alwaysEmitIntoClient
+public func fragileFuncUsingOverload() -> any CustomStringConvertible {
+  return overloadedFunc()
+}
+
+//--- Client.swift
+
+import Library
+
+// CHECK-NON-API: String
+// CHECK-API: 1234
+print(fragileFuncUsingOverload())
diff --git a/test/ModuleInterface/availability-library-level-api-5_8.swiftinterface b/test/ModuleInterface/availability-library-level-api-5_8.swiftinterface
@@ -0,0 +1,18 @@
+// swift-interface-format-version: 1.0
+// swift-compiler-version: Apple Swift version 5.8 (swiftlang-5.8.0.117.59 clang-1403.0.22.8.50)
+// swift-module-flags: -target arm64-apple-macosx11 -enable-library-evolution -swift-version 5 -library-level api -module-name Test
+
+// RUN: %target-swift-frontend -typecheck-module-from-interface -verify -module-name Test %s
+
+// REQUIRES: OS=macosx
+
+import Swift
+
+@available(macOS 11, *)
+public struct S {}
+
+// This typealias ought to be @available(macOS 11, *) since it references `S`
+// and the module was compiled with `-library-level api`. However, given that
+// the interface was produced with tools that are older than Swift 5.9 we
+// typecheck availability with the deployment target as the floor.
+public typealias A = S
diff --git a/test/ModuleInterface/availability-library-level-api-5_9.swiftinterface b/test/ModuleInterface/availability-library-level-api-5_9.swiftinterface
@@ -0,0 +1,17 @@
+// swift-interface-format-version: 1.0
+// swift-compiler-version: Apple Swift version 5.9
+// swift-module-flags: -target arm64-apple-macosx11 -enable-library-evolution -swift-version 5 -library-level api -module-name Test
+
+// RUN: not %target-swift-frontend -typecheck-module-from-interface -module-name Test %s 2>&1 | %FileCheck %s
+
+// REQUIRES: OS=macosx
+
+import Swift
+
+@available(macOS 11, *)
+public struct S {}
+
+public typealias A = S
+
+// CHECK: error: 'S' is only available in macOS 11 or newer; clients of 'Test' may have a lower deployment target
+// CHECK: error: failed to verify module interface of 'Test' due to the errors above;
