Codesign w/ debugging entitlement for backtraces on macOS #7010
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,13 +33,19 @@ extension LLBuildManifestBuilder { | |
| testInputs = [] | ||
| } | ||
|
|
||
| // Create a phony node to represent the entire target. | ||
| let targetName = try buildProduct.product.getLLBuildTargetName(config: self.buildConfig) | ||
|
There was a problem hiding this comment. please check if this has performance implications, I vaguely remeber @neonichu ran into something like that with this API, tho no 100% sure There was a problem hiding this comment. This phony node is not new, it's only moved from previous L62 where it was declared too late for us. I had to move it up to this line. |
||
| let output: Node = .virtual(targetName) | ||
|
|
||
| let finalProductNode: Node | ||
| switch buildProduct.product.type { | ||
| case .library(.static): | ||
| finalProductNode = try .file(buildProduct.binaryPath) | ||
| try self.manifest.addShellCmd( | ||
| name: cmdName, | ||
| description: "Archiving \(buildProduct.binaryPath.prettyPath())", | ||
| inputs: (buildProduct.objects + [buildProduct.linkFileListPath]).map(Node.file), | ||
| outputs: [finalProductNode], | ||
| arguments: try buildProduct.archiveArguments() | ||
| ) | ||
|
|
||
|
|
@@ -49,23 +55,55 @@ extension LLBuildManifestBuilder { | |
| + [buildProduct.linkFileListPath] | ||
| + testInputs | ||
|
|
||
| let shouldCodeSign: Bool | ||
| let linkedBinaryNode: Node | ||
| let linkedBinaryPath = try buildProduct.binaryPath | ||
| if case .executable = buildProduct.product.type, | ||
| buildParameters.targetTriple.isMacOSX, | ||
| buildParameters.debuggingParameters.shouldEnableDebuggingEntitlement { | ||
| shouldCodeSign = true | ||
| linkedBinaryNode = try .file(buildProduct.binaryPath, isMutated: true) | ||
| } else { | ||
| shouldCodeSign = false | ||
| linkedBinaryNode = try .file(buildProduct.binaryPath) | ||
| } | ||
|
|
||
| try self.manifest.addShellCmd( | ||
| name: cmdName, | ||
| description: "Linking \(buildProduct.binaryPath.prettyPath())", | ||
| inputs: inputs.map(Node.file), | ||
| outputs: [linkedBinaryNode], | ||
| arguments: try buildProduct.linkArguments() | ||
| ) | ||
|
|
||
| if shouldCodeSign { | ||
| let basename = try buildProduct.binaryPath.basename | ||
| let plistPath = try buildProduct.binaryPath.parentDirectory | ||
| .appending(component: "\(basename)-entitlement.plist") | ||
| self.manifest.addEntitlementPlistCommand( | ||
| entitlement: "com.apple.security.get-task-allow", | ||
| outputPath: plistPath | ||
| ) | ||
|
|
||
| let cmdName = try buildProduct.product.getCommandName(config: self.buildConfig) | ||
| let codeSigningOutput = Node.virtual(targetName + "-CodeSigning") | ||
| try self.manifest.addShellCmd( | ||
| name: "\(cmdName)-entitlements", | ||
| description: "Applying debug entitlements to \(buildProduct.binaryPath.prettyPath())", | ||
| inputs: [linkedBinaryNode, .file(plistPath)], | ||
| outputs: [codeSigningOutput], | ||
| arguments: buildProduct.codeSigningArguments(plistPath: plistPath, binaryPath: linkedBinaryPath) | ||
| ) | ||
| finalProductNode = codeSigningOutput | ||
| } else { | ||
| finalProductNode = linkedBinaryNode | ||
| } | ||
| } | ||
|
|
||
| self.manifest.addNode(output, toTarget: targetName) | ||
| self.manifest.addPhonyCmd( | ||
| name: output.name, | ||
| inputs: [finalProductNode], | ||
| outputs: [output] | ||
| ) | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -477,6 +477,12 @@ public struct BuildOptions: ParsableArguments { | |
| ) | ||
| public var linkTimeOptimizationMode: LinkTimeOptimizationMode? | ||
|
|
||
| @Flag(help: .hidden) | ||
| public var enableGetTaskAllowEntitlement: Bool = false | ||
|
|
||
| @Flag(help: .hidden) | ||
| public var disableGetTaskAllowEntitlement: Bool = false | ||
|
There was a problem hiding this comment. you can probably marge this into one optional Bool setting There was a problem hiding this comment. It doesn't look There was a problem hiding this comment. What is the difference between true/false/nil behaviors? There was a problem hiding this comment. IIRC I had to provide a default value, but the default value depended on values of other options. I assume There was a problem hiding this comment. Hmm so you want to infer true/false based on other flags unless explicitly specified... that is reasonable. I don't think we support Optional Flags yet, but I wonder if you could use an EnumerableFlag instead. There was a problem hiding this comment. You can actually have a boolean flag as long as there's an inversion, which I think is exactly what you want in this case. The declaration would look like: @Flag(inversion: .prefixedEnableDisable)
var getTaskAllowEntitlement: Bool?It looks like we might not allow an There was a problem hiding this comment. yes, this is what I had in mind. @MaxDesiatov There was a problem hiding this comment. My bad, it actually does support an optional default value, at least with There was a problem hiding this comment. Addressed in #7028 |
||
|
|
||
| // @Flag works best when there is a default value present | ||
| // if true, false aren't enough and a third state is needed | ||
| // nil should not be the goto. Instead create an enum | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the Swift open source project | ||
| // | ||
| // Copyright (c) 2020-2023 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See http://swift.org/LICENSE.txt for license information | ||
| // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import struct Basics.Triple | ||
| import enum PackageModel.BuildConfiguration | ||
|
|
||
| extension BuildParameters { | ||
| public struct Debugging: Encodable { | ||
| public init( | ||
| debugInfoFormat: DebugInfoFormat = .dwarf, | ||
| targetTriple: Triple, | ||
| shouldEnableDebuggingEntitlement: Bool | ||
| ) { | ||
| self.debugInfoFormat = debugInfoFormat | ||
|
|
||
| // Per rdar://112065568 for backtraces to work on macOS a special entitlement needs to be granted on the final | ||
| // executable. | ||
| self.shouldEnableDebuggingEntitlement = targetTriple.isMacOSX && shouldEnableDebuggingEntitlement | ||
| } | ||
|
|
||
| public var debugInfoFormat: DebugInfoFormat | ||
|
|
||
| /// Whether the produced executable should be codesigned with the debugging entitlement, enabling enhanced | ||
| /// backtraces on macOS. | ||
| public var shouldEnableDebuggingEntitlement: Bool | ||
| } | ||
|
|
||
| /// Represents the debugging strategy. | ||
| /// | ||
| /// Swift binaries requires the swiftmodule files in order for lldb to work. | ||
| /// On Darwin, linker can directly take the swiftmodule file path using the | ||
| /// -add_ast_path flag. On other platforms, we convert the swiftmodule into | ||
| /// an object file using Swift's modulewrap tool. | ||
| public enum DebuggingStrategy { | ||
| case swiftAST | ||
| case modulewrap | ||
| } | ||
|
|
||
| /// The debugging strategy according to the current build parameters. | ||
| public var debuggingStrategy: DebuggingStrategy? { | ||
| guard configuration == .debug else { | ||
| return nil | ||
| } | ||
|
|
||
| if targetTriple.isApple() { | ||
| return .swiftAST | ||
| } | ||
| return .modulewrap | ||
| } | ||
|
|
||
| /// Represents the debug information format. | ||
| /// | ||
| /// The debug information format controls the format of the debug information | ||
| /// that the compiler generates. Some platforms support debug information | ||
| // formats other than DWARF. | ||
| public enum DebugInfoFormat: String, Encodable { | ||
| /// DWARF debug information format, the default format used by Swift. | ||
| case dwarf | ||
| /// CodeView debug information format, used on Windows. | ||
| case codeview | ||
| /// No debug information to be emitted. | ||
| case none | ||
| } | ||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to have a command line option to let you specify the code signing identity (here we're using
-).Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO that should come as a separate PR to make this facility a bit more general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I only mention it because people who are aware of how this works will pretty quickly notice that we're signing things and ask how to sign with a different identity.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, but signing with a different identity or with a different entitlement requires a public API design and likely an evolution proposal. Scoping this to one specific case means we don't have to block backtraces with this potential future design work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. No objection from me.