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 4 pull requests #131138

Closed
wants to merge 19 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJung and others added 19 commits September 30, 2024 08:37
This reverts commit 5a7058c.

In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually
**load-bearing** [1] for (at least) `rustc` and `rustdoc` to have the
kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot |
false` will ICE and `rustdoc --print=sysroot | false` will panic on a
broken pipe.

[rust-lang#131059]: rust-lang#131059
[1]: rust-lang#131059 (comment)
…yukang

Add unstable support for outputting file checksums for use in cargo

Adds an unstable option that appends file checksums and expected lengths to the end of the dep-info file such that `cargo` can read and use these values as an alternative to file mtimes.

This PR powers the changes made in this cargo PR rust-lang/cargo#14137

Here's the tracking issue for the cargo feature rust-lang/cargo#14136.
…li-obk

panic when an interpreter error gets unintentionally discarded

One important invariant of Miri is that when an interpreter error is raised (*in particular* a UB error), those must not be discarded: it's not okay to just check `foo().is_err()` and then continue executing.

This seems to catch new contributors by surprise fairly regularly, so this PR tries to make it so that *if* this ever happens, we get a panic rather than a silent missed UB bug. The interpreter error type now contains a "guard" that panics on drop, and that is explicitly passed to `mem::forget` when an error is deliberately discarded.

Fixes rust-lang/miri#3855
…r-ozkan

Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags"

In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for
(at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc
--print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe.

This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future
fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the
`-Zon-broken-pipe=kill` flag for rustc binary builds.

I could not figure out how to write a regression test for the `rustc --print=sysroot | false`
behavior on Unix, so this is a plain revert for now.

This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach.

See more details at <rust-lang#131059 (comment)> and in the timeline below.

### Timeline of kill-process-on-broken-pipe behavior changes

See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling.

- From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if
  output pipe is broken yet the program tries to use `println!` and such, there will be a broken
  pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376].
- [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a
  sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if
  `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses
  `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`.
- A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `#
  [unix_sigpipe = "sig_dfl"]` attribute.
- [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of
  `rustc_driver::set_sigpipe_handler`.
- [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of
  `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed.
- Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior
  was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag
  `-Zon-broken-pipe=kill`.
    - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main
      binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe
      behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and
      `rustc` during compile steps.
- [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri
  tests because at the time the PR was written, this would lead to a couple of cargo test failures.
  Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo`
  without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool
  build cache. This is not a problem at the time, because nothing (not even miri) tests built stage
  1 cargo (all used initial cargo).
- In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not
  just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but
  absent in beta cargo, which was blocking a beta backport.
- [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since
  `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache
  invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing
  `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980].

[rust-lang#34376]: rust-lang#34376
[rust-lang#49606]: rust-lang#49606
[rust-lang#97889]: rust-lang#97889
[rust-lang#102587]: rust-lang#102587
[rust-lang#103495]: rust-lang#103495
[rust-lang#124480]: rust-lang#124480
[rust-lang#130634]: rust-lang#130634
[rust-lang#130642]: rust-lang#130642
[rust-lang#130739]: rust-lang#130739
[rust-lang#130980]: rust-lang#130980
[rust-lang#131059]: rust-lang#131059
[^1]: rust-lang#131059 (comment)

r? `@onur-ozkan` (or bootstrap)
A couple of fixes for dataflow graphviz dumps

A couple of trivial drive-by fixes to issues I noticed while debugging my buggy borrowck code:

One is a fix of the `-Zdump-mir-dataflow` file extensions, the dataflow graphviz files are currently dumped  as `..dot`.

<details>

```console
-rw-rw-r-- 1 lqd lqd 13051 Oct  1 23:21 mir_dump/issue_47680.main.-------.borrows.borrowck..dot
-rw-rw-r-- 1 lqd lqd 13383 Oct  1 23:21 mir_dump/issue_47680.main.-------.ever_init.borrowck..dot
-rw-rw-r-- 1 lqd lqd 13591 Oct  1 23:21 mir_dump/issue_47680.main.-------.maybe_init.borrowck..dot
-rw-rw-r-- 1 lqd lqd  9257 Oct  1 23:21 mir_dump/issue_47680.main.-------.maybe_init.elaborate_drops..dot
-rw-rw-r-- 1 lqd lqd 14086 Oct  1 23:21 mir_dump/issue_47680.main.-------.maybe_uninit.borrowck..dot
-rw-rw-r-- 1 lqd lqd  9257 Oct  1 23:21 mir_dump/issue_47680.main.-------.maybe_uninit.elaborate_drops..dot
```

<summary>Some examples on nightly</summary>

</details>

And the other is for the specific `Borrows` dataflow analysis, whose domain is loans but shows locations when dumped (the location where the loan is introduced). It's not a huge deal but we didn't even print these locations in MIR dumps, and in general cross-referencing loan data (like loan liveness) is more annoying without this change.

<details>

![Untitled](https://github.com/user-attachments/assets/b325a6e9-1aee-4655-8441-d3b1b55ded3c)

<summary>Here's how it'll look in case inquisitive minds want to know</summary>

</details>

The visualization state diff display is still suboptimal in loops for some of the effects escaping a block, e.g. a gen that's not dominated/postdominated by a kill will not show up in statement diffs. (This happens in the previous screenshot, there's no `+bw1` anywhere). We can fix that in the future.
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc 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. rollup A PR which is a rollup labels Oct 2, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Contributor

bors commented Oct 2, 2024

📌 Commit 18ae5f6 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 Oct 2, 2024
@workingjubilee
Copy link
Member

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants