Skip to content

🍒[5.7][IRGen/Distributed] Adjust protocol requirement and witness table to handle distributed thunks #60189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 22, 2022

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Jul 22, 2022

Description: Pointer authentication would fail on arm64e for distributed method witnesses where the requirement. This PR makes the necessary changes for the verification to look at the right discriminators.

  • Determine whether protocol requirement is async using SILDeclRef

This is necessary because SILDeclRef of the distributed thunk witness is anchored
on the requirement declaration which might not be async, but the witness itself is
always async throws.

  • Mark protocol requirement descriptor for distributed thunk as async

Fixes a bug where descriptor for protocol requirement associated with
distributed thunk wasn't marked as async that results in ptrauth failures
because discriminator would be misaligned between requirement and witness.

Risk: Low, only fixes pointer auth for distributed protocol requirements and actors
Review by: @DougGregor @tkremenek
Testing: PR Testing, validated on arm64e
Original PR: #60184
Radar: rdar://97228310 rdar://96520492

xedin added 3 commits July 22, 2022 09:26
…lRef`

While building witness table record, let's use `hasAsync()` check on
`SILDeclRef` of the requirement instead of reaching for the underlying
declaration because the reference should be the source of truth about
`async` and other attributes.

In case of distributed thunk witness, which is always `async throws`,
the underlying declaration could be sync because it's the protocol
requirement declaration.
… async

Fixes a bug where descriptor for protocol requirement associated with
distributed thunk wasn't marked as `async` that results in ptrauth failures
because discriminator would be misaligned between requirement and witness.

Resolves: rdar://96520492
@ktoso ktoso requested a review from a team as a code owner July 22, 2022 00:30
@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

@swift-ci please test apple silicon

@ktoso ktoso added distributed Feature → concurrency: distributed actor r5.7 labels Jul 22, 2022
@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

@swift-ci please nominate

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

5.7 Linux CI is broken: rdar://97408321

/home/build-user/swift-driver/Sources/swift-help/main.swift:156:1: error: expressions are not allowed at the top level
SwiftHelp.main()
^
error: fatalError

@shahmishal
Copy link
Member

@xedin
Copy link
Contributor

xedin commented Jul 22, 2022

swiftlang/swift-package-manager#5684
@swift-ci please test macOS platform

@xedin
Copy link
Contributor

xedin commented Jul 22, 2022

swiftlang/swift-package-manager#5684
@swift-ci please test apple silicon

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

Reverts got merged (and thus failures like swiftpm failed (ret=128): ['git', 'fetch', 'origin', 'pull/5684/merge:ci_pr_5684', '--tags'] fatal: couldn't find remote ref pull/5684/merge; update-checkout failed, fix errors and try again) trying again then...

@swift-ci please test apple silicon

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

@swift-ci please test apple silicon

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Jul 22, 2022

We're green, yay! Just need approval now 🙏

@ktoso ktoso merged commit d8b7ece into release/5.7 Jul 22, 2022
@ktoso ktoso deleted the pick-rdar-96520492 branch July 22, 2022 22:22
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants