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.

Sign up for GitHub

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

Jump to bottom

Fix insufficient logic when searching for the underlying allocation #124761

Merged
merged 1 commit into from
May 8, 2024
Merged

Fix insufficient logic when searching for the underlying allocation #124761

merged 1 commit into from
May 8, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented May 5, 2024

This PR fixes the logic inside the invalid_reference_casting lint, when trying to lint on bigger memory layout casts.

More specifically when looking for the "underlying allocation" we were wrongly assuming that when we got &mut slice[index] that slice[index] was the allocation, but it's not.

Fixes #124685

@Urgau
Fix insufficient logic when searching for the underlying allocation
cd6a0c8 
in the `invalid_reference_casting` lint, when trying to lint on
bigger memory layout casts.
@rustbot
Copy link
Collaborator

rustbot commented May 5, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added 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. labels May 5, 2024
@jieyouxu
Copy link
Member

jieyouxu commented May 8, 2024

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 8, 2024

📌 Commit cd6a0c8 has been approved by jieyouxu

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 May 8, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request May 8, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 8, 2024
@bors
Auto merge of rust-lang#124890 - matthiaskrgr:rollup-25iq88h, r=matth…
c3202af 
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#124548 (Handle normalization failure in `struct_tail_erasing_lifetimes`)
 - rust-lang#124761 (Fix insufficient logic when searching for the underlying allocation)
 - rust-lang#124864 (rustdoc: use stability, instead of features, to decide what to show)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9fce3dc into rust-lang:master May 8, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 8, 2024
@rust-timer
Unrolled build for rust-lang#124761
ac66264 
Rollup merge of rust-lang#124761 - Urgau:ref-casting_bigger_slice_index, r=jieyouxu

Fix insufficient logic when searching for the underlying allocation

This PR fixes the logic inside the `invalid_reference_casting` lint, when trying to lint on bigger memory layout casts.

More specifically when looking for the "underlying allocation" we were wrongly assuming that when we got `&mut slice[index]` that `slice[index]` was the allocation, but it's not.

Fixes rust-lang#124685
@Urgau Urgau deleted the ref-casting_bigger_slice_index branch May 8, 2024 19:40
@saethlin saethlin mentioned this pull request May 8, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 9, 2024
@matthiaskrgr
Rollup merge of rust-lang#124908 - saethlin:ref-casting_bigger_place_…
48b1e1a 
…projection, r=fee1-dead

Handle field projections like slice indexing in invalid_reference_casting

r? `@Urgau`

I saw the implementation in rust-lang#124761, and I was wondering if we also need to handle field access. We do. Without this PR, we get this errant diagnostic:
```
error: casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused
  --> /home/ben/rust/tests/ui/lint/reference_casting.rs:262:18
   |
LL |         let r = &mut v.0;
   |                      --- backing allocation comes from here
LL |         let ptr = r as *mut i32 as *mut Vec3<i32>;
   |                   ------------------------------- casting happend here
LL |         unsafe { *ptr = Vec3(0, 0, 0) }
   |                  ^^^^^^^^^^^^^^^^^^^^
   |
   = note: casting from `i32` (4 bytes) to `Vec3<i32>` (12 bytes)
```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 9, 2024
@rust-timer
Unrolled build for rust-lang#124908
7833133 
Rollup merge of rust-lang#124908 - saethlin:ref-casting_bigger_place_projection, r=fee1-dead

Handle field projections like slice indexing in invalid_reference_casting

r? `@Urgau`

I saw the implementation in rust-lang#124761, and I was wondering if we also need to handle field access. We do. Without this PR, we get this errant diagnostic:
```
error: casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused
  --> /home/ben/rust/tests/ui/lint/reference_casting.rs:262:18
   |
LL |         let r = &mut v.0;
   |                      --- backing allocation comes from here
LL |         let ptr = r as *mut i32 as *mut Vec3<i32>;
   |                   ------------------------------- casting happend here
LL |         unsafe { *ptr = Vec3(0, 0, 0) }
   |                  ^^^^^^^^^^^^^^^^^^^^
   |
   = note: casting from `i32` (4 bytes) to `Vec3<i32>` (12 bytes)
```
@saethlin saethlin mentioned this pull request May 10, 2024
Open
1 task
@saethlin saethlin added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 1, 2024
@apiraino
Copy link
Contributor

apiraino commented Jun 6, 2024

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 6, 2024
@cuviper cuviper mentioned this pull request Jun 6, 2024
Merged
@cuviper cuviper modified the milestones: 1.80.0, 1.79.0 Jun 6, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 6, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2024
@bors
Auto merge of rust-lang#126093 - cuviper:beta-next, r=cuviper
49f3109 
[beta] backports

- Fix insufficient logic when searching for the underlying allocation rust-lang#124761
- Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908
- Handle Deref expressions in invalid_reference_casting rust-lang#124978
- Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184
- Wrap Context.ext in AssertUnwindSafe rust-lang#125392
- Revert problematic opaque type change rust-lang#125489
- ast: Revert a breaking attribute visiting order change rust-lang#125734
- Update to LLVM 18.1.7 rust-lang#126061

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2024
@bors
Auto merge of rust-lang#126093 - cuviper:beta-next, r=cuviper
f4d7315 
[beta] backports

- Fix insufficient logic when searching for the underlying allocation rust-lang#124761
- Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908
- Handle Deref expressions in invalid_reference_casting rust-lang#124978
- Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184
- Wrap Context.ext in AssertUnwindSafe rust-lang#125392
- Revert problematic opaque type change rust-lang#125489
- ast: Revert a breaking attribute visiting order change rust-lang#125734
- Update to LLVM 18.1.7 rust-lang#126061
- Revert "Disallow ambiguous attributes on expressions" on beta rust-lang#126102 / rust-lang#126101
- Silence double-symlink errors while building solaris toolchain rust-lang#126011

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2024
@bors
Auto merge of rust-lang#126093 - cuviper:beta-next, r=cuviper
7bb6510 
[beta] backports

- Fix insufficient logic when searching for the underlying allocation rust-lang#124761
- Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908
- Handle Deref expressions in invalid_reference_casting rust-lang#124978
- Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184
- Wrap Context.ext in AssertUnwindSafe rust-lang#125392
- Revert problematic opaque type change rust-lang#125489
- ast: Revert a breaking attribute visiting order change rust-lang#125734
- Update to LLVM 18.1.7 rust-lang#126061
- Revert "Disallow ambiguous attributes on expressions" on beta rust-lang#126102 / rust-lang#126101
- Silence double-symlink errors while building solaris toolchain rust-lang#126011

r? cuviper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jieyouxu jieyouxu

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.

False positives from invalid_reference_casting
7 participants
@Urgau @rustbot @jieyouxu @bors @apiraino @cuviper @saethlin