Insert checks for enum discriminants when debug assertions are enabled
Similar to the existing null-pointer and alignment checks, this checks for valid enum discriminants on creation of enums through unsafe transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```
An extension of this check will be done in a follow-up that explicitly sanitizes for extern enum values that come into Rust from e.g. C/C++.
This check is similar to Miri's capabilities of checking for valid construction of enum values.
This PR is inspired by saethlin@'s PR
https://github.com/rust-lang/rust/pull/104862. Thank you so much for keeping this code up and the detailed comments!
I also pair-programmed large parts of this together with vabr-g@.
r? `@saethlin`
Similar to the existing nullpointer and alignment checks, this checks
for valid enum discriminants on creation of enums through unsafe
transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```
An extension of this check will be done in a follow-up that explicitly
sanitizes for extern enum values that come into Rust from e.g. C/C++.
This check is similar to Miri's capabilities of checking for valid
construction of enum values.
This PR is inspired by saethlin@'s PR
https://github.com/rust-lang/rust/pull/104862. Thank you so much for
keeping this code up and the detailed comments!
I also pair-programmed large parts of this together with vabr-g@.
Remove dead instructions in terminate blocks
Terminate blocks look pretty in the IR I've looked at, so no actual perf delta from this. But it seems reasonable to note produce unused IR.
Port `#[track_caller]` to the new attribute system
r? ``@oli-obk``
depends on https://github.com/rust-lang/rust/pull/142493Closesrust-lang/rust#142783
(didn't add a test for this, this situation should simply never come up again, the code was simply wrong. lmk if I should add it, but it won't test something very useful)
Avoid a bitcast FFI call in transmuting
For things that only change the valid ranges, we can just return the input, rather than making the `LLVMBuildBitCast` call and having *it* then do nothing.
I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
Rollup of 9 pull requests
Successful merges:
- rust-lang/rust#142645 (Also emit suggestions for usages in the `non_upper_case_globals` lint)
- rust-lang/rust#142657 (mbe: Clean up code with non-optional `NonterminalKind`)
- rust-lang/rust#142799 (rustc_session: Add a structure for keeping both explicit and default sysroots)
- rust-lang/rust#142805 (Emit a single error when importing a path with `_`)
- rust-lang/rust#142882 (Lazy init diagnostics-only local_names in borrowck)
- rust-lang/rust#142883 (Add impl_trait_in_bindings tests from rust-lang/rust#61773)
- rust-lang/rust#142943 (Don't include current rustc version string in feature removed help)
- rust-lang/rust#142965 ([RTE-497] Ignore `c-link-to-rust-va-list-fn` test on SGX platform)
- rust-lang/rust#142972 (Add a missing mailmap entry)
r? `@ghost`
`@rustbot` modify labels: rollup
rustc_std_internal_symbol is meant to call functions from crates where
there is no direct dependency on said crate. As they either have to be
added to symbols.o or rustc has to introduce an implicit dependency on
them to avoid linker errors. The latter is done for some things like the
panic runtime, but adding these symbols to symbols.o allows removing
those implicit dependencies.
[win][aarch64] Fix linking statics on Arm64EC, take 2
Arm64EC builds recently started to fail due to the linker not finding a symbol:
```
symbols.o : error LNK2001: unresolved external symbol #_ZN3std9panicking11EMPTY_PANIC17hc8d2b903527827f1E (EC Symbol)
C:\Code\hello-world\target\arm64ec-pc-windows-msvc\debug\deps\hello_world.exe : fatal error LNK1120: 1 unresolved externals
```
It turns out that `EMPTY_PANIC` is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with `#` (as only functions are prefixed with this character), whereas Rust was prefixing with `#` when attempting to import it.
The fix is to have Rust not prefix statics with `#` when importing.
Adding tests discovered another issue: we need to correctly mark static exported from dylibs with `DATA`, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them.
Fixesrust-lang/rust#138541
Resurrects rust-lang/rust#140176 now that rust-lang/rust#141061 is merged, which removes the incompatibility with `__rust_no_alloc_shim_is_unstable`.
r? ``@wesleywiser``
CC ``@bjorn3``
fix `-Zmin-function-alignment` on functions without attributes
tracking issue: https://github.com/rust-lang/rust/issues/82232
related: https://github.com/rust-lang/rust/pull/142854
The minimum function alignment was skipped on functions without attributes (because the logic was in a loop that only runs if there is at least one attribute). The underlying reason we didn't catch this before is that in our testing we generally apply `#[no_mangle]` to functions that are tested. I've added a test now that deliberately has no attributes.
r? `@workingjubilee`
the minimum function alignment was skipped on functions without attributes. That is because in our testing we generally apply `#[no_mangle]` to functions that are tested. I've added a test now that deliberately has no attributes
centralize `-Zmin-function-alignment` logic
tracking issue: https://github.com/rust-lang/rust/issues/82232
discussed in: https://github.com/rust-lang/rust/pull/142824#discussion_r2160056244
Apply the `-Zmin-function-alignment` value to the alignment field of the function attributes when those are created, so that individual backends don't need to consider it.
The one exception right now is cranelift, because it can't yet set the alignment for individual functions, but it can (and does) set the global minimum function alignment.
cc ``@RalfJung`` I think this is an improvement regardless, is there anything else that should be done for miri?
Refactor Translator
My main motivation was to simplify the usage of `SilentEmitter` for users like rustfmt. A few refactoring opportunities arose along the way.
* Replace `Translate` trait with `Translator` struct
* Replace `Emitter: Translate` with `Emitter::translator`
* Split `SilentEmitter` into `FatalOnlyEmitter` and `SilentEmitter`
Extract some shared code from codegen backend target feature handling
There's a bunch of code duplication between the GCC and LLVM backends in target feature handling. This moves that into new shared helper functions in `rustc_codegen_ssa`.
The first two commits should be purely refactoring. I am fairly sure the LLVM-side behavior stays the same; if the GCC side deliberately diverges from this then I may have missed that. I did account for one divergence, which I do not know is deliberate or not: GCC does not seem to use the `-Ctarget-feature` flag to populate `cfg(target_feature)`. That seems odd, since the `-Ctarget-feature` flag is used to populate the return value of `global_gcc_features` which controls the target features actually used by GCC. ``@GuillaumeGomez`` ``@antoyo`` is there a reason `target_config` ignores `-Ctarget-feature` but `global_gcc_features` does not? The second commit also cleans up a bunch of unneeded complexity added in https://github.com/rust-lang/rust/pull/135927.
The third commit extracts some shared logic out of the functions that populate `cfg(target_feature)` and the backend target feature set, respectively. This one actually has some slight functional changes:
- Before, with `-Ctarget-feature=-feat`, if there is some other feature `x` that implies `feat` we would *not* add `-x` to the backend target feature set. Now, we do. This fixesrust-lang/rust#134792.
- The logic that removes `x` from `cfg(target_feature)` in this case also changed a bit, avoiding a large number of calls to the (uncached) `sess.target.implied_target_features` (if there were a large number of positive features listed before a negative feature) but instead constructing a full inverse implication map when encountering the first negative feature. Ideally this would be done with queries but the backend target feature logic runs before `tcx` so we can't use that...
- Previously, if feature "a" implied "b" and "b" was unstable, then using `-Ctarget-feature=+a` would also emit a warning about `b`. I had to remove this since when accounting for negative implications, this emits a ton of warnings in a bunch of existing tests... I assume this was unintentional anyway.
The fourth commit increases consistency of the GCC backend with the LLVM backend.
The last commit does some further cleanup:
- Get rid of RUSTC_SPECIAL_FEATURES. It was only needed for s390x "backchain", but since LLVM 19 that is always a regular target feature so we don't need this hack any more. The hack also has various unintended side-effects so we don't want to keep it. Fixes https://github.com/rust-lang/rust/issues/142412.
- Move RUSTC_SPECIFIC_FEATURES handling into the shared parse_rust_feature_flag helper so all consumers of `-Ctarget-feature` that only care about actual target features (and not "crt-static") have it. Previously, we actually set `cfg(target_feature = "crt-static")` twice: once in the backend target feature logic, and once specifically for that one feature. IIUC, some targets are meant to ignore `-Ctarget-feature=+crt-static`, it seems like before this PR that flag still incorrectly enabled `cfg(target_feature = "crt-static")` (but I didn't test this).
- Move fixed_x18 handling together with retpoline handling.
- Forbid setting fixed_x18 as a regular target feature, even unstably. It must be set via the `-Z` flag.
``@bjorn3`` I did not touch the cranelift backend here, since AFAIK it doesn't really support target features. But if you ever do, please use the new helpers. :)
Cc ``@workingjubilee``
rewrite `optimize` attribute to use new attribute parsing infrastructure
r? ```@oli-obk```
I'm afraid we'll get quite a few of these PRs in the future. If we get a lot of trivial changes I'll start merging multiple into one PR. They should be easy to review :)
Waiting on #138165 first
For things that only change the valid ranges, we can just skip the `LLVMBuildBitCast` call.
I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
This does change the logic a bit: previously, we didn't forward reverse
implications of negated features to the backend, instead relying on the backend
to handle the implication itself.
use `#[align]` attribute for `fn_align`
Tracking issue: https://github.com/rust-lang/rust/issues/82232https://github.com/rust-lang/rfcs/pull/3806 decides to add the `#[align]` attribute for alignment of various items. Right now it's used for functions with `fn_align`, in the future it will get more uses (statics, struct fields, etc.)
(the RFC finishes FCP today)
r? `@ghost`
CodeGen: rework Aggregate implemention for rvalue_creates_operand cases
A non-trivial refactor pulled out from rust-lang/rust#138759
r? workingjubilee
The previous implementation I'd written here based on `index_by_increasing_offset` is complicated to follow and difficult to extend to non-structs.
This changes the implementation, without actually changing any codegen (thus no test changes either), to be more like the existing `extract_field` (<2b0274c71d/compiler/rustc_codegen_ssa/src/mir/operand.rs (L345-L425)>) in that it allows setting a particular field directly.
Notably I've found this one much easier to get right, in particular because having the `OperandRef<Result<V, Scalar>>` gives a really useful thing to include in ICE messages if something did happen to go wrong.
Change __rust_no_alloc_shim_is_unstable to be a function
This fixes a long sequence of issues:
1. A customer reported that building for Arm64EC was broken: #138541
2. This was caused by a bug in my original implementation of Arm64EC support, namely that only functions on Arm64EC need to be decorated with `#` but Rust was decorating statics as well.
3. Once I corrected Rust to only decorate functions, I started linking failures where the linker couldn't find statics exported by dylib dependencies. This was caused by the compiler not marking exported statics in the generated DEF file with `DATA`, thus they were being exported as functions not data.
4. Once I corrected the way that the DEF files were being emitted, the linker started failing saying that it couldn't find `__rust_no_alloc_shim_is_unstable`. This is because the MSVC linker requires the declarations of statics imported from other dylibs to be marked with `dllimport` (whereas it will happily link to functions imported from other dylibs whether they are marked `dllimport` or not).
5. I then made a change to ensure that `__rust_no_alloc_shim_is_unstable` was marked as `dllimport`, but the MSVC linker started emitting warnings that `__rust_no_alloc_shim_is_unstable` was marked as `dllimport` but was declared in an obj file. This is a harmless warning which is a performance hint: anything that's marked `dllimport` must be indirected via an `__imp` symbol so I added a linker arg in the target to suppress the warning.
6. A customer then reported a similar warning when using `lld-link` (<https://github.com/rust-lang/rust/pull/140176#issuecomment-2872448443>). I don't think it was an implementation difference between the two linkers but rather that, depending on the obj that the declaration versus uses of `__rust_no_alloc_shim_is_unstable` landed in we would get different warnings, so I suppressed that warning as well: #140954.
7. Another customer reported that they weren't using the Rust compiler to invoke the linker, thus these warnings were breaking their build: <https://github.com/rust-lang/rust/pull/140176#issuecomment-2881867433>. At that point, my original change was reverted (#141024) leaving Arm64EC broken yet again.
Taking a step back, a lot of these linker issues arise from the fact that `__rust_no_alloc_shim_is_unstable` is marked as `extern "Rust"` in the standard library and, therefore, assumed to be a foreign item from a different crate BUT the Rust compiler may choose to generate it either in the current crate, some other crate that will be statically linked in OR some other crate that will by dynamically imported.
Worse yet, it is impossible while building a given crate to know if `__rust_no_alloc_shim_is_unstable` will statically linked or dynamically imported: it might be that one of its dependent crates is the one with an allocator kind set and thus that crate (which is compiled later) will decide depending if it has any dylib dependencies or not to import `__rust_no_alloc_shim_is_unstable` or generate it. Thus, there is no way to know if the declaration of `__rust_no_alloc_shim_is_unstable` should be marked with `dllimport` or not.
There is a simple fix for all this: there is no reason `__rust_no_alloc_shim_is_unstable` must be a static. It needs to be some symbol that must be linked in; thus, it could easily be a function instead. As a function, there is no need to mark it as `dllimport` when dynamically imported which avoids the entire mess above.
There may be a perf hit for changing the `volatile load` to be a `tail call`, so I'm happy to change that part back (although I question what the codegen of a `volatile load` would look like, and if the backend is going to try to use load-acquire semantics).
Build with this change applied BEFORE #140176 was reverted to demonstrate that there are no linking issues with either MSVC or MinGW: <https://github.com/rust-lang/rust/actions/runs/15078657205>
Incidentally, I fixed `tests/run-make/no-alloc-shim` to work with MSVC as I needed it to be able to test locally (FYI for #128602)
r? `@bjorn3`
cc `@jieyouxu`
Another refactor pulled out from 138759
The previous implementation I'd written here based on `index_by_increasing_offset` is complicated to follow and difficult to extend to non-structs.
This changes the implementation, without actually changing any codegen (thus no test changes either), to be more like the existing `extract_field` (<2b0274c71d/compiler/rustc_codegen_ssa/src/mir/operand.rs (L345-L425)>) in that it allows setting a particular field directly.
Notably I've found this one much easier to get right, in particular because having the `OperandRef<Result<V, Scalar>>` gives a really useful thing to include in ICE messages if something did happen to go wrong.
