Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix ICE in non-operand aggregate_raw_ptr intrinsic codegen #125184

Merged
merged 1 commit into from
May 18, 2024
Merged

Fix ICE in non-operand aggregate_raw_ptr intrinsic codegen #125184

merged 1 commit into from
May 18, 2024

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented May 16, 2024 •
edited
Loading

Introduced in #123840
Found in #121571, cc @clarfonthey

@rustbot
Copy link
Collaborator

rustbot commented May 16, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 16, 2024
@scottmcm scottmcm mentioned this pull request May 16, 2024
Merged
@scottmcm scottmcm changed the title Fix ICE in non-operand aggregate_raw_ptr instrinsic codegen Fix ICE in non-operand aggregate_raw_ptr intrinsic codegen May 16, 2024
scottmcm
scottmcm commented May 16, 2024
View reviewed changes
tests/codegen/intrinsics/aggregate-thin-pointer.rs
use std::intrinsics::aggregate_raw_ptr;

// InstSimplify replaces these with casts if it can, which means they're almost
// never seen in codegen, but PR#121571 found a way, so add a test for it.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ICE demonstration: https://godbolt.org/z/Y7o63MzPn

@clarfonthey
Copy link
Contributor

clarfonthey commented May 16, 2024 •
edited
Loading

Hmm, I might be doing something wrong, but this doesn't seem to actually fix the issue encountered in #121571; rebased on this commit and still running into the same ICE. I pushed the rebased version onto that branch for you to test, which should also help verify if it's still breaking on CI and isn't just my setup.

EDIT: Yup, it's still happening on CI. #121571 (comment)

EDIT 2: Oh, it's just that the bug is in the bootstrap compiler. :(

@fee1-dead
Copy link
Member

r? compiler

@rustbot rustbot assigned jieyouxu and unassigned fee1-dead May 18, 2024
jieyouxu
jieyouxu approved these changes May 18, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The changes look good to me.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 18, 2024

📌 Commit f60f2e8 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 18, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request May 18, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2024
@bors
Auto merge of rust-lang#125254 - matthiaskrgr:rollup-ukvsxbc, r=matth…
c00957a 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#125117 (Improve parser)
 - rust-lang#125184 (Fix ICE in non-operand `aggregate_raw_ptr` intrinsic codegen)
 - rust-lang#125240 (Temporarily revert to NonZeroUsize in rustc-abi to fix building on stable)
 - rust-lang#125248 (Migrate `run-make/rustdoc-scrape-examples-invalid-expr` to `rmake.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e4e7568 into rust-lang:master May 18, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2024
@rust-timer
Unrolled build for rust-lang#125184
d27af5d 
Rollup merge of rust-lang#125184 - scottmcm:fix-thin-ptr-ice, r=jieyouxu

Fix ICE in non-operand `aggregate_raw_ptr` intrinsic codegen

Introduced in rust-lang#123840
Found in rust-lang#121571, cc `@clarfonthey`
@scottmcm scottmcm deleted the fix-thin-ptr-ice branch May 19, 2024 01:35
@saethlin saethlin added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 24, 2024
@saethlin
Copy link
Member

I would like this in beta so that the stage0 compiler does not ICE if it gets passed -Zmir-opt-level=0. Yes I can work around this issue without huge difficulty, but the diff here is so small.

@apiraino
Copy link
Contributor

apiraino commented Jun 5, 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 5, 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

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.
9 participants
@scottmcm @rustbot @clarfonthey @fee1-dead @jieyouxu @bors @saethlin @apiraino @cuviper