Rollup of 13 pull requests - #162293
Closed
JonathanBrouwer wants to merge 32 commits into
Closed
Conversation
Store identity argument indices instead of bound generic arguments so callers can index concrete alias arguments without changing rigidness through instantiation.
Match the existing params_in_repr / unsizing_params convention; the compiler already treats generic arg counts as u32-sized.
Treat missing outlives information as no restriction so all sources can be intersected uniformly. Keep bivariant alias arguments out of the final region walk.
`User::new_uninit_bytes` and `User::drop` are asking the host to alloc/dealloc memory with potentially mismatched alignment, as the enclave side is unconditionally over-aligning on allocation but not doing the same on free. - Ex: `User::<ByteBuffer>` -> `alloc(_, align=8)` -> `drop()` -> `free(_, align=1)` For most hosts running stock x86_64-linux + glibc malloc, I don't believe this mismatch is an issue, since posix `free` ignores the alignment anyway. My guess is that if you're using jemalloc, which does care about the dealloc alignment, then something _might_ go wrong. It's also not clear that we can just round-up the alignment on `free`, since `User::from_raw` exists, and there's various places that call it outside std. We should probably just remove the min. alignment until we come up with a more satisfactory solution. My guess is that the right place to do the min. alignment optimization is in the host-side enclave-runner: <https://github.com/fortanix/rust-sgx/blob/be93e7abe92eff4b5610e15fe21b16196ace1e6e/intel-sgx/enclave-runner-sgx/src/usercalls/mod.rs#L1596> and other places that hand memory to the SGX enclave. NB. The min. alignment exists for performance reasons (see: `copy_from_userspace`). It's highly preferable if all memory copied from userspace is at least 8 byte aligned, otherwise we have to fallback to a super slow copy routine for the unaligned prefix (and suffix).
`Cargo::cargo` adds LLVM's library search path to `rustflags` for `ToolRustcPrivate`/`Codegen` so that tools linking against compiler libraries can find `libLLVM`. However, it always queried `host_llvm_config()`, which resolves to the *host*'s `llvm-config` regardless of the requested `target`. When cross-compiling, this appends the host's LLVM libdir to the target's link flags, which can cause linking to fail. Only use `llvm-config --libdir` when `target` is the host. Otherwise, ensure the `Llvm` step for `target` and derive the libdir from its `root_dir()` instead of invoking `llvm-config`, since the resulting binary may not be executable on the host if it was built for a different target.
There are various types used to carry name resolution results across crate boundaries. They are scattered across places like `rustc_middle::ty`, `rustc_middle::metadata`, and `rustc_hir::def`. This commit moves them into the new module, a more logical place for them to live. As part of this it eliminates the small `rustc_middle::metadata` module. One nice consequence of this change: it removes the single use of a `LocalDefId` in `rustc_ast`. (This is what got my attention in the first place.)
This sets `--all-features` when documenting bootstrap tool crates, and enables rustdoc's `#![feature(doc_cfg)]` to display which items are feature-gated.
bootstrap: use target's LLVM libdir when cross-compiling `Cargo::cargo` adds LLVM's library search path to `rustflags` for `ToolRustcPrivate`/`Codegen` so that tools linking against compiler libraries can find `libLLVM`. However, it always queried `host_llvm_config()`, which resolves to the *host*'s `llvm-config` regardless of the requested `target`. When cross-compiling, this appends the host's LLVM libdir to the target's link flags, which can cause linking to fail. Only use `llvm-config --libdir` when `target` is the host. Otherwise, ensure the `Llvm` step for `target` and derive the libdir from its `root_dir()` instead of invoking `llvm-config`, since the resulting binary may not be executable on the host if it was built for a different target.
… r=JohnTitor std::sys::pal::sgx: fix mismatched alloc/free alignment ### Why the PR? I've got a local `miri` branch that's able to test `x86_64-fortanix-unknown-sgx`, so I can get better assurance about our enclaves. It's now complaining about a bunch of stuff in std :sweat_smile: ### Context 1. `x86_64-fortanix-unknown-sgx` enclaves can request the untrusted host enclave runner to allocate/free memory in userspace and get a pointer to it in return. 2. There's a userspace/enclave space memory split for `x86_64-fortanix-unknown-sgx` enclaves. It's a bit like the userspace/kernel space split, where the kernel doesn't trust pointers from userspace and is very paranoid about copying data to/from userspace. ### Problem In the enclave, `User::new_uninit_bytes` and `User::drop` are requesting the host to alloc/dealloc memory with potentially mismatched alignment, as the enclave side is unconditionally over-aligning on allocation but not doing the same on free. - Ex: `User::<ByteBuffer>` -> `alloc(_, align=8)` -> `drop()` -> `free(_, align=1)` See: <https://github.com/rust-lang/rust/blob/main/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs> ```rust // Enclave-side impl<T: ?Sized> User<T> where T: UserSafe, { // This function returns memory that is practically uninitialized, but is // not considered "unspecified" or "undefined" for purposes of an // optimizing compiler. This is achieved by returning a pointer from // from outside as obtained by `super::alloc`. fn new_uninit_bytes(size: usize) -> Self { unsafe { // Mustn't call alloc with size 0. let ptr = if size > 0 { // `copy_to_userspace` is more efficient when data is 8-byte aligned let alignment = cmp::max(T::align_of(), 8); // <------------------------- HERE rtunwrap!(Ok, super::alloc(size, alignment)) as _ } else { T::align_of() as _ // dangling pointer ok for size 0 }; if let Ok(v) = crate::panic::catch_unwind(|| T::from_raw_sized(ptr, size)) { User(NonNull::new_userref(v)) } else { rtabort!("Got invalid pointer from alloc() usercall") } } } // ... } // ... impl<T: ?Sized> Drop for User<T> where T: UserSafe, { fn drop(&mut self) { unsafe { let ptr = (*self.0.as_ptr()).0.get(); // vvvvvvvvvvvvv------------------ HERE super::free(ptr as _, size_of_val(&mut *ptr), T::align_of()); } } } ``` This min. alignment optimization was introduced in rust-lang@6f7d193. See below for more details on why. The two usercalls, `super::alloc` and `super::free`, are eventually handled by the host runner. They just delegate to the `System` allocator: See: <https://github.com/fortanix/rust-sgx/blob/master/intel-sgx/enclave-runner-sgx/src/usercalls/mod.rs> ```rust // Host-side / userspace impl<'tcs> IOHandlerInput<'tcs> { // ... #[inline(always)] fn alloc(&self, size: usize, alignment: usize) -> IoResult<*mut u8> { unsafe { // vvvvvvvvv--------------- UNCHANGED let layout = Layout::from_size_align(size, alignment) .map_err(|_| IoErrorKind::InvalidInput)?; if layout.size() == 0 { return Err(IoErrorKind::InvalidInput.into()); } let ptr = System.alloc(layout); if ptr.is_null() { Err(IoErrorKind::Other.into()) } else { Ok(ptr) } } } #[inline(always)] fn free(&self, ptr: *mut u8, size: usize, alignment: usize) -> IoResult<()> { unsafe { // vvvvvvvvv--------------- UNCHANGED let layout = Layout::from_size_align(size, alignment) .map_err(|_| IoErrorKind::InvalidInput)?; if size == 0 { return Ok(()); } Ok(System.dealloc(ptr, layout)) } } // ... } ``` It also appears that `enclave-runner-sgx` assumes that there's no `#[global_allocator]` override (<https://github.com/fortanix/rust-sgx/blob/master/intel-sgx/enclave-runner-sgx/src/usercalls/interface.rs#L333>). For most enclave hosts running stock x86_64-unknown-linux-gnu (glibc malloc), I don't believe this mismatch is currently an issue, since posix `free` ignores the alignment anyway. If you did swap in jemalloc, which does care about the dealloc alignment, then something would definitely go wrong elsewhere, as the you'd have mismatched allocators (`System` above vs `Box<_>`/`Vec<_>` using `Global`). ### Solutions It's not clear that we can round-up the alignment on `free`, since `User::from_raw` exists, and there's various places that call it outside std. We should probably just remove the in-enclave min. alignment until we come up with a more satisfactory solution. My guess is that the right place to do the min. alignment optimization is on enclave-runner-sgx side: <https://github.com/fortanix/rust-sgx/blob/master/intel-sgx/enclave-runner-sgx/src/usercalls/mod.rs#L1596> and other places that hand memory to the SGX enclave. ### Why over-align in the first place? The min. alignment exists for performance reasons (see: `copy_from_userspace`). It's highly preferable if all memory copied from userspace is at least 8 byte aligned, otherwise we have to fallback to a super slow copy routine for the unaligned prefix (and suffix).
bootstrap: Include feature-gated items in bootstrap tool docs I noticed that the [nightly-rustc docs for `build_helper`](https://doc.rust-lang.org/nightly/nightly-rustc/build_helper/index.html) don't include the `metrics` module, because it's gated behind the non-default feature flag `feature = "metrics"`. This PR fixes that by using `--all-features` to document all features, and enabling rustdoc's `#![feature(doc_cfg)]` [(via -`Zcrate-attr`)](rust-lang#138287) so that rustdoc will indicate which items require feature flags. The first two commits are a general cleanup of `tool_doc!` to pull almost all of its non-trivial code out of the macro and into regular functions.
type system const items via direct rhs fixes rust-lang#161264 see also zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) a const item with a `direct!` rhs is now a type system transparent direct const, similar to a type const: ```rust const C: T = core::direct_const_arg!(V); ``` if the macroless feature is enabled, the macroless heuristic also applies here this PR puts us in an awkward middle ground, between the present world with `type const`, and the future of GCA as discussed in this zulip thread: [#project-const-generics > talkies at last](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/talkies.20at.20last/with/620616030) tl;dr we're yeeting `type const` and replacing it with `const C: T = gca!(V);`, and with this PR, *both* syntaxes are supported at the same time, which is weird and awkward. But, incremental improvement is good, doing the whole thing at once is too much! some notes on the change: - the is_type_const boolean on the DefKind now corresponds to whether the `type const` syntax is used, *not* whether it is a type system transparent const - the `const_of_item` query now returns `Option`, and is `Some` when it is a type system transparent direct const. - it is a little spicy that `const_of_item` returns `None` if there is no RHS rather than panicing - if it panics, it's vaguely annoying to guard against this in callsites, returning `None` is a bit more convenient. API design is hard, idk. - the `TyCtxt` method `is_direct_const` is true if it's either a `type const`, or if it's a direct const - this logic will eventually change to: is true if it either has the `#[always_gca]` attribute, or if it has a `gca!` rhs. - should we serialize the `const_of_item` query for anon consts, or just guard against the DefKind in some helper that returns the actually serialized query impl? Behavior is the same, idk perf jank or whatever. r? @BoxyUwU
…n, r=petrochenkov Introduce `rustc_middle::middel::resolve` There are various types used to carry name resolution results across crate boundaries. They are scattered across places like `rustc_middle::ty`, `rustc_middle::metadata`, and `rustc_hir::def`. This commit moves them into the new module, a more logical place for them to live. As part of this it eliminates the small `rustc_middle::metadata` module. One nice consequence of this change: it removes the single use of a `LocalDefId` in `rustc_ast`. (This is what got my attention in the first place.) r? @petrochenkov
box: fixup map/try_map deallocate calls
`Box::{map, try_map}` unsoundly called `deallocate` on the previous box's pointer, even if it was a ZST. This was probably okay for `Global` but it's definitely not for arbitrary custom allocators.
r? libs
…, r=Darksonn alloc: a bunch of safety comments Following up from rust-lang#160941. Triaging this is what found the errors in rust-lang#162285 & rust-lang#162286. More to come, but I didn't want to make the review effort too high on any single PR. r? libs
…xt-triats-pt2, r=lcnr Remove `RegionExt`; move methods to `Region` in `rustc_type_ir` Removes `RegionExt` from `rustc_middle` with all methods on `Region`. Some changes I think are worth pointing out (of which are all in the first commit -> f949bb7) ; - Changed the signature of `Region::new_late_param` to accept a `I::LateParamRegion` where previously it was able to construct a `LateParamRegion` from some method parameters. **added to interner:** - `fn span_delayed_bug(self, span: Self::Span, msg: impl ToString) -> Self::ErrorGuaranteed;` which could be useful elsewhere when porting things across to `rustc_type_ir` - `fn generics_of_early_param_region_def_id(self, def_id: Self::DefId, ebr: Self::EarlyParamRegion) -> Self::DefId;` which is quite nasty but calling `generics_of` returned another type that I would have possibly create a trait for which felt more messy. - `fn get_re_var_lifetime(self, var_idx: usize) -> Option<Region<'tcx>>` need to get a region in `Region::new_var`. **traits added to inherent** - `RegionName` so we can get the names of `LateParamRegion` and `EarlyParamRegion` with a `get_name()` and also `is_named()`. - `DefIdGetter` so we can get the `DefId` of `kind` in `LateParamRegion` r? lcnr Part of rust-lang#159654
…t, r=lcnr,jackh726,adwinwhite traits: Represent live alias arguments as bitsets Split out of rust-lang#160212 per review / Zulip: https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/rigid.20aliases.20in.20region.20handling/near/615557841 `live_args_for_alias_from_outlives_bounds` and `args_known_to_outlive_alias_params` now return identity arg indices (`DenseBitSet`) instead of `EarlyBinder<GenericArg>`. Callers just do `args[idx].visit_with(...)`, so we stop laundering rigidness through binder instantiate. Also drops the old BitSet FIXME. imo this is worth doing on its own even without the ICE fix. every time we shoved identity params through `EarlyBinder` we were writing down something we don't actually know, and this module is only going to grow. better to make the invariant explicit now than keep paying for it later. No behavioral change intended. the rigid-alias ICE / `extract_verify_if_eq` bits stay on rust-lang#160212. btw if that one should stack on this instead of staying independent, lmk and I'll rebase it asap.
Implement `Rng` for `Box` Tracking issue: rust-lang#130703 Like rust-lang#159435, this implementation must be implemented before the stabilization of `Rng`. r? @joshtriplett
…nethercote Fix incorrect meta span Fixes rust-lang#161472 We should use `attr_item.span` which covers the entire `MetaItem`.
…itor std: fix typo
Add regression test from 1.98.1 Regression test for rust-lang#161441. The file contents are taken directly from [`41c7a25` (rust-lang#161555)](rust-lang@41c7a25), which was previously committed directly to the 1.98.1 stable release, but not to the main branch.
rustbot
added
O-SGX
Member
Author
Contributor
rust-bors
Bot
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Sep 4, 2026
Rollup of 13 pull requests try-job: dist-various-1 try-job: test-various try-job: x86_64-gnu-aux try-job: x86_64-gnu-llvm-21-3 try-job: x86_64-msvc-1 try-job: aarch64-apple-1 try-job: aarch64-apple-2 try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: i686-msvc-2
Member
|
Closing in favour of #162297. |
rust-bors
Bot
added
S-waiting-on-author
Contributor
|
This pull request was unapproved due to being closed. |
Contributor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
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.
Successful merges:
rustc_middle::middle::resolve#162277 (Introducerustc_middle::middel::resolve)RegionExt; move methods toRegioninrustc_type_ir#160509 (RemoveRegionExt; move methods toRegioninrustc_type_ir)RngforBox#162212 (ImplementRngforBox)r? @ghost
Create a similar rollup