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 10 pull requests #122511

Merged
merged 33 commits into from
Mar 15, 2024
Merged

Rollup of 10 pull requests #122511

merged 33 commits into from
Mar 15, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Elliot-Roberts and others added 30 commits February 26, 2024 23:11
`-s` option doesn't perfectly fit into debuginfo()'s semantics and may unexpectedly
remove metadata in shared libraries. Remove the implementation and suggest user to
use `strip` utility instead.
This will allow MIR building to check whether a function is eligible for
coverage instrumentation, and avoid collecting branch coverage info if it is
not.
Reason:

In order to build the Windows version of the Rust toolchain for the Android platform, the following patch to the cc is crate is required to avoid incorrectly determining that we are building with the Android NDK: rust-lang/cc-rs@57853c4

This patch is present in version 1.0.80 and newer versions of the cc crate. The rustc source distribution currently has 3 different versions of cc in the vendor directory, only one of which has the necessary fix.

We (the Android Rust toolchain) are currently maintaining local patches to upgrade the cc crate dependency versions, which we would like to upstream.
[AIX] Remove AixLinker's debuginfo() implementation

AIX ld's `-s` option doesn't perfectly fit` debuginfo()`'s semantics and may unexpectedly remove metadata in shared libraries. Remove the implementation of `AixLinker` and suggest user to use `strip` utility instead.
change std::process to drop supplementary groups based on CAP_SETGID

A trivial rebase of rust-lang#95982

Should fix rust-lang#39186 (from what I can tell)

Original description:

> Fixes rust-lang#88716
>
> * Before this change, when a process was given a uid via `std::os::unix::process::CommandExt.uid`, there would be a `setgroups` call (when the process runs) to clear supplementary groups for the child **if the parent was root** (to remove potentially unwanted permissions).
> * After this change, supplementary groups are cleared if we have permission to do so, that is, if we have the CAP_SETGID capability.
>
> This new behavior was agreed upon in rust-lang#88716 but there was a bit of uncertainty from `@Amanieu` here: [rust-lang#88716 (comment)](rust-lang#88716 (comment))
>
> > I agree with this change, but is it really necessary to ignore an EPERM from setgroups? If you have permissions to change UID then you should also have permissions to change groups. I would feel more comfortable if we documented set_uid as requiring both UID and GID changing permissions.
>
> The way I've currently written it, we ignore an EPERM as that's what rust-lang#88716 originally suggested. I'm not at all an expert in any of this so I'd appreciate feedback on whether that was the right way to go.
Make incremental sessions identity no longer depend on the crate names provided by source code

This makes incremental sessions identity no longer depend on the crate names provided by source code, implementing
rust-lang/compiler-team#726.

r? ````@oli-obk````
…leywiser

Copy byval argument to alloca if alignment is insufficient

Fixes rust-lang#122211

"Ignore whitespace" recommended.
coverage: Initial support for branch coverage instrumentation

