Fix PowerPC 32-bit ABI: coerce small aggregates to integers in GPRs#502
Open
SebTardif wants to merge 161 commits into
Open
Fix PowerPC 32-bit ABI: coerce small aggregates to integers in GPRs#502SebTardif wants to merge 161 commits into
SebTardif wants to merge 161 commits into
Conversation
The current Linux driver treats every sendmsg call as a separate record. That is, it behaves as though MSG_EOR is always set. But that might not be true forever. And the current FreeBSD driver does _not_ do that, so setting MSG_EOR is important for portability.
* Handle generic reborrow in expression-use adjustment walking * Require generic reborrow to be terminal in adjustment walks
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
Co-authored-by: Kevin Reid <kpreid@switchb.org>
Co-authored-by: Kevin Reid <kpreid@switchb.org>
LLVM 23 recently changed SimplifyCFG to avoid integer lookup tables, and that perturbed these two tests in ways that look harmless to me.
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
When a `for` loop is missing its `in`, the parser suggested inserting one based only on the token after the pattern. A malformed binding such as `for i i in 0..10` was therefore "corrected" to `for i in i in 0..10`, which does not parse. Only suggest inserting `in` when the loop header does not already contain one before the body.
Document the built-in `must_use` attribute in the standard library using the `#[doc(attribute = "...")]` mechanism, following the existing `keyword_docs.rs` pattern.
When passing multiple `-Zsanitizer` flags to the compiler (e.g., `-Zsanitizer=address -Zsanitizer=shadow-call-stack`), the options parser was overwriting the previous values in `target_modifiers` instead of accumulating them. This resulted in only the last sanitizer being recorded in the crate metadata's target modifiers, even though the frontend correctly enabled all of them. The only way to provide multiple sanitizers was to combine them into a comma-separated list passed to a single `-Zsanitizer=` flag, but this does not fit well into the GN build system where different targets may pass different combinations of sanitizer flags. Consequently, this caused spurious "incompatible target modifiers" ABI mismatch errors when linking against dependencies compiled with accumulated flags (e.g., `-Zsanitizer=address,shadow-call-stack`). Fix this by entry-modifying the `target_modifiers` map to accumulate the sanitizers as a comma-separated list when the option is `sanitizer`. Also add a codegen test verifying that both CFI and SafeStack attributes/metadata are present when enabled together via multiple flags. Test: ./x.py test tests/codegen-llvm/sanitizer/multiple-sanitizers.rs
To get a `LintVec` for a lint pass we sometimes use `get_lints` and sometimes use `lint_vec`. It would be nice to only have one, but doing that is tricky. In the meantime, this commit makes the naming more consistent. - By always using the name `get_lints` for the methods that take `self`. - By always using the name `lint_vec` for the methods that have no parameters.
These are both types that impl `LintPass` but in a degenerate way: only the `lint_vec` method is ever used. This commit changes them to just be a `lint_vec` function in an appropriately-named module. The commit also removes the use of `HardwiredLints` in `late_lint_crate`, which had no effect because all the `HardwiredLints::check_*` methods were no-ops.
…=jhpratt `impl [const] Default for BTreeMap` Tracking issue: rust-lang#143894.
Codegen ctors in Runtime mir phase https://github.com/rust-lang/rust/pull/156141/changes#diff-f18405dedc545b19aa3ee04cd08b17e1e0fa1b5876e46c6b445eaaa7e54618eaR421 (part of rust-lang#156141) The mir of constructors used to be generated in MirPhase::Built, but are only used during codegen. That means they'd have a weird MirPhase, since all other items have MirPhase::Runtime during codegen. This PR still generates ctor mir in MirPhase::Built, but then immediately does a phase change to Runtime after running no passes. Fixes rust-lang#158037 <summary> Old description of another, worse way to solve this <details> starts using `TypingMode::PostTypeckUntilBorrowck` during mir building with the next solver. This is the right typing mode, and the typing mode contains a list of opaque types. However, previously, we instead just used `TypingMode::Typeck` with an empty list of opaques. That is the wrong typing mode, but it also meant we collected opaque types of slightly fewer things. Now that we do collect their opaque types, we're doing so for items that we've never gathered opaque types for before and get an ICE. There are no tests associated with this change. @lqd tried to reproduce it, but it seems surprisingly hard except by bootstrapping rustc itself. This change makes `RUSTFLAGS_NOT_BOOTSTRAP=-Znext-solver x build compiler --stage 2` work, which I verified locally. See [#t-infra/announcements > Next solver / polonius alpha pre-stabilization CI job](https://rust-lang.zulipchat.com/#narrow/channel/533458-t-infra.2Fannouncements/topic/Next.20solver.20.2F.20polonius.20alpha.20pre-stabilization.20CI.20job/with/602503884) for work towards making bootstraps with the next solver a CI job. </details> </summary> r? @oli-obk
…ss35 Stabilize `substr_range` and `subslice_range` Closes rust-lang#126769 [Stabilization Report](rust-lang#126769 (comment))
…o-enum, r=Urgau Change `EarlyCheckNode` from a trait to an enum. It only has two impls, both of which are tuples, which is ugly. An enum is much simpler and clearer. Also, `EarlyContextAndPass` doesn't need to be public. r? @Urgau
…, r=Kobzol Convert '.' to '_' in bootstrap envify This means you can now use remote-test-client/remote-test-server with targets that contain a '.' in the name. See rust-lang#158090 r? @Kobzol
…nathanBrouwer Rollup of 5 pull requests Successful merges: - rust-lang#157878 (`impl [const] Default for BTreeMap`) - rust-lang#158040 (Codegen ctors in Runtime mir phase) - rust-lang#141266 (Stabilize `substr_range` and `subslice_range`) - rust-lang#158109 (Change `EarlyCheckNode` from a trait to an enum.) - rust-lang#158118 (Convert '.' to '_' in bootstrap envify)
…o, r=JonathanBrouwer Revert "Add expansion info to implied bounds" This reverts PR rust-lang#157702 because it causes an ICE and a significant perf regression. This PR also adds a regression test for the ICE. r? ghost cc @oli-obk @estebank @davidtwco Fixes rust-lang#158093
remove `check_loop_match` from `check_attr.rs` as it only verifies that the target is a loop, which is already enforced by `check_target` in `target_checking.rs` via `ALLOWED_TARGETS`, and its `LoopMatchAttr` diagnostic.
remove `check_const_continue` from `check_attr.rs` as it only verifies that the target is a break, which is already enforced by `check_target` in `target_checking.rs` via `ALLOWED_TARGETS`, and it's's `ConstContinueAttr` diagnostic.
ensure the new solver bootstraps on CI With rust-lang#158040, the new solver should now be able to bootstrap, and this PR ensures on CI it doesn't regress. This is part of the plan in rust-lang#157780 until stabilization. This renames the job as well to something hopefully clearer, from `x86_64-gnu-pre-stabilization` to `x86_64-gnu-next-trait-solver-polonius`. And thus, also needs rust-lang/rustc-dev-guide#2904 to have the correct `doc_url`. r? kobzol
…thanBrouwer Rename `lint-rust-version` to `hint-msrv` As suggested here: rust-lang#157574 (comment). I agree that this version is more clear about semantics, and reflects the consensus of the previous MCP. Tracking issue: rust-lang#157574
…enkov
Implement `#[diagnostic::on_unknown]` for modules.
This lets you customize the error message if an item can't be found in a module. Primarily my motivation for this is that I was trying to dogfood `diagnostic::on_unknown` in my own crates and realized I was writing a lot of duplicate attributes and it'd be simpler if it just applied to things at a module level.
For example:
```rust
#[diagnostic::on_unknown(message = "oh no, `{Unresolved}` is not in module `{This}`")]
pub mod x {
pub struct Foo;
}
pub use x::Bar;
```
```
error[E0432]: oh no, `Bar` is not in module `x`
--> $DIR/on_module.rs:9:9
|
LL | pub use x::Bar;
| ^^^---
| |
| no `Bar` in `x`
|
= note: unresolved import `x::Bar`
```
Also, as shown, this implements `{Unresolved}` to reference whatever couldn't be found.
Inner attribute syntax (`#![diagnostic::on_unknown]`) is supported, however it (still) is an error to use inner tool attributes without `#![feature(custom_inner_attributes)]`.
cc @estebank @weiznich
Point to the unstable segment of an import path instead of to the whole path Brought to you by me being confused why a stable item reported an instability error.
`-Znext-solver` Emit error instead of ICE when combining {int, float} var with alias
Fixes rust-lang#158064
r? lcnr
…risDenton std: use correct low surrogate range in Windows standard I/O code #377 is an LLM-generated issue where the LLM managed to find a wrong constant in `std`. Low surrogates do indeed start at `0xDC00`, not `0xDCEE`. This is kind of hard to test for, maybe you have ideas Chris? r? @ChrisDenton
…isDenton std: correctly report file size on UWP Found by an LLM in #397. UWP uses [`GetFileInformationByHandleEx`](https://learn.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-getfileinformationbyhandleex) instead of [GetFileInformationByHandle](https://learn.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getfileinformationbyhandle), but uses the wrong field to determine file size – `EndOfFile` is a different field than `AllocationSize`. [`file_size`](https://doc.rust-lang.org/nightly/std/os/windows/fs/trait.MetadataExt.html#tymethod.file_size) should report the length of the file, not its size on disk, and only `EndOfFile` reports the former (c.f. https://learn.microsoft.com/en-us/windows/win32/api/FileAPI/nf-fileapi-setendoffile#remarks): > Each file stream has the following: > * File size: the size of the data in a file, to the byte. > * Allocation size: the size of the space that is allocated for a file on a disk, which is always an even multiple of the cluster size. > * Valid data length: the length of the data in a file that is actually written, to the byte. This value is always less than or equal to the file size. r? @ChrisDenton
…athanBrouwer Remove redundant check for `#[loop_match]` and `#[const_continue]` Updates rust-lang#153101 remove `check_loop_match` and `check_const_continue` from `check_attr.rs` as they only verifies that the target, which is already enforced by `check_target` in `target_checking.rs` via `ALLOWED_TARGETS`. r? @JonathanBrouwer
…nathanBrouwer Rollup of 8 pull requests Successful merges: - rust-lang#158129 (ensure the new solver bootstraps on CI) - rust-lang#158134 (Rename `lint-rust-version` to `hint-msrv`) - rust-lang#157926 (Implement `#[diagnostic::on_unknown]` for modules.) - rust-lang#158075 (Point to the unstable segment of an import path instead of to the whole path) - rust-lang#158084 (`-Znext-solver` Emit error instead of ICE when combining {int, float} var with alias) - rust-lang#158128 (std: use correct low surrogate range in Windows standard I/O code) - rust-lang#158132 (std: correctly report file size on UWP) - rust-lang#158138 (Remove redundant check for `#[loop_match]` and `#[const_continue]`)
All aggregates were unconditionally passed indirectly, but the PPC32 SVR4 ABI passes small structs (<= 8 bytes) by value in GPRs, coerced to integer types. This caused ABI mismatch with C compilers for extern "C" functions with small struct arguments. Small aggregates are now coerced to their integer equivalents: 1 byte -> i8 (single GPR) 2 bytes -> i16 (single GPR) 3-4 bytes -> i32 (single GPR) 5-8 bytes -> i64 (GPR pair) >8 bytes -> indirect (unchanged) Fixes #500 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
SebTardif
added
bug
2 tasks
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
20 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.
Summary
On PowerPC 32-bit (Linux), ALL aggregates were passed indirectly via pointer, regardless of size. The SVR4 ABI requires small structs (8 bytes or less) to be passed by value in GPRs.
Fixes #500
Root Cause
The
|| arg.layout.is_aggregate()causes ALL aggregates to go indirect.Fix
Separate the two conditions. For aggregates, coerce small ones to integer register types per the SVR4 ABI:
Reg::i8()Reg::i16()Reg::i32()Reg::i64()(GPR pair)Impact
PowerPC 32-bit Linux is a tier 3 target, but this fixes a genuine ABI mismatch with C compilers for
extern "C"functions with small struct arguments.