Fix FFI-unwind unsoundness with mixed panic mode
UB maybe introduced when an FFI exception happens in a `C-unwind` foreign function and it propagates through a crate compiled with `-C panic=unwind` into a crate compiled with `-C panic=abort` (#96926).
To prevent this unsoundness from happening, we will disallow a crate compiled with `-C panic=unwind` to be linked into `panic-abort` *if* it contains a call to `C-unwind` foreign function or function pointer. If no such call exists, then we continue to allow such mixed panic mode linking because it's sound (and stable). In fact we still need the ability to do mixed panic mode linking for std, because we only compile std once with `-C panic=unwind` and link it regardless panic strategy.
For libraries that wish to remain compile-once-and-linkable-to-both-panic-runtimes, a `ffi_unwind_calls` lint is added (gated under `c_unwind` feature gate) to flag any FFI unwind calls that will cause the linkable panic runtime be restricted.
In summary:
```rust
#![warn(ffi_unwind_calls)]
mod foo {
#[no_mangle]
pub extern "C-unwind" fn foo() {}
}
extern "C-unwind" {
fn foo();
}
fn main() {
// Call to Rust function is fine regardless ABI.
foo::foo();
// Call to foreign function, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`.
unsafe { foo(); }
//~^ WARNING call to foreign function with FFI-unwind ABI
let ptr: extern "C-unwind" fn() = foo::foo;
// Call to function pointer, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`.
ptr();
//~^ WARNING call to function pointer with FFI-unwind ABI
}
```
Fix#96926
`@rustbot` label: T-compiler F-c_unwind
Only keep a single query for well-formed checking
There are currently 3 queries to perform wf checks on different item-likes. This complexity is not required.
This PR replaces the query by:
- one query per item;
- one query to invoke it for a whole module.
This allows to remove HIR `ParItemLikeVisitor`.
Rename `impl_constness` to `constness`
The current code is a basis for `is_const_fn_raw`, and `impl_constness`
is no longer a valid name, which is previously used for determining the
constness of impls, and not items in general.
r? `@oli-obk`
The current code is a basis for `is_const_fn_raw`, and `impl_constness`
is no longer a valid name, which is previously used for determining the
constness of impls, and not items in general.
Compute lifetimes in scope at diagnostic time
The set of available lifetimes is currently computed during lifetime resolution on HIR. It is only used for one diagnostic.
In this PR, HIR lifetime resolution just reports whether elided lifetimes are well-defined at the place of use. The diagnostic code is responsible for building a list of lifetime names if elision is not allowed.
This will allow to remove lifetime resolution on HIR eventually.
Iterate over `maybe_unused_trait_imports` when checking dead trait imports
Closes#96873
r? `@cjgillot`
Some questions, if you have time:
- Is there a way to shorten the `rustc_data_structures::fx::FxIndexSet` path in the query declaration? I wasn't sure where to put a `use`.
- Was returning by reference from the query the right choice here?
- How would I go about evaluating the importance of the `is_dummy()` call in `check_crate`? I don't see failing tests when I comment it out. Should I just try to determine whether dummy spans can ever be put into `maybe_unused_trait_imports`?
- Am I doing anything silly with the various ID types?
- Is that `let-else` with `unreachable!()` bad? (i.e is there a better idiom? Would `panic!("<explanation>")` be better?)
- If I want to evaluate the perf of using a `Vec` as mentioned in #96873, is the best way to use the CI or is it feasible locally?
Thanks :)
Try to cache region_scope_tree as a query
This PR will attempt to restore `region_scope_tree` as a query so that caching works again. It seems that `region_scope_tree` could be re-computed for nested items after all, which could explain the performance regression introduced by #95563.
cc `@Mark-Simulacrum` `@pnkfelix` I will try to trigger a perf run here.
Cache more queries on disk
One of the principles of incremental compilation is to allow saving results on disk to avoid recomputing them.
This PR investigates persisting a lot of queries whose result are to be saved into metadata.
Some of the queries are cheap reads from HIR, but we may also want to get rid of these reads for incremental lowering.
Add a query for checking whether a function is an intrinsic.
work towards #93145
This will reduce churn when we add more ways to declare intrinsics
r? `@scottmcm`
don't encode only locally used attrs
Part of https://github.com/rust-lang/compiler-team/issues/505.
We now filter builtin attributes before encoding them in the crate metadata in case they should only be used in the local crate. To prevent accidental misuse `get_attrs` now requires the caller to state which attribute they are interested in. For places where that isn't trivially possible, I've added a method `fn get_attrs_unchecked` which I intend to remove in a followup PR.
After this pull request landed, we can then slowly move all attributes to only be used in the local crate while being certain that we don't accidentally try to access them from extern crates.
cc https://github.com/rust-lang/rust/pull/94963#issuecomment-1082924289
Cleanup `DebuggerVisualizerFile` type and other minor cleanup of queries.
Merge the queries for debugger visualizers into a single query.
Revert move of `resolve_path` to `rustc_builtin_macros`. Update dependencies in Cargo.toml for `rustc_passes`.
Respond to PR comments. Load visualizer files into opaque bytes `Vec<u8>`. Debugger visualizers for dynamically linked crates should not be embedded in the current crate.
Update the unstable book with the new feature. Add the tracking issue for the debugger_visualizer feature.
Respond to PR comments and minor cleanups.
Implement Valtree to ConstValue conversion
Once we start to use `ValTree`s in the type system we will need to be able to convert them into `ConstValue` instances, which we want to continue to use after MIR construction.
r? `@oli-obk`
cc `@RalfJung`
Generate synthetic object file to ensure all exported and used symbols participate in the linking
Fix#50007 and #47384
This is the synthetic object file approach that I described in https://github.com/rust-lang/rust/pull/95363#issuecomment-1079932354, allowing all exported and used symbols to be linked while still allowing them to be GCed.
Related #93791, #95363
r? `@petrochenkov`
cc `@carbotaniuman`
Improved diagnostic on failure to meet send bound on future in a foreign crate
Provide a better diagnostic on failure to meet send bound on futures in a foreign crate.
fixes#78543
Refactor HIR item-like traversal (part 1)
Issue #95004
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- use tcx.hir_crate_items to introduce a tcx.hir().par_items(impl Fn(hir::ItemId)) to traverse all items in parallel;
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
cc `@cjgillot`
Adding diagnostic data on generators to the crate metadata and using it to provide
a better diagnostic on failure to meet send bound on futures originated from a foreign crate
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- add par_items(impl Fn(hir::ItemId)) to traverse all items in parallel
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
Remember mutability in `DefKind::Static`.
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
Swap DtorckConstraint to DropckConstraint
This change was made as per suspicion that this struct was never renamed after consistent use of DropCk.
This also clarifies the meaning behind the name of this structure.
Fixes https://github.com/rust-lang/rust/issues/94310
This change was made as per suspicion that this struct was never renamed after consistent use of DropCk.
This also clarifies the meaning behind the name of this structure.
There are a few places were we have to construct it, though, and a few
places that are more invasive to change. To do this, we create a
constructor with a long obvious name.
This commit makes `AdtDef` use `Interned`. Much the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
Make it possible to pretty print invalid constants by introducing a
fallible variant of `destructure_const` and falling back to debug
formatting when it fails.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.
Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
we need separate arena for it, because we can't use the `Dropless` one any
more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
Stabilize `-Z instrument-coverage` as `-C instrument-coverage`
(Tracking issue for `instrument-coverage`: https://github.com/rust-lang/rust/issues/79121)
This PR stabilizes support for instrumentation-based code coverage, previously provided via the `-Z instrument-coverage` option. (Continue supporting `-Z instrument-coverage` for compatibility for now, but show a deprecation warning for it.)
Many, many people have tested this support, and there are numerous reports of it working as expected.
Move the documentation from the unstable book to stable rustc documentation. Update uses and documentation to use the `-C` option.
Addressing questions raised in the tracking issue:
> If/when stabilized, will the compiler flag be updated to -C instrument-coverage? (If so, the -Z variant could also be supported for some time, to ease migrations for existing users and scripts.)
This stabilization PR updates the option to `-C` and keeps the `-Z` variant to ease migration.
> The Rust coverage implementation depends on (and automatically turns on) -Z symbol-mangling-version=v0. Will stabilizing this feature depend on stabilizing v0 symbol-mangling first? If so, what is the current status and timeline?
This stabilization PR depends on https://github.com/rust-lang/rust/pull/90128 , which stabilizes `-C symbol-mangling-version=v0` (but does not change the default symbol-mangling-version).
> The Rust coverage implementation implements the latest version of LLVM's Coverage Mapping Format (version 4), which forces a dependency on LLVM 11 or later. A compiler error is generated if attempting to compile with coverage, and using an older version of LLVM.
Given that LLVM 13 has now been released, requiring LLVM 11 for coverage support seems like a reasonable requirement. If people don't have at least LLVM 11, nothing else breaks; they just can't use coverage support. Given that coverage support currently requires a nightly compiler and LLVM 11 or newer, allowing it on a stable compiler built with LLVM 11 or newer seems like an improvement.
The [tracking issue](https://github.com/rust-lang/rust/issues/79121) and the [issue label A-code-coverage](https://github.com/rust-lang/rust/labels/A-code-coverage) link to a few open issues related to `instrument-coverage`, but none of them seem like showstoppers. All of them seem like improvements and refinements we can make after stabilization.
The original `-Z instrument-coverage` support went through a compiler-team MCP at https://github.com/rust-lang/compiler-team/issues/278 . Based on that, `@pnkfelix` suggested that this needed a stabilization PR and a compiler-team FCP.
Return an indexmap in `all_local_trait_impls` query
The data structure previously used here required that `DefId` be `Ord`. As part of #90317, we do not want `DefId` to implement `Ord`.
Make dead code check a query.
Dead code check is run for each invocation of the compiler, even if no modifications were involved.
This PR makes dead code check a query keyed on the module. This allows to skip the check when a module has not changed.
To perform this, a query `live_symbols_and_ignored_derived_traits` is introduced to encapsulate the global analysis of finding live symbols. The second query `check_mod_deathness` outputs diagnostics for each module based on this first query's results.
Store def_id_to_hir_id as variant in hir_owner.
If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.
Related to #89278
r? `@oli-obk`
If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.
[code coverage] Fix missing dead code in modules that are never called
The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).
The partitioning logic also caused issues in #85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.
This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.
Fixes#91661Fixes#86177Fixes#85718Fixes#79622
r? ```@tmandry```
cc ```@richkadel```
This PR is not urgent so please don't let it interrupt your holidays! 🎄🎁
rustdoc: Introduce a resolver cache for sharing data between early doc link resolution and later passes
The refactoring parts of https://github.com/rust-lang/rust/pull/88679, shouldn't cause any slowdowns.
r? `@jyn514`
Continue supporting -Z instrument-coverage for compatibility for now,
but show a deprecation warning for it.
Update uses and documentation to use the -C option.
Move the documentation from the unstable book to stable rustc
documentation.
The issue here is that the logic used to determine which CGU to put the
dead function stubs in doesn't handle cases where a module is never
assigned to a CGU.
The partitioning logic also caused issues in #85461 where inline
functions were duplicated into multiple CGUs resulting in duplicate
symbols.
This commit fixes the issue by removing the complex logic used to assign
dead code stubs to CGUs and replaces it with a much simplier model: we
pick one CGU to hold all the dead code stubs. We pick a CGU which has
exported items which increases the likelihood the linker won't throw
away our dead functions and we pick the smallest to minimize the impact
on compilation times for crates with very large CGUs.
Fixes#86177Fixes#85718Fixes#79622
Index and hash HIR as part of lowering
Part of https://github.com/rust-lang/rust/pull/88186
~Based on https://github.com/rust-lang/rust/pull/88880 (see merge commit).~
Once HIR is lowered, it is later indexed by the `index_hir` query and hashed for `crate_hash`. This PR moves those post-processing steps to lowering itself. As a side objective, the HIR crate data structure is refactored as an `IndexVec<LocalDefId, Option<OwnerInfo<'hir>>>` where `OwnerInfo` stores all the relevant information for an HIR owner.
r? `@michaelwoerister`
cc `@petrochenkov`
polymorphization: shims and predicates
Supersedes #75737 and #75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with `type_id` and polymorphization to investigate but this should get polymorphization in a reasonable state to work on.
- #75737 and #75414 both worked but were blocked on having the rest of the test suite pass (with polymorphization enabled) with and without the PRs. It makes more sense to just land these so that the changes are in.
- #75737's changes remove the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt.
- #75414's changes remove all logic which marks parameters as used based on their presence in predicates - given #75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped.
- Polymorphization now explicitly checks (and skips) foreign items, this is necessary for stage2 bootstrapping to work when polymorphization is enabled.
- The conditional determining the emission of a note adding context to a post-monomorphization error has been modified. Polymorphization results in `optimized_mir` running for shims during collection where that wouldn't happen previously, some errors are emitted during `optimized_mir` and these were considered post-monomorphization errors with the existing logic (more errors and shims have a `DefId` coming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from using `optimized_mir`, so it seemed more reasonable to not change the conditional.
- `characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷♂️.
r? `@lcnr`
Turn vtable_allocation() into a query
This PR removes the untracked vtable-const-allocation cache from the `tcx` and turns the `vtable_allocation()` method into a query.
The change is pretty straightforward and should be backportable without too much effort.
Fixes https://github.com/rust-lang/rust/issues/89598.
This commit removes the restriction of `InstanceDef::Item` on
polymorphization, so that shims can now be polymorphized.
Signed-off-by: David Wood <david.wood@huawei.com>
Querify `FnAbi::of_{fn_ptr,instance}` as `fn_abi_of_{fn_ptr,instance}`.
*Note: opening this PR as draft because it's based on #88499*
This more or less replicates the `LayoutOf::layout_of` setup from #88499, to replace `FnAbi::of_{fn_ptr,instance}` with `FnAbiOf::fn_abi_of_{fn_ptr,instance}`, and also route them through queries (which `layout_of` has used for a while).
The two changes at the use sites (other than the names) are:
* return type is now wrapped in `&'tcx`
* the value *is* interned, which may affect performance
* the `extra_args` list is now an interned `&'tcx ty::List<Ty<'tcx>>`
* should be cheap (it's empty for anything other than C variadics)
Theoretically, a `FnAbiOfHelpers` implementer could choose to keep the `Result<...>` instead of eagerly erroring, but the only existing users of these APIs are codegen backends, so they don't (want to) take advantage of this.
At least miri could make use of this, since it prefers propagating errors (it "just" doesn't use `FnAbi` yet - cc `@RalfJung).`
The way this is done is probably less efficient than what is possible, because the queries handle the correctness-oriented API (i.e. the split into `fn` pointers vs instances), whereas a lower-level query could end up with more reuse between different instances with identical signatures.
r? `@nagisa` cc `@oli-obk` `@bjorn3`
Gather module items after lowering.
This avoids having a non-local analysis inside lowering.
By implementing `hir_module_items` using a visitor, we make sure that iterations and visitors are consistent.
