Simplify syscall register and bind #24546
Merged
Conversation
This file contains 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
jackcmay
commented
Apr 21, 2022
| @@ -146,6 +146,9 @@ pub fn create_executor( | |||
| reject_callx_r10: invoke_context | |||
| .feature_set | |||
| .is_active(&reject_callx_r10::id()), | |||
| dynamic_stack_frames: false, | |||
| enable_sdiv: false, | |||
| optimize_rodata: false, | |||
There was a problem hiding this comment.
We can add features for these items in later PRs, @alessandrod I think these are all yours correct?
There was a problem hiding this comment.
Correct! Yup sure there's no rush. For the first two we don't have a (released) toolchain that can exercise those features yet anyway, and for optimize_rodata I want to land solana-labs/rbpf#301 first
jeffwashington
pushed a commit
to jeffwashington/solana
that referenced
this pull request
Apr 22, 2022
Lichtso
added a commit
to Lichtso/solana
that referenced
this pull request
Apr 25, 2022
jackcmay
commented
Apr 26, 2022
| .register_syscall_by_name(b"sol_get_rent_sysvar", SyscallGetRentSysvar::call)?; | ||
| register_feature_gated_syscall!( | ||
| syscall_registry, | ||
| disable_fees_sysvar, |
There was a problem hiding this comment.
@Lichtso I suspect this is the issue; invert of the logic on the disable_fees_sysvar, should be !disable_fees_sysvar.
There was a problem hiding this comment.
Yes, looks promising. Can you test that?
- Checkout 6bbfef7 the commit before Bumps solana_rbpf to v0.2.27 #24694 made it in
- Revert the revert (revert #24546 #24647)
- Fix this boolean inversion
- Test by running a validator
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Apr 27, 2022
This reverts commit f6257a4.
This was referenced Apr 27, 2022
jackcmay
added a commit
to jackcmay/solana
that referenced
this pull request
Apr 27, 2022
jeffwashington
pushed a commit
to jackcmay/solana
that referenced
this pull request
Apr 27, 2022
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Apr 27, 2022
* Revert "declare syscalls with macro (solana-labs#24564)" 38bdb40 * Revert "Simplify syscall register and bind (solana-labs#24546)" 28ed2a9 Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
jackcmay
added a commit
to jackcmay/solana
that referenced
this pull request
Apr 27, 2022
Lichtso
pushed a commit
that referenced
this pull request
Apr 28, 2022
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Jun 29, 2022
* Revert "declare syscalls with macro (solana-labs#24564)" 38bdb40 * Revert "Simplify syscall register and bind (solana-labs#24546)" 28ed2a9 Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
jeffwashington
pushed a commit
to jeffwashington/solana
that referenced
this pull request
Jun 29, 2022
* Revert "Bumps solana_rbpf to v0.2.27 (solana-labs#24694)" This reverts commit f3d27cc. * Simplify syscall register and bind (solana-labs#24546) * declare syscalls with macro (solana-labs#24564)
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Jun 29, 2022
* Revert "declare syscalls with macro (solana-labs#24564)" 38bdb40 * Revert "Simplify syscall register and bind (solana-labs#24546)" 28ed2a9 Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Jun 29, 2022
* Revert "declare syscalls with macro (solana-labs#24564)" 38bdb40 * Revert "Simplify syscall register and bind (solana-labs#24546)" 28ed2a9 Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
jeffwashington
added a commit
to jeffwashington/solana
that referenced
this pull request
Jun 29, 2022
* Revert "declare syscalls with macro (solana-labs#24564)" 38bdb40 * Revert "Simplify syscall register and bind (solana-labs#24546)" 28ed2a9 Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Syscall registration and binding are currently disjoint operations requiring the callers to explicitly do both for each syscall.
This can lead to mismatch or missing registration/binds and complicates the code. Another side effect is that it is difficult/impossible given the current API for callers to filter syscalls at the registration stage but allow them at the binding stage. One example is the effort to deprecate/disallow syscalls by preventing new deployments of programs that rely on a disallowed syscall while continuing to support existing programs that already rely on them.
Summary of Changes
These changes depend and build off the changes here: solana-labs/rbpf#293
Syscalls are registered once, and later the bind operation uses a common context to bind all the previously registered syscalls. Thus, no longer requires an explicit bind for each syscall and without the complications of keeping two identical lists of syscalls for registration and bind.
Fixes #