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 #104702

Merged
merged 27 commits into from
Nov 22, 2022
Merged

Rollup of 7 pull requests #104702

merged 27 commits into from
Nov 22, 2022

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

scottmcm and others added 27 commits April 1, 2022 18:51
The strict provenance APIs are a better version of these, and things like `.addr()` work for the "that cast looks sketchy" case even if the full strict provenance stuff never happens.  So I think it's fine to move away from these, and encourage the others instead.
Changing to those doesn't introduce any new unsoundness over the existing ones, so they're the better "if you won't want to think about it" replacement.  But also mention the strict provenance APIs, as that's what we'd rather they use instead.
This fixes an issue where the C and asm sources built by
compiler_builtins were being compiled as ELF objects instead of PE
objects. This wasn't noticed before because it doesn't cause
compiler_builtins or rustc to fail to build. You only see a failure when
a program is built that references one of the symbols in an ELF object.

Compiling with clang fixes this because the `cc` crate converts the UEFI
targets into Windows targets that clang understands, causing it to
produce PE objects.

Note that this requires compiler_builtins >= 0.1.84.

Fixes rust-lang#104326
This syncs it with how the UEFI targets are built in dist-various-2.
Remove bad impl for Eq

Update Cargo.lock and fix last ValidAlign
If clang isn't the C compiler used for the UEFI targets, or if the wrong
`--target` is passed to clang, we will get ELF objects in some
rlibs. This will cause problems at link time when trying to compile a
UEFI program that uses any of those objects. Add a check to the dist
step for UEFI targets that reads each rlib with the `object` crate and
fails with an error if any non-COFF objects are found.
Add slice methods for indexing via an array of indices.

Disclaimer: It's been a while since I contributed to the main Rust repo, apologies in advance if this is large enough already that it should've been an RFC.

---

# Update:

- Based on feedback, removed the `&[T]` variant of this API, and removed the requirements for the indices to be sorted.

# Description

This adds the following slice methods to `core`:

```rust
impl<T> [T] {
    pub unsafe fn get_many_unchecked_mut<const N: usize>(&mut self, indices: [usize; N]) -> [&mut T; N];
    pub fn get_many_mut<const N: usize>(&mut self, indices: [usize; N]) -> Option<[&mut T; N]>;
}
```

This allows creating multiple mutable references to disjunct positions in a slice, which previously required writing some awkward code with `split_at_mut()` or `iter_mut()`. For the bound-checked variant, the indices are checked against each other and against the bounds of the slice, which requires `N * (N + 1) / 2` comparison operations.

This has a proof-of-concept standalone implementation here: https://crates.io/crates/index_many

Care has been taken that the implementation passes miri borrow checks, and generates straight-forward assembly (though this was only checked on x86_64).

# Example

```rust
let v = &mut [1, 2, 3, 4];
let [a, b] = v.get_many_mut([0, 2]).unwrap();
std::mem::swap(a, b);
*v += 100;
assert_eq!(v, &[3, 2, 101, 4]);
```

# Codegen Examples

<details>
  <summary>Click to expand!</summary>

Disclaimer: Taken from local tests with the standalone implementation.

## Unchecked Indexing:

```rust
pub unsafe fn example_unchecked(slice: &mut [usize], indices: [usize; 3]) -> [&mut usize; 3] {
    slice.get_many_unchecked_mut(indices)
}
```

```nasm
example_unchecked:
 mov     rcx, qword, ptr, [r9]
 mov     r8, qword, ptr, [r9, +, 8]
 mov     r9, qword, ptr, [r9, +, 16]
 lea     rcx, [rdx, +, 8*rcx]
 lea     r8, [rdx, +, 8*r8]
 lea     rdx, [rdx, +, 8*r9]
 mov     qword, ptr, [rax], rcx
 mov     qword, ptr, [rax, +, 8], r8
 mov     qword, ptr, [rax, +, 16], rdx
 ret
```

## Checked Indexing (Option):

```rust
pub unsafe fn example_option(slice: &mut [usize], indices: [usize; 3]) -> Option<[&mut usize; 3]> {
    slice.get_many_mut(indices)
}
```

```nasm
 mov     r10, qword, ptr, [r9, +, 8]
 mov     rcx, qword, ptr, [r9, +, 16]
 cmp     rcx, r10
 je      .LBB0_7
 mov     r9, qword, ptr, [r9]
 cmp     rcx, r9
 je      .LBB0_7
 cmp     rcx, r8
 jae     .LBB0_7
 cmp     r10, r9
 je      .LBB0_7
 cmp     r9, r8
 jae     .LBB0_7
 cmp     r10, r8
 jae     .LBB0_7
 lea     r8, [rdx, +, 8*r9]
 lea     r9, [rdx, +, 8*r10]
 lea     rcx, [rdx, +, 8*rcx]
 mov     qword, ptr, [rax], r8
 mov     qword, ptr, [rax, +, 8], r9
 mov     qword, ptr, [rax, +, 16], rcx
 ret
.LBB0_7:
 mov     qword, ptr, [rax], 0
 ret
```

## Checked Indexing (Panic):

```rust
pub fn example_panic(slice: &mut [usize], indices: [usize; 3]) -> [&mut usize; 3] {
    let len = slice.len();
    match slice.get_many_mut(indices) {
        Some(s) => s,
        None => {
            let tmp = indices;
            index_many::sorted_bound_check_failed(&tmp, len)
        }
    }
}
```

```nasm
example_panic:
 sub     rsp, 56
 mov     rax, qword, ptr, [r9]
 mov     r10, qword, ptr, [r9, +, 8]
 mov     r9, qword, ptr, [r9, +, 16]
 cmp     r9, r10
 je      .LBB0_6
 cmp     r9, rax
 je      .LBB0_6
 cmp     r9, r8
 jae     .LBB0_6
 cmp     r10, rax
 je      .LBB0_6
 cmp     rax, r8
 jae     .LBB0_6
 cmp     r10, r8
 jae     .LBB0_6
 lea     rax, [rdx, +, 8*rax]
 lea     r8, [rdx, +, 8*r10]
 lea     rdx, [rdx, +, 8*r9]
 mov     qword, ptr, [rcx], rax
 mov     qword, ptr, [rcx, +, 8], r8
 mov     qword, ptr, [rcx, +, 16], rdx
 mov     rax, rcx
 add     rsp, 56
 ret
.LBB0_6:
 mov     qword, ptr, [rsp, +, 32], rax
 mov     qword, ptr, [rsp, +, 40], r10
 mov     qword, ptr, [rsp, +, 48], r9
 lea     rcx, [rsp, +, 32]
 mov     edx, 3
 call    index_many::bound_check_failed
 ud2
```
</details>

# Extensions

There are multiple optional extensions to this.

## Indexing With Ranges

This could easily be expanded to allow indexing with `[I; N]` where `I: SliceIndex<Self>`.  I wanted to keep the initial implementation simple, so I didn't include it yet.

## Panicking Variant

We could also add this method:

```rust
impl<T> [T] {
    fn index_many_mut<const N: usize>(&mut self, indices: [usize; N]) -> [&mut T; N];
}
```

This would work similar to the regular index operator and panic with out-of-bound indices. The advantage would be that we could more easily ensure good codegen with a useful panic message, which is non-trivial with the `Option` variant.

This is implemented in the standalone implementation, and used as basis for the codegen examples here and there.
…, r=dtolnay

Deprecate the unstable `ptr_to_from_bits` feature

I propose that we deprecate the (unstable!) `to_bits` and `from_bits` methods on raw pointers.  (With the intent to ~~remove them once `addr` has been around long enough to make the transition easy on people -- maybe another 6 weeks~~ remove them fairly soon after, as the strict and expose versions have been around for a while already.)

