From 441c85989cbd9e138c249493cde64fe4d8f3f7c0 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Sun, 13 Feb 2022 09:30:27 -0800 Subject: [PATCH 1/5] Add -dead_strip / --gc-sections for release builds In the past it was unsafe to pass -dead_strip because there was some swift metadata that could be stripped. It seems that at this point all of those cases are either fixed or marked as @llvm.used, so passing these flags should safely reduce binary size in some cases. If there are other cases where this isn't safe we should likely annotate them as @llvm.used anyways. Related: https://bugs.swift.org/browse/SR-521 https://github.com/apple/swift-package-manager/pull/215 --- Sources/Build/BuildPlan.swift | 18 ++++++++++++++++++ Tests/BuildTests/BuildPlanTests.swift | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index f5d74294f8f..51a78e38ece 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -1200,6 +1200,21 @@ public final class ProductBuildDescription { return args.filter({ !invalidArguments.contains($0) }) } + private var deadStripArguments: [String] { + switch buildParameters.configuration { + case .debug: + return [] + case .release: + if buildParameters.triple.isDarwin() { + return ["-Xlinker", "-dead_strip"] + } else if buildParameters.triple.isWindows() { + return ["-Xlinker", "/OPT:REF"] + } else { + return ["-Xlinker", "--gc-sections"] + } + } + } + /// The arguments to link and create this product. public func linkArguments() throws -> [String] { var args = [buildParameters.toolchain.swiftCompiler.pathString] @@ -1246,12 +1261,14 @@ public final class ProductBuildDescription { case .manifest: args += ["-emit-executable"] } + args += deadStripArguments case .library(.dynamic): args += ["-emit-library"] if buildParameters.triple.isDarwin() { let relativePath = "@rpath/\(buildParameters.binaryRelativePath(for: product).pathString)" args += ["-Xlinker", "-install_name", "-Xlinker", relativePath] } + args += deadStripArguments case .executable, .snippet: // Link the Swift stdlib statically, if requested. if buildParameters.shouldLinkStaticSwiftStdlib { @@ -1262,6 +1279,7 @@ public final class ProductBuildDescription { } } args += ["-emit-executable"] + args += deadStripArguments // If we're linking an executable whose main module is implemented in Swift, // we rename the `__main` entry point symbol to `_main` again. diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 1155f28218e..19e71c155f0 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -466,7 +466,7 @@ final class BuildPlanTests: XCTestCase { XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [ "/fake/path/to/swiftc", "-g", "-L", "/path/to/build/release", "-o", "/path/to/build/release/exe", "-module-name", "exe", "-emit-executable", - "-Xlinker", "-rpath", "-Xlinker", "@loader_path", + "-Xlinker", "-dead_strip", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@/path/to/build/release/exe.product/Objects.LinkFileList", "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx", "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift-5.5/macosx", @@ -1029,7 +1029,7 @@ final class BuildPlanTests: XCTestCase { XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [ "/fake/path/to/swiftc", "-g", "-L", "/path/to/build/release", "-o", "/path/to/build/release/exe", "-module-name", "exe", "-emit-executable", - "-Xlinker", "-rpath", "-Xlinker", "@loader_path", + "-Xlinker", "-dead_strip", "-Xlinker", "-rpath", "-Xlinker", "@loader_path", "@/path/to/build/release/exe.product/Objects.LinkFileList", "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx", "-target", hostTriple.tripleString(forPlatformVersion: "12.0"), From 92ee0c3feb5b3fcc2c5d76a85a0eb9b10ecebda1 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 14 Feb 2022 10:17:09 -0800 Subject: [PATCH 2/5] Add --disable-dead-strip flag --- Sources/Build/BuildPlan.swift | 4 ++ Sources/Commands/Options.swift | 3 ++ Sources/Commands/SwiftTool.swift | 1 + Sources/SPMBuildCore/BuildParameters.swift | 5 ++ Tests/BuildTests/BuildPlanTests.swift | 60 +++++++++++++++++++++- 5 files changed, 71 insertions(+), 2 deletions(-) diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index 51a78e38ece..9633bfcb823 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -1201,6 +1201,10 @@ public final class ProductBuildDescription { } private var deadStripArguments: [String] { + if buildParameters.disableDeadStrip { + return [] + } + switch buildParameters.configuration { case .debug: return [] diff --git a/Sources/Commands/Options.swift b/Sources/Commands/Options.swift index 75635af7153..5582e9b3a27 100644 --- a/Sources/Commands/Options.swift +++ b/Sources/Commands/Options.swift @@ -375,6 +375,9 @@ public struct SwiftToolOptions: ParsableArguments { @Option(name: .customLong("resolver-fingerprint-checking")) var resolverFingerprintCheckingMode: FingerprintCheckingMode = .warn + @Flag(name: .customLong("disable-dead-strip"), help: "Disable dead code stripping by the linker") + var disableDeadStrip: Bool = false + @Flag(name: .customLong("netrc"), help: .hidden) var _deprecated_netrc: Bool = false diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift index cf9c4505699..a4351df4894 100644 --- a/Sources/Commands/SwiftTool.swift +++ b/Sources/Commands/SwiftTool.swift @@ -842,6 +842,7 @@ public class SwiftTool { isXcodeBuildSystemEnabled: options.buildSystem == .xcode, printManifestGraphviz: options.printManifestGraphviz, forceTestDiscovery: options.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery + disableDeadStrip: options.disableDeadStrip, isTTY: isTTY ) }) diff --git a/Sources/SPMBuildCore/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters.swift index bcf0ab9085a..88667f32925 100644 --- a/Sources/SPMBuildCore/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters.swift @@ -172,6 +172,9 @@ public struct BuildParameters: Encodable { // What strategy to use to discover tests public var testDiscoveryStrategy: TestDiscoveryStrategy + /// Whether to disable dead code stripping by the linker + public var disableDeadStrip: Bool + public var isTTY: Bool public init( @@ -200,6 +203,7 @@ public struct BuildParameters: Encodable { printManifestGraphviz: Bool = false, enableTestability: Bool? = nil, forceTestDiscovery: Bool = false, + disableDeadStrip: Bool = false, isTTY: Bool = false ) { let triple = destinationTriple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompiler) @@ -237,6 +241,7 @@ public struct BuildParameters: Encodable { self.enableTestability = enableTestability ?? (.debug == configuration) // decide if to enable the use of test manifests based on platform. this is likely to change in the future self.testDiscoveryStrategy = triple.isDarwin() ? .objectiveC : .manifest(generate: forceTestDiscovery) + self.disableDeadStrip = disableDeadStrip self.isTTY = isTTY } diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 19e71c155f0..ed90fe233c7 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -69,7 +69,8 @@ final class BuildPlanTests: XCTestCase { canRenameEntrypointFunctionName: Bool = false, destinationTriple: TSCUtility.Triple = hostTriple, indexStoreMode: BuildParameters.IndexStoreMode = .off, - useExplicitModuleBuild: Bool = false + useExplicitModuleBuild: Bool = false, + disableDeadStrip: Bool = false ) -> BuildParameters { return BuildParameters( dataPath: buildPath, @@ -82,7 +83,8 @@ final class BuildPlanTests: XCTestCase { shouldLinkStaticSwiftStdlib: shouldLinkStaticSwiftStdlib, canRenameEntrypointFunctionName: canRenameEntrypointFunctionName, indexStoreMode: indexStoreMode, - useExplicitModuleBuild: useExplicitModuleBuild + useExplicitModuleBuild: useExplicitModuleBuild, + disableDeadStrip: disableDeadStrip ) } @@ -472,6 +474,60 @@ final class BuildPlanTests: XCTestCase { "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift-5.5/macosx", "-target", defaultTargetTriple, ]) + #else + XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [ + "/fake/path/to/swiftc", "-g", "-L", "/path/to/build/release", + "-o", "/path/to/build/release/exe", "-module-name", "exe", "-emit-executable", + "-Xlinker", "--gc-sections", "-Xlinker", "-rpath=$ORIGIN", + "@/path/to/build/release/exe.product/Objects.LinkFileList", + "-target", defaultTargetTriple, + ]) + #endif + } + + func testBasicReleasePackageNoDeadStrip() throws { + let fs = InMemoryFileSystem(emptyFiles: + "/Pkg/Sources/exe/main.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadPackageGraph( + fs: fs, + manifests: [ + Manifest.createRootManifest( + name: "Pkg", + path: .init("/Pkg"), + targets: [ + TargetDescription(name: "exe", dependencies: []), + ]), + ], + observabilityScope: observability.topScope + ) + XCTAssertNoDiagnostics(observability.diagnostics) + + let result = try BuildPlanResult(plan: BuildPlan( + buildParameters: mockBuildParameters(config: .release, disableDeadStrip: true), + graph: graph, + fileSystem: fs, + observabilityScope: observability.topScope + )) + + result.checkProductsCount(1) + result.checkTargetsCount(1) + + let exe = try result.target(for: "exe").swiftTarget().compileArguments() + XCTAssertMatch(exe, ["-swift-version", "4", "-O", "-g", .equal(j), "-DSWIFT_PACKAGE", "-module-cache-path", "/path/to/build/release/ModuleCache", .anySequence]) + + #if os(macOS) + XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [ + "/fake/path/to/swiftc", "-g", "-L", "/path/to/build/release", + "-o", "/path/to/build/release/exe", "-module-name", "exe", "-emit-executable", + "-Xlinker", "-rpath", "-Xlinker", "@loader_path", + "@/path/to/build/release/exe.product/Objects.LinkFileList", + "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx", + "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift-5.5/macosx", + "-target", defaultTargetTriple, + ]) #else XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [ "/fake/path/to/swiftc", "-g", "-L", "/path/to/build/release", From 2afc913e016eaca1533db4e9dc3c5469b6407956 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 14 Feb 2022 11:01:10 -0800 Subject: [PATCH 3/5] Rename flag / variable --- Sources/Build/BuildPlan.swift | 2 +- Sources/Commands/Options.swift | 7 +++++-- Sources/Commands/SwiftTool.swift | 2 +- Sources/SPMBuildCore/BuildParameters.swift | 6 +++--- Tests/BuildTests/BuildPlanTests.swift | 6 +++--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index 9633bfcb823..ed834d60403 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -1201,7 +1201,7 @@ public final class ProductBuildDescription { } private var deadStripArguments: [String] { - if buildParameters.disableDeadStrip { + if buildParameters.linkerDeadStrip { return [] } diff --git a/Sources/Commands/Options.swift b/Sources/Commands/Options.swift index 5582e9b3a27..f822dc1e908 100644 --- a/Sources/Commands/Options.swift +++ b/Sources/Commands/Options.swift @@ -375,8 +375,11 @@ public struct SwiftToolOptions: ParsableArguments { @Option(name: .customLong("resolver-fingerprint-checking")) var resolverFingerprintCheckingMode: FingerprintCheckingMode = .warn - @Flag(name: .customLong("disable-dead-strip"), help: "Disable dead code stripping by the linker") - var disableDeadStrip: Bool = false + @Flag( + name: .customLong("dead-strip"), + inversion: .prefixedEnableDisable, + help: "Disable/enable dead code stripping by the linker") + var linkerDeadStrip: Bool = true @Flag(name: .customLong("netrc"), help: .hidden) var _deprecated_netrc: Bool = false diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift index a4351df4894..8c3483406f6 100644 --- a/Sources/Commands/SwiftTool.swift +++ b/Sources/Commands/SwiftTool.swift @@ -842,7 +842,7 @@ public class SwiftTool { isXcodeBuildSystemEnabled: options.buildSystem == .xcode, printManifestGraphviz: options.printManifestGraphviz, forceTestDiscovery: options.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery - disableDeadStrip: options.disableDeadStrip, + linkerDeadStrip: options.linkerDeadStrip, isTTY: isTTY ) }) diff --git a/Sources/SPMBuildCore/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters.swift index 88667f32925..15776aad522 100644 --- a/Sources/SPMBuildCore/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters.swift @@ -173,7 +173,7 @@ public struct BuildParameters: Encodable { public var testDiscoveryStrategy: TestDiscoveryStrategy /// Whether to disable dead code stripping by the linker - public var disableDeadStrip: Bool + public var linkerDeadStrip: Bool public var isTTY: Bool @@ -203,7 +203,7 @@ public struct BuildParameters: Encodable { printManifestGraphviz: Bool = false, enableTestability: Bool? = nil, forceTestDiscovery: Bool = false, - disableDeadStrip: Bool = false, + linkerDeadStrip: Bool = false, isTTY: Bool = false ) { let triple = destinationTriple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompiler) @@ -241,7 +241,7 @@ public struct BuildParameters: Encodable { self.enableTestability = enableTestability ?? (.debug == configuration) // decide if to enable the use of test manifests based on platform. this is likely to change in the future self.testDiscoveryStrategy = triple.isDarwin() ? .objectiveC : .manifest(generate: forceTestDiscovery) - self.disableDeadStrip = disableDeadStrip + self.linkerDeadStrip = linkerDeadStrip self.isTTY = isTTY } diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index ed90fe233c7..d0fad1cdfe9 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -70,7 +70,7 @@ final class BuildPlanTests: XCTestCase { destinationTriple: TSCUtility.Triple = hostTriple, indexStoreMode: BuildParameters.IndexStoreMode = .off, useExplicitModuleBuild: Bool = false, - disableDeadStrip: Bool = false + linkerDeadStrip: Bool = false ) -> BuildParameters { return BuildParameters( dataPath: buildPath, @@ -84,7 +84,7 @@ final class BuildPlanTests: XCTestCase { canRenameEntrypointFunctionName: canRenameEntrypointFunctionName, indexStoreMode: indexStoreMode, useExplicitModuleBuild: useExplicitModuleBuild, - disableDeadStrip: disableDeadStrip + linkerDeadStrip: linkerDeadStrip ) } @@ -506,7 +506,7 @@ final class BuildPlanTests: XCTestCase { XCTAssertNoDiagnostics(observability.diagnostics) let result = try BuildPlanResult(plan: BuildPlan( - buildParameters: mockBuildParameters(config: .release, disableDeadStrip: true), + buildParameters: mockBuildParameters(config: .release, linkerDeadStrip: true), graph: graph, fileSystem: fs, observabilityScope: observability.topScope From 9cc0b7583423ce3dd52753275ab727d9c004a055 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 15 Feb 2022 10:32:48 -0800 Subject: [PATCH 4/5] Add CHANGELOG entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da8ed529178..09d644eb35b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ Swift 5.7 Add a `--disable-testable-imports` flag to `swift test` with which tests are built without the testability feature (`import @tstable` disabled). +* [#4135] + + Enable linker dead stripping for all platforms. This can be disabled with `--disable-dead-strip` + Swift 5.6 ----------- * [SE-0332] @@ -217,3 +221,4 @@ Swift 3.0 [#3901]: https://github.com/apple/swift-package-manager/pull/3901 [#3942]: https://github.com/apple/swift-package-manager/pull/3942 [#4119]: https://github.com/apple/swift-package-manager/pull/4119 +[#4135]: https://github.com/apple/swift-package-manager/pull/4135 From c25c28b8a85abc3ac050ac45b946da0b4ca4dd42 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 15 Feb 2022 15:01:23 -0800 Subject: [PATCH 5/5] Fix inverted condition --- Sources/Build/BuildPlan.swift | 2 +- Sources/SPMBuildCore/BuildParameters.swift | 2 +- Tests/BuildTests/BuildPlanTests.swift | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index ed834d60403..33ac07e6bca 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -1201,7 +1201,7 @@ public final class ProductBuildDescription { } private var deadStripArguments: [String] { - if buildParameters.linkerDeadStrip { + if !buildParameters.linkerDeadStrip { return [] } diff --git a/Sources/SPMBuildCore/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters.swift index 15776aad522..ab65c689ffa 100644 --- a/Sources/SPMBuildCore/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters.swift @@ -203,7 +203,7 @@ public struct BuildParameters: Encodable { printManifestGraphviz: Bool = false, enableTestability: Bool? = nil, forceTestDiscovery: Bool = false, - linkerDeadStrip: Bool = false, + linkerDeadStrip: Bool = true, isTTY: Bool = false ) { let triple = destinationTriple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompiler) diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index d0fad1cdfe9..1f6b92e12e5 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -70,7 +70,7 @@ final class BuildPlanTests: XCTestCase { destinationTriple: TSCUtility.Triple = hostTriple, indexStoreMode: BuildParameters.IndexStoreMode = .off, useExplicitModuleBuild: Bool = false, - linkerDeadStrip: Bool = false + linkerDeadStrip: Bool = true ) -> BuildParameters { return BuildParameters( dataPath: buildPath, @@ -506,7 +506,7 @@ final class BuildPlanTests: XCTestCase { XCTAssertNoDiagnostics(observability.diagnostics) let result = try BuildPlanResult(plan: BuildPlan( - buildParameters: mockBuildParameters(config: .release, linkerDeadStrip: true), + buildParameters: mockBuildParameters(config: .release, linkerDeadStrip: false), graph: graph, fileSystem: fs, observabilityScope: observability.topScope