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 5 pull requests #136754

Merged
merged 18 commits into from
Feb 9, 2025
Merged

Rollup of 5 pull requests #136754

merged 18 commits into from
Feb 9, 2025

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 8, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

ChrisDenton and others added 18 commits January 26, 2025 05:42
This allows Rust on Fuchsia to use a number of function calls from libc:

* dirfd
* fdatasync
* flock with LOCK_EX, LOCK_SH, LOCK_NB, LOCK_UN
* fstatat
Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- from observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors
- Debuggers need to be able to handle a variety of versions withing the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
updates the test added in the previous commit to ensure no warning is
printed.
…mulacrum

Windows: remove readonly files

When calling `remove_file`, we shouldn't fail to delete readonly files. As the test makes clear, this make the Windows behaviour consistent with other platforms. This also makes us internally consistent with `remove_dir_all`.

try-job: x86_64-msvc-ext1
Allow Rust to use a number of libc filesystem calls

This allows Rust on Fuchsia to use a number of function calls from libc:

* dirfd
* fdatasync
* flock with LOCK_EX, LOCK_SH, LOCK_NB, LOCK_UN
* fstatat

cc rust-lang#120426

try-job: dist-various-2
Implement `x perf` directly in bootstrap

Discussed [here](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Turning.20.60x.20perf.60.20into.20a.20first.20class.20command).

Implementing the command directly in bootstrap let's us correctly build the compiler toolchain based on input arguments (such as include rustdoc in the toolchain [only] when needed), and it also makes the CLI interface nicer.

r? ``@onur-ozkan``
…=saethlin

Detect (non-raw) borrows of null ZST pointers in CheckNull

Fixes rust-lang#136568. Ensures that we check that borrows of derefs are non-null in the `CheckNull` pass **even if** it's a ZST pointee.

I'm actually surprised that this is UB in Miri, but if it's certainly UB, then this PR modifies the null check to be stricter. I couldn't find anywhere in https://doc.rust-lang.org/reference/behavior-considered-undefined.html that discusses this case specifically, but I didn't read it too closely, or perhaps it's just missing a bullet point.

On the contrary, if this is actually erroneous UB in Miri, then I'm happy to close this (and perhaps fix the null check in Miri to exclude ZSTs?)

On the double contrary, if this is still an "open question", I'm also happy to close this and wait for a decision to be made.

r? ``@saethlin`` cc ``@RalfJung`` (perhaps you feel strongly about this change)
…e_behavior, r=jieyouxu

Pick the max DWARF version when LTO'ing modules with different versions

Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- From observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning.
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors.
- Debuggers need to be able to handle a variety of versions within the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions.

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
adds a test to ensure no warning is printed.

Fixes rust-lang#130041 which fails due to these warnings being printed

cc rust-lang#103057
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustc-dev-guide Area: rustc-dev-guide O-unix Operating system: Unix-like O-windows Operating system: Windows 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Feb 8, 2025
@Urgau
Copy link
Member Author

Urgau commented Feb 8, 2025

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Feb 8, 2025

📌 Commit 5ec56e5 has been approved by Urgau

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 8, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#136213 (Allow Rust to use a number of libc filesystem calls)
 - rust-lang#136530 (Implement `x perf` directly in bootstrap)
 - rust-lang#136601 (Detect (non-raw) borrows of null ZST pointers in CheckNull)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Testing commit 5ec56e5 with merge 0edbadb...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Feb 9, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 9, 2025
@workingjubilee
Copy link
Member

praying for spurious?

@bors retry

@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 Feb 9, 2025
@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Testing commit 5ec56e5 with merge cd6ca8e2b30c7f8a586e1cc51604d20be6ee55fd...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#136213 (Allow Rust to use a number of libc filesystem calls)
 - rust-lang#136530 (Implement `x perf` directly in bootstrap)
 - rust-lang#136601 (Detect (non-raw) borrows of null ZST pointers in CheckNull)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 9, 2025

💥 Test timed out

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

The job x86_64-apple-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@Urgau
Copy link
Member Author

Urgau commented Feb 9, 2025

Timeout at:

test [debuginfo-lldb] tests/debuginfo/destructured-for-loop-variable.rs has been running for a long time

does not seems to be related to any PRs here. Maybe also spurious. Let's try again.

@bors retry

@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 Feb 9, 2025
@matthiaskrgr
Copy link
Member

could be related to #136659
note that dwarf is a debug info format

@Urgau
Copy link
Member Author

Urgau commented Feb 9, 2025

sure, but the test passed on the other runners, so I have a hard time believing it failed bc of the PR, especially bc it's a timeout and not a failed matching

@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Testing commit 5ec56e5 with merge a26e97be8826d408309fffbd8168362365719f50...

@bors
Copy link
Contributor

bors commented Feb 9, 2025

☀️ Test successful - checks-actions
Approved by: Urgau
Pushing a26e97b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 9, 2025
@bors bors merged commit a26e97b into rust-lang:master Feb 9, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 9, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#134679 Windows: remove readonly files 2257b070afbb490c8e369e7481fbf1e361141ba3 (link)
#136213 Allow Rust to use a number of libc filesystem calls e95ddc702752372a5dfd9e6584fd37965aae268e (link)
#136530 Implement x perf directly in bootstrap fee0291f08e35b727955a9f1421e7d58d6183cd5 (link)
#136601 Detect (non-raw) borrows of null ZST pointers in CheckNull 3747e701bf4e40869c2132a302f2e18503049f02 (link)
#136659 Pick the max DWARF version when LTO'ing modules with differ… 7919ff805a7e0cfd7c1c9f964d14869b361b217a (link)

previous master: 1ff21350fd

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 (a26e97b): 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 (primary -1.9%, secondary -2.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)
2.5% [2.1%, 3.0%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.9% [-9.6%, -2.0%] 3
Improvements ✅
(secondary)
-2.0% [-2.0%, -2.0%] 1
All ❌✅ (primary) -1.9% [-9.6%, 3.0%] 5

Cycles

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

Binary size

Results (primary -0.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.2% [0.1%, 0.4%] 5
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.2% [-0.4%, -0.0%] 13
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.4%, 0.4%] 18

Bootstrap: 781.536s -> 777.631s (-0.50%)
Artifact size: 329.13 MiB -> 329.11 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustc-dev-guide Area: rustc-dev-guide merged-by-bors This PR was explicitly merged by bors. O-unix Operating system: Unix-like O-windows Operating system: Windows 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-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.