The APIs that came from the strict provenance explorations (rust-lang#95228) are a more holistic version of these, and things like `.expose_addr()` work for the "that cast looks sketchy" case even if the full strict provenance stuff never happens.  (As a bonus, `addr` is even shorter than `to_bits`, though it is only applicable if people can use full strict provenance! `addr` is *not* a direct replacement for `to_bits`.)  So I think it's fine to move away from the `{to|from}_bits` methods, and encourage the others instead.

That also resolves the worry that was brought up (I forget where) that `q.to_bits()` and `(*q).to_bits()` both work if `q` is a pointer-to-floating-point, as they also have a `to_bits` method.

Tracking issue rust-lang#91126
Code search: https://github.com/search?l=Rust&p=1&q=ptr_to_from_bits&type=Code

For potential pushback, some users in case they want to chime in
- `@RSSchermer` https://github.com/RSSchermer/ARWA/blob/365bb68541447453fc44f6fbcc5d394bb94c14e9/arwa/src/html/custom_element.rs#L105
- `@strax` https://github.com/strax/pbr/blob/99616d1dbf42f93ec8dd668d05b3180649558180/openexr/src/core/alloc.rs#L36
- `@MiSawa` https://github.com/MiSawa/pomelo/blob/577c6223588d539295a71ff125d8f249e59f4146/crates/kernel/src/timer.rs#L50
Make the Box one-liner more descriptive

I would like to avoid a definition that relies on itself.

r? `@GuillaumeGomez`
Constify remaining `Layout` methods

Makes the methods on `Layout` that aren't yet unstably const, under the same feature and issue, rust-lang#67521. Most of them required no changes, only non-trivial change is probably constifying `ValidAlignment` which may affect rust-lang#102072
mark sys_common::once::generic::Once::new const-stable

Attempt to address rust-lang#103191 by marking the impl const-stable.
Picked the declaration from the callsite:
https://github.com/rust-lang/rust/blob/21b246587c2687935bd6004ffa5dcc4f4dd6600d/library/std/src/sync/once.rs#L67

This is similar to rust-lang#98457.

With this in, `python3 x.py build library/std --target x86_64-unknown-none` succeeds.
…r=Mark-Simulacrum

Use clang for the UEFI targets

This fixes an issue where the C and asm sources built by compiler_builtins were being compiled as ELF objects instead of PE objects. This wasn't noticed before because it doesn't cause compiler_builtins or rustc to fail to build. You only see a failure when a program is built that references one of the symbols in an ELF object.

Compiling with clang fixes this because the cc crate converts the UEFI targets into Windows targets that clang understands, causing it to produce PE objects.

Also update compiler_builtins to 0.1.84 to pull in some necessary fixes for compiling the UEFI targets with clang.

Fixes rust-lang#104326
…piler-errors

Move macro_rules diagnostics to diagnostics module

This will make it easier to add more diagnostics in the future in a centralized place.
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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. labels Nov 22, 2022
@rustbot rustbot added 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. rollup A PR which is a rollup labels Nov 22, 2022
@Manishearth
Copy link
Member Author

@bors r+ p=7 rollup=never

@bors
Copy link
Contributor

bors commented Nov 22, 2022

📌 Commit 9043dfd has been approved by Manishearth

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 Nov 22, 2022
@bors
Copy link
Contributor

bors commented Nov 22, 2022

⌛ Testing commit 9043dfd with merge 11c2dc8d992ddd7997290e35cc97c92161d200c0...

@bors
Copy link
Contributor

bors commented Nov 22, 2022

💔 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 Nov 22, 2022
@Manishearth
Copy link
Member Author

    Finished dev [unoptimized + debuginfo] target(s) in 1.22s
`--no-sandbox` is being used. Be very careful!
Running 89 rustdoc-gui (1 concurrently) ...
.......... (10/89)
......... (20/89)
.......... (30/89)
.......... (40/89)
.......... (50/89)
.......... (60/89)
.......... (70/89)
.......... (80/89)
.........  (89/89)


/checkout/src/test/rustdoc-gui/code-sidebar-toggle.goml code-sidebar-toggle... FAILED
Error:  (line 4) Error: Execution context was destroyed, most likely because of a navigation.: for command `wait-for: "#sidebar-toggle"`

@Manishearth
Copy link
Member Author

@bors retry force

@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 Nov 22, 2022
@bors
Copy link
Contributor

bors commented Nov 22, 2022

⌛ Testing commit 9043dfd with merge a78c9be...

@Manishearth
Copy link
Member Author

cc @rust-lang/rustdoc potential flakiness?

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
.......... (80/89)
.........  (89/89)


/checkout/src/test/rustdoc-gui/code-sidebar-toggle.goml code-sidebar-toggle... FAILED
[ERROR] (line 4) Error: Execution context was destroyed, most likely because of a navigation.: for command `wait-for: "#sidebar-toggle"`
Build completed unsuccessfully in 0:02:15

@bors
Copy link
Contributor

bors commented Nov 22, 2022

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing a78c9be to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 22, 2022
@bors bors merged commit a78c9be into rust-lang:master Nov 22, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 22, 2022
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#104638 0c3255ed2af2247810687f2668d4819b355d021f
#104622 2a229c767d9969a1712be5c041346b03eef811b8
#103193 70fc12379cadb726bf184608015ad0a266aa4444
#102207 c3beea6ce51cab5ef57447383c761566599d88ae
#101655 166b8e097cd8dfb988429d930be4413fc9396e91
#95583 9e152f495d759d1e7ca9847885e51de68b5c8499
#83608 f71ac7f9100c75f806a5edbf2a417a85b57ec8cb

previous master: 0f7d81754d

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 (a78c9be): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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

Cycles

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

@Manishearth Manishearth deleted the rollup-75hagzd branch November 24, 2022 07:25
@GuillaumeGomez
Copy link
Member

For the GUI failure, linked to #93784.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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.

None yet