(This is a review-ready version of the changes that were drafted in rust-lang#118305.)

This PR adds support for branch coverage instrumentation, gated behind the unstable flag value `-Zcoverage-options=branch`. (Coverage instrumentation must also be enabled with `-Cinstrument-coverage`.)

During THIR-to-MIR lowering (MIR building), if branch coverage is enabled, we collect additional information about branch conditions and their corresponding then/else blocks. We inject special marker statements into those blocks, so that the `InstrumentCoverage` MIR pass can reliably identify them even after the initially-built MIR has been simplified and renumbered.

The rest of the changes are mostly just plumbing needed to gather up the information that was collected during MIR building, and include it in the coverage metadata that we embed in the final binary.

Note that `llvm-cov show` doesn't print branch coverage information in its source views by default; that needs to be explicitly enabled with `--show-branches=count` or similar.

---

The current implementation doesn't have any support for instrumenting `if let` or let-chains. I think it's still useful without that, and adding it would be non-trivial, so I'm happy to leave that for future work.
Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification`  and  `unused_imports`

fixes rust-lang#121331

For an `item` that triggers lint unnecessary_qualification, if the `use item` which imports this item is also trigger unused import, fixing the two lints at the same time may lead to the problem that the `item` cannot be found.
This PR will avoid reporting lint unnecessary_qualification when conflict occurs.

r? ``@petrochenkov``
…, r=scottmcm

Implement `Duration::as_millis_{f64,f32}`

Implementation of rust-lang#122451.

Linked const-unstability to rust-lang#72440, so the post there should probably be updated to mentions the 2 new methods when/if this PR is merged.
…workingjubilee

Update version of cc crate

Reason:

In order to build the Windows version of the Rust toolchain for the Android platform, the following patch to the cc is crate is required to avoid incorrectly determining that we are building with the Android NDK: rust-lang/cc-rs@57853c4

This patch is present in version 1.0.80 and newer versions of the cc crate. The rustc source distribution currently has 3 different versions of cc in the vendor directory, only one of which has the necessary fix.

We (the Android Rust toolchain) are currently maintaining local patches to upgrade the cc crate dependency versions, which we would like to upstream.
…Nilstrieb

Make `SubdiagMessageOp` well-formed

`WF(Diag<'_, G>)` requires `G: EmissionGuarantee`, but we don't currently check this is true due to limitations in the solver. Probably still worth enforcing.

r? `@nnethercote` (or anyone can r+ this, really)
@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Mar 14, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit 6ce3110 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 Mar 14, 2024
@bors
Copy link
Contributor

bors commented Mar 14, 2024

⌛ Testing commit 6ce3110 with merge 0c3602d...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117118 ([AIX] Remove AixLinker's debuginfo() implementation)
 - rust-lang#121650 (change std::process to drop supplementary groups based on CAP_SETGID)
 - rust-lang#121764 (Make incremental sessions identity no longer depend on the crate names provided by source code)
 - rust-lang#122212 (Copy byval argument to alloca if alignment is insufficient)
 - rust-lang#122322 (coverage: Initial support for branch coverage instrumentation)
 - rust-lang#122373 (Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification`  and  `unused_imports`)
 - rust-lang#122479 (Implement `Duration::as_millis_{f64,f32}`)
 - rust-lang#122487 (Rename `StmtKind::Local` variant into `StmtKind::Let`)
 - rust-lang#122498 (Update version of cc crate)
 - rust-lang#122503 (Make `SubdiagMessageOp` well-formed)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member Author

@bors r-

@matthiaskrgr matthiaskrgr reopened this Mar 14, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit 6ce3110 has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Mar 15, 2024

⌛ Testing commit 6ce3110 with merge c2901f5...

@bors
Copy link
Contributor

bors commented Mar 15, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 15, 2024
@bors bors merged commit c2901f5 into rust-lang:master Mar 15, 2024
23 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 15, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#117118 [AIX] Remove AixLinker's debuginfo() implementation 0c451d3f8d49a4c58116216ed2cabf0a8db25213 (link)
#121650 change std::process to drop supplementary groups based on C… 3137f94f26870e51a5a3ce22293778cb7559d0fa (link)
#121764 Make incremental sessions identity no longer depend on the … 6623c597665db888d2c19a9f3d41c3eba91c6b2e (link)
#122212 Copy byval argument to alloca if alignment is insufficient 44209f2f65581945504be63ff57bedd12a2131fd (link)
#122322 coverage: Initial support for branch coverage instrumentati… 72b9f97bc9cbeb855751c06f69a5c5f4395942dd (link)
#122373 Fix the conflict problem between the diagnostics fixes of l… b38ff4e9ca88faf0829f37ee16eec71f2fc588ba (link)
#122479 Implement Duration::as_millis_{f64,f32} 6dea840cf4782e13df3636419cab1d6b90918920 (link)
#122487 Rename StmtKind::Local variant into StmtKind::Let 5595138f529965db832cedf805aa78280336f04f (link)
#122498 Update version of cc crate d52a453071d861a164f732aefcc33d92f3a0abe0 (link)
#122503 Make SubdiagMessageOp well-formed 7f203000f56e6d10b3c25c71374e88826bbbb04c (link)

previous master: f4b771bf1f

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 (c2901f5): 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

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.7% [2.7%, 2.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% [-2.6%, -2.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-2.6%, 2.7%] 2

Cycles

Results

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)
-1.4% [-1.4%, -1.4%] 1
Improvements ✅
(secondary)
-4.0% [-4.0%, -4.0%] 1
All ❌✅ (primary) -1.4% [-1.4%, -1.4%] 1

Binary size

Results

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

Bootstrap: 669.645s -> 671.201s (0.23%)
Artifact size: 311.49 MiB -> 311.47 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. O-unix Operating system: Unix-like 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-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.