-
Notifications
You must be signed in to change notification settings - Fork 135
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
Compile with stable Rust #1372
Comments
For a first step we can focus on getting Enarx onto stable, although we'll eventually need to get all of its dependencies onto stable as well (after a cursory examination I think the only dependency currently making use of unstable features is Sallyport). Here's a list of all the unstable features currently in use by Enarx. These should all eventually have their own tracking issue once some cursory investigation has been done to determine the best course of action for each feature.
|
For me this is a mandatory requirement to include Enarx to my regular kernel testing flow. Not having this makes it extremely hard to compile Enarx into initramfs bundled with the kernel image, when compiling a minimal rootfs with BuildRoot. I already have in-progress BuildRoot fork waiting for this: https://github.com/jarkkojs/buildroot-enarx. |
Could we take the following soft path:
This way we converge to the goal but it does not require any major rework at once. To summarize: we should consider stable Rust implementation "better" in a code review when it is possible in the first place. |
Can you provide some short plain English descriptions what these are (by editing the original comment)? |
One more thing to note for the future: as soon as rust-lang/cargo#9992 lands we'll begin using a new, unstable |
@jarkkojs can you use |
What is it? |
I fully understand this. Not having that causes some awkwardness in the current codebase. |
RUSTC_BOOTSTRAP is a secret implementation detail of the Rust compiler that allows nightly Rust to pretend to be stable Rust, which is used for the purpose of bootstrapping the standard library during the release process. It is not to be used by anyone other than the Rust compiler in any production capacity; if I were a code auditor I would reject any codebase that used it. |
Regarding |
We may also begin relying on a new feature, |
I have confirmed that the proposed #1608 removes all uses of the |
Not using them. Heap is created dynamically.It allows to manager virtual memory areas with their real addresses. @npmccallum i.e. I'm using ledger with real coordinates now. |
I've also just noticed that one of the tests has |
While poking around in our dependencies it appears as though the only external dependency (i.e. not in the Enarx repo) that makes use of unstable features is sallyport. I've identified two new unstable features used by sallyport, |
Make enarx and all dependencies compile on a stable release of Rust, using only stable Rust features. This may involve several approaches:
Feature status
-Z bindeps
(Cargo feature): high risk. This feature (a.k.a. "artifact dependencies") represents a substantial amount of new code in Cargo and a handful of issues have already surfaced: https://github.com/rust-lang/cargo/issues?q=is%3Aissue+is%3Aopen+artifact+label%3AZ-bindeps . Silver lining is that the fact that others are finding issues means that other people are eager to use this in earnest, which is important for eventual stabilization. The Cargo team may be reluctant to stabilize as long as outstanding issues exist. Furthermore, the Cargo team is relatively understaffed and may not have the energy necessary to pursue stabilization without significant external assistance.-Z sparse-registry
(Cargo feature):low risk. This is a nice-to-have for achieving free speedups in CI and during ordinary development, but not strictly necessary. It also is likely on track to stabilize in the short term.Resolved by Stabilize sparse-registry rust-lang/cargo#11224 .asm_const
: medium risk. The transitive blocker for this,inline_consts
, is blocked on a relatively minor syntactic question. However, even this is enough to delay stabilization indefinitely. Fortunately, if syntax is truly the only remaining blocker forinline_const
, then we may be able to remove it as a blocker forasm_const
. The path forward here is to either resolve the syntactic question forinline_const
or to convince Amanieu to decouple it fromasm_const
, which will require getting assurance from those behind the const machinery that the semantics are set in stone.asm_sym
:medium risk. Blocked on rewriting sym support forResolved by build: bump Rust toolchain to 2022-11-04 #2303 .global_asm!
, which is underway (Implement sym operands for global_asm! rust-lang/rust#94468) but we need to clarify whether or not that alone will suffice to unblockasm_sym
. Even if so, it's unclear when that work will be completed.naked_functions
:low risk. The lang team has all but greenlit the stabilization of this feature, contingent on the resolution of a small number of open questions that should not be difficult to resolve. We are actively contributing towards stabilization here: [Tracking]: Pursue stabilization for Rust RFC 2972: "Constrained Naked Functions" #1269Resolved by feat: remove naked functions #2350 .x86_64-unknown-none
(as a way to avoid the need for-Z build-std
):low risk. This has a prominent advocate in Josh Triplett, and we can help him out by doing the labor of initiating the process. We are actively contributing towards stabilization here: Pursue promoting x86_64-unknown-none to a Tier 2 Rust platform #1528Resolved by Promote x86_64-unknown-none target to Tier 2 and distribute build artifacts rust-lang/rust#95705 .generic_const_exprs
andconst_mut_refs
:low risk. The code making use of these features needs to be rewritten for SGX2, which is a deliverable for Enarx 0.4, so we just need to ensure that the eventual rewrite doesn't make use of these.Resolved by refactor(shim-sgx): remove unnecessary feature flags #1655 .c_size_t
: low risk. These features provide trivial nice-to-have type aliases. I foresee little resistance to stabilizing them, but even if they are not stabilized we could trivially remove our usage of them without difficulty.core_ffi_c
:low risk. Same as above.Resolved by Stabilizecore::ffi:c_*
and re-export instd::ffi
rust-lang/rust#98315 .nonnull_slice_from_raw_parts
: low risk. This is a minor library addition that is already on the verge of stabilization. All that is required is for a few more libs team members to check their boxes and for a stabilization report to be filed, and the latter is something that we can contribute to move this along.slice_ptr_len
: low risk. At first glance this appears problematic as concerns have been raised about the feature in general, however this feature provides APIs on three types, and the outstanding concerns only apply to two of those types, whereas the third type is fine, and we're exclusively using this API on that third type. Other users have already noticed this and have split out the non-problematic API into its own feature, which is well on its way towards stabillization (Partially stabilize(const_)slice_ptr_len
feature by stabilizingNonNull::len
rust-lang/rust#94640) and just needs a nudge to push it over the finish line, which we can aid with.slice_ptr_get
: medium risk. This API is currently being employed as a way to avoid potential UB in one place in sallyport (see fix(sallyport): avoid UB by removing implicit reference from indexing with range #2069). There may be other ways to work around the issue, although given the security implications I would prefer to use the standard API if able. Stabilization of this API is highly motivated by it being possibly the only safe way to perform certain obscure unsafe code patterns, but whether or not there is appetite to stabilize it quickly is unknown. It's also possible that the language itself will change to make the use of this API unnecessary.The text was updated successfully, but these errors were encountered: