Skip to content
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

Rollup of 7 pull requests #129046

Merged
merged 23 commits into from
Aug 13, 2024
Merged

Rollup of 7 pull requests #129046

merged 23 commits into from
Aug 13, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

joboet and others added 23 commits August 4, 2024 18:39
This PR makes a number of changes to the UNIX randomness implementation:
* Use `io::Error` for centralized error handling
* Move the file-fallback logic out of the `getrandom`-specific module
* Stop redefining the syscalls on macOS and DragonFly, they have appeared in `libc`
* Add a `OnceLock` to cache the random device file descriptor
to pick up changes to `./x miri` and `./x test` commands
Moves the CFI ui tests to the cfi directory and removes the cfi prefix
from tests file names similarly to how the cfi codegen tests are
organized.
Refactor `powerpc64` call ABI handling

As the [specification](https://openpowerfoundation.org/specifications/64bitelfabi/) for the ELFv2 ABI states that returned aggregates are returned like arguments as long as they are at most two doublewords, I've merged the `classify_arg` and `classify_ret` functions to reduce code duplication. The only functional change is to fix rust-lang#128579: the `classify_ret` function was incorrectly handling aggregates where `bits > 64 && bits < 128`. I've used the aggregate handling implementation from `classify_arg` which doesn't have this issue.

`@awilfox` could you test this on `powerpc64-unknown-linux-musl`? I'm only able to cross-test on `powerpc64-unknown-linux-gnu` and `powerpc64le-unknown-linux-gnu` locally at the moment, and as a tier 3 target `powerpc64-unknown-linux-musl` has zero CI coverage.

Fixes: rust-lang#128579
…Denton

std: refactor UNIX random data generation

This PR makes a number of changes to the UNIX randomness implementation:
* Use `io::Error` for centralized error handling
* Move the file-fallback logic out of the `getrandom`-specific module
* Stop redefining the syscalls on macOS and DragonFly, they have appeared in `libc`
* Add a `OnceLock` to cache the random device file descriptor
…ubilee

Remove unused lifetime parameter from spawn_unchecked

Amanieu caught this when reviewing the stabilization proposal in rust-lang#55132.

The `'a` lifetime here is useless. The signature is asking the caller of `spawn_unchecked` to "give me any lifetime that is shorter than your F's and T's lifetime", which they can always to with no effect, because arbitrarily short lifetimes exist.
bootstrap: don't use rustflags for `--rustc-args`

r? `@onur-ozkan`

This is going to require a bit of context.

rust-lang#47558 has added `--rustc-args` to `./x test` to allow passing flags when building `compiletest` tests. It was made specifically because using `RUSTFLAGS` would rebuild the compiler/stdlib, which would in turn require the flag you want to build tests with to successfully bootstrap.

rust-lang#113178 made the request that it also works for other tests and doctests. This is not trivial to support across the board for `library`/`compiler` unit-tests/doctests and across stages. This issue was closed in rust-lang#113948 by using `RUSTFLAGS`, seemingly incorrectly since rust-lang#123489 fixed that part to make it work.

Unfortunately rust-lang#123489/rust-lang#113948 have regressed the goals of `--rustc-args`:
- now we can't use rustc args that don't bootstrap, to run the UI tests: we can't test incomplete features. The new trait solver doesn't bootstrap, in-progress borrowck/polonius changes don't bootstrap, some other features are similarly incomplete, etc.
- using the flag now rebuilds everything from scratch: stage0 stdlib, stage1 compiler, stage1 stdlib. You don't need to re-do all this to compile UI tests, you only need the latter to run stdlib tests with a new flag, etc. This happens for contributors, but also on CI today. (Not to mention that in doing that it will rebuild things with flags that are not meant to be used, e.g. stdlib cfgs that don't exist in the compiler; or you could also imagine that this silently enables flags that were not meant to be enabled in this way).

Since then, rust-lang@bd71c71 has started using it to test a stdlib feature, relying on the fact that it now rebuilds everything. So rust-lang#125011 also regressed CI times more than necessary because it rebuilds everything instead of just stage 1 stdlib.

