fix ICE when `asm_const` and `const_refs_to_static` are combined by folkertdev · Pull Request #129472 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation44 Commits2 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

fixes #129462
fixes #126896
fixes #124164

I think this is a case that was missed in the fix for #125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm const operand.

I'm not 100% sure that span_mirbug_and_err is the right macro here, but it is used earlier with builtin_deref and seems to do the trick.

r? @lcnr

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

Aug 23, 2024

I think this should be fixing a buch of known other ICEs, too. Try running the crashes test suite

no luck there

> ./x test --keep-stage 1 tests/crashes/

running 232 tests
........................................................................................  88/232
........................................................................................ 176/232
........................................................

test result: ok. 232 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5.63s

This comment has been minimized.

// FIXME(#129952): We probably want a more principled approach here.
if let Err(terr) = ty.error_reported() {
self.infcx.set_tainted_by_errors(terr);
}

Choose a reason for hiding this comment

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

I think that maybe we want to do it for all signatures, i.e. assign this match to a variable and then check whether that variable has any error_reported. I expect that to fix even more issues 😁

I am otherwise quite happy with this change

Choose a reason for hiding this comment

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

no further crashes are fixed by that change. I did rename #129952 to reflect that the tainting now occurs in a different place.

thanks for your patience and good work ❤️

@bors r+ rollup

📌 Commit 49e3b9a has been approved by lcnr

It is now in the queue for this repository.

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

Sep 5, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Sep 5, 2024

…m-const, r=lcnr

fix ICE when asm_const and const_refs_to_static are combined

fixes rust-lang#129462 fixes rust-lang#126896 fixes rust-lang#124164

I think this is a case that was missed in the fix for rust-lang#125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm const operand.

I'm not 100% sure that span_mirbug_and_err is the right macro here, but it is used earlier with builtin_deref and seems to do the trick.

r? @lcnr

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

…iaskrgr

Rollup of 12 pull requests

Successful merges:

rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
rust-lang#129472 (fix ICE when asm_const and const_refs_to_static are combined)
rust-lang#129653 (clarify that addr_of creates read-only pointers)
rust-lang#129775 (bootstrap: Try to track down why initial_libdir sometimes fails)
rust-lang#129939 (explain why Rvalue::Len still exists)
rust-lang#129940 (s390x: Fix a regression related to backchain feature)
rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
rust-lang#129943 (use the bootstrapped compiler for test-float-parse test)
rust-lang#129944 (Add compat note for trait solver change)
rust-lang#129947 (Add digit separators in Duration examples)
rust-lang#129955 (Temporarily remove fmease from the review rotation)
rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

…iaskrgr

Rollup of 12 pull requests

Successful merges:

rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
rust-lang#129472 (fix ICE when asm_const and const_refs_to_static are combined)
rust-lang#129653 (clarify that addr_of creates read-only pointers)
rust-lang#129775 (bootstrap: Try to track down why initial_libdir sometimes fails)
rust-lang#129939 (explain why Rvalue::Len still exists)
rust-lang#129940 (s390x: Fix a regression related to backchain feature)
rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
rust-lang#129943 (use the bootstrapped compiler for test-float-parse test)
rust-lang#129944 (Add compat note for trait solver change)
rust-lang#129947 (Add digit separators in Duration examples)
rust-lang#129955 (Temporarily remove fmease from the review rotation)
rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

…iaskrgr

Rollup of 12 pull requests

Successful merges:

rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
rust-lang#129472 (fix ICE when asm_const and const_refs_to_static are combined)
rust-lang#129653 (clarify that addr_of creates read-only pointers)
rust-lang#129775 (bootstrap: Try to track down why initial_libdir sometimes fails)
rust-lang#129939 (explain why Rvalue::Len still exists)
rust-lang#129940 (s390x: Fix a regression related to backchain feature)
rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
rust-lang#129943 (use the bootstrapped compiler for test-float-parse test)
rust-lang#129944 (Add compat note for trait solver change)
rust-lang#129947 (Add digit separators in Duration examples)
rust-lang#129955 (Temporarily remove fmease from the review rotation)
rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Sep 6, 2024

…m-const, r=lcnr

fix ICE when asm_const and const_refs_to_static are combined

fixes rust-lang#129462 fixes rust-lang#126896 fixes rust-lang#124164

I think this is a case that was missed in the fix for rust-lang#125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm const operand.

I'm not 100% sure that span_mirbug_and_err is the right macro here, but it is used earlier with builtin_deref and seems to do the trick.

r? @lcnr

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Sep 6, 2024

…m-const, r=lcnr

fix ICE when asm_const and const_refs_to_static are combined

fixes rust-lang#129462 fixes rust-lang#126896 fixes rust-lang#124164

I think this is a case that was missed in the fix for rust-lang#125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm const operand.

I'm not 100% sure that span_mirbug_and_err is the right macro here, but it is used earlier with builtin_deref and seems to do the trick.

r? @lcnr

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 6, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 6, 2024

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Sep 6, 2024

Rollup merge of rust-lang#129472 - folkertdev:const-refs-to-static-asm-const, r=lcnr

fix ICE when asm_const and const_refs_to_static are combined

fixes rust-lang#129462 fixes rust-lang#126896 fixes rust-lang#124164

I think this is a case that was missed in the fix for rust-lang#125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm const operand.

I'm not 100% sure that span_mirbug_and_err is the right macro here, but it is used earlier with builtin_deref and seems to do the trick.

r? @lcnr

Labels

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.

fix ICE when asm_const and const_refs_to_static are combined by folkertdev · Pull Request #129472 · rust-lang/rust (original) (raw)

Conversation

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix ICE when `asm_const` and `const_refs_to_static` are combined by folkertdev · Pull Request #129472 · rust-lang/rust (original) (raw)