It's not easy for me to know how to properly fix rust-lang#113178 in bootstrap, but rust-lang#113948/rust-lang#123489 are not it since they regress the initial intent. I'd think bootstrap would have to know from the list of test targets that are passed that the `library` or `compiler` paths that are passed could require rebuilding these crates with different rustflags, probably also depending on stages. Therefore I would not be able to fix it, and will just try in this PR to unregress the situation to unblock the initial use-case.

It seems miri now also uses `./x miri --rustc-args` in this incorrect meaning to rebuild the `library` paths they support to run with the new args. I've not made any bootstrap changes related to `./x miri` in this PR, so `--rustc-args` wouldn't work there anymore. I'd assume this would need to use rustflags again but I don't know how to make that work properly in bootstrap, hence opening as draft, so you can tell me how to do that. I assume we don't want to break their use-case again now that it exists, even though there are ways to use `./x test` to do exactly that.

`RUSTFLAGS_NOT_BOOTSTRAP=flag ./x test library/std` is a way to run unit tests with a new flag without rebuilding everything, while with rust-lang#123489 there is no way anymore to run tests with a flag that doesn't bootstrap.

---
edit: after review, this PR:
- renames `./x test --rustc-args` to `./x test --compiletest-rustc-args` as it only applies there, and cannot use rustflags for this purpose.
- fixes the regression that using these args rebuilt everything from scratch
- speeds up some CI jobs via the above point
- removes `./x miri --rustc-args` as only library tests are supported, needs to rebuild libstd, and `./x miri --compiletest-rustc-args` wouldn't work since compiletests are not supported.
Slightly refactor `TargetSelection` in bootstrap

Mostly a drive-by refactoring of `TargetSelection` to reduce some manual "windows-gnu" detection and also accesses to the `triple` field.

r? `@onur-ozkan`
…-to-cfi-directory, r=compiler-errors

CFI: Move CFI ui tests to cfi directory

Move the CFI ui tests to the cfi directory and removes the cfi prefix from tests file names similarly to how the cfi codegen tests are organized.
Fix blessing of rmake tests

Fixes rust-lang#129038.

When running in `--bless` mode, we now set the value of `RUSTC_BLESS_TEST` to the current test's source directory. This allows the diff helper in `run_make_support` to find the original snapshot file in the source directory and bless that, instead of unhelpfully blessing the temporary copy in `build`.

r? `@jieyouxu`
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc O-unix Operating system: Unix-like PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 13, 2024
@rustbot rustbot added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 13, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Collaborator

bors commented Aug 13, 2024

📌 Commit 42f70c2 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2024
@bors
Copy link
Collaborator

bors commented Aug 13, 2024

⌛ Testing commit 42f70c2 with merge 80eb5a8...

@bors
Copy link
Collaborator

bors commented Aug 13, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 80eb5a8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 13, 2024
@bors bors merged commit 80eb5a8 into rust-lang:master Aug 13, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 13, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#128643 Refactor powerpc64 call ABI handling 5d8e580f06eb307457c769feee7933f6521292cc (link)
#128655 std: refactor UNIX random data generation c84c15873d7facf7da5de4110efa3313d368d6bb (link)
#128745 Remove unused lifetime parameter from spawn_unchecked fbd64e29ff5466ed14e50fa31a0c766e1add1bce (link)
#128841 bootstrap: don't use rustflags for --rustc-args e984d91736c8842835eaedddac172059c359a611 (link)
#128983 Slightly refactor TargetSelection in bootstrap 6ff854b3e9a602be7a772dd9bccd31916121921e (link)
#129026 CFI: Move CFI ui tests to cfi directory 652cd0e0f2f5a6c1cdd22ef84cad1c6822d436a2 (link)
#129040 Fix blessing of rmake tests 7513ebef32889f7c20f9b9682c13e52d86a1d97f (link)

previous master: a2e1d154d5

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (80eb5a8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -1.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.2% [-1.2%, -1.2%] 1
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.0%, secondary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.1%] 12
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 35
Improvements ✅
(primary)
-0.1% [-0.1%, -0.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [-0.1%, 0.1%] 13

Bootstrap: 753.626s -> 752.976s (-0.09%)
Artifact size: 341.40 MiB -> 341.40 MiB (0.00%)

@matthiaskrgr matthiaskrgr deleted the rollup-9x4xgak branch September 1, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. O-unix Operating system: Unix-like PG-exploit-mitigations Project group: Exploit mitigations rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.