fix ICE when `asm_const` and `const_refs_to_static` are combined #129472

folkertdev · 2024-08-23T14:42:26Z

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

compiler/rustc_borrowck/src/type_check/mod.rs

compiler/rustc_mir_build/src/thir/cx/expr.rs

compiler/rustc_hir_typeck/src/lib.rs

oli-obk · 2024-09-04T08:15:20Z

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

folkertdev · 2024-09-04T08:19:51Z

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

tests/ui/polymorphization/taint.rs

lcnr · 2024-09-04T12:13:31Z

compiler/rustc_borrowck/src/universal_regions.rs

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


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

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

compiler/rustc_borrowck/src/universal_regions.rs

lcnr · 2024-09-05T07:28:52Z

thanks for your patience and good work ❤️

@bors r+ rollup

bors · 2024-09-05T07:28:54Z

📌 Commit 49e3b9a has been approved by lcnr

It is now in the queue for this repository.

…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`

…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

…iaskrgr Rollup of 11 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#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) r? `@ghost` `@rustbot` modify labels: rollup

…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```

…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````

…iaskrgr Rollup of 11 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#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) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 14 pull requests Successful merges: - rust-lang#128919 (Add an internal lint that warns when accessing untracked data) - rust-lang#129021 (Check WF of source type's signature on fn pointer cast) - 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#129781 (Make `./x.py <cmd> compiler/<crate>` aware of the crate's features) - rust-lang#129939 (explain why Rvalue::Len still exists) - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc) - 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) - rust-lang#129969 (Make `Ty::boxed_ty` return an `Option`) - rust-lang#129995 (Remove wasm32-wasip2's tier 2 status from release notes) r? `@ghost` `@rustbot` modify labels: rollup

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``

rustbot assigned lcnr Aug 23, 2024

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

dingxiangfei2009 mentioned this pull request Aug 29, 2024

Stabilize const_refs_to_static #129759

Open

lcnr reviewed Sep 2, 2024

View reviewed changes

compiler/rustc_borrowck/src/type_check/mod.rs Outdated Show resolved Hide resolved

folkertdev force-pushed the const-refs-to-static-asm-const branch from 3805bdc to 4f65da6 Compare September 2, 2024 15:22

lcnr reviewed Sep 2, 2024

View reviewed changes

compiler/rustc_mir_build/src/thir/cx/expr.rs Outdated Show resolved Hide resolved

folkertdev mentioned this pull request Sep 4, 2024

taint on type error in UniversalRegionsBuilder::compute_inputs_and_output #129952

Open

lcnr reviewed Sep 4, 2024

View reviewed changes

compiler/rustc_hir_typeck/src/lib.rs Outdated Show resolved Hide resolved

folkertdev force-pushed the const-refs-to-static-asm-const branch 2 times, most recently from 5e33c81 to 774fbc7 Compare September 4, 2024 11:04

This comment has been minimized.

Sign in to view

folkertdev force-pushed the const-refs-to-static-asm-const branch from 774fbc7 to 957502d Compare September 4, 2024 12:03

lcnr reviewed Sep 4, 2024

View reviewed changes

tests/ui/polymorphization/taint.rs Outdated Show resolved Hide resolved

lcnr reviewed Sep 4, 2024

View reviewed changes

folkertdev force-pushed the const-refs-to-static-asm-const branch from 957502d to 7ec7724 Compare September 4, 2024 13:11

lcnr reviewed Sep 4, 2024

View reviewed changes

compiler/rustc_borrowck/src/universal_regions.rs Outdated Show resolved Hide resolved

folkertdev added 2 commits September 4, 2024 20:06

propagate tainted_by_errors in MirBorrowckCtxt::emit_errors

f7679d0

fix ICE when asm_const and const_refs_to_static are combined

49e3b9a

folkertdev force-pushed the const-refs-to-static-asm-const branch from 7ec7724 to 49e3b9a Compare September 4, 2024 18:07

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 mentioned this pull request Sep 5, 2024

Rollup of 12 pull requests #129984

Closed

matthiaskrgr mentioned this pull request Sep 5, 2024

Rollup of 11 pull requests #129999

Merged

workingjubilee mentioned this pull request Sep 6, 2024

Rollup of 15 pull requests #130007

Closed

workingjubilee mentioned this pull request Sep 6, 2024

Rollup of 14 pull requests #130010

Closed

bors merged commit 3daa015 into rust-lang:master Sep 6, 2024
6 checks passed

rustbot added this to the 1.83.0 milestone Sep 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ICE when `asm_const` and `const_refs_to_static` are combined #129472

fix ICE when `asm_const` and `const_refs_to_static` are combined #129472

folkertdev commented Aug 23, 2024 •

edited

Loading

oli-obk commented Sep 4, 2024

folkertdev commented Sep 4, 2024

This comment has been minimized.

lcnr Sep 4, 2024

folkertdev Sep 4, 2024

lcnr commented Sep 5, 2024 •

edited

Loading

bors commented Sep 5, 2024

fix ICE when asm_const and const_refs_to_static are combined #129472

fix ICE when asm_const and const_refs_to_static are combined #129472

Conversation

folkertdev commented Aug 23, 2024 • edited Loading

oli-obk commented Sep 4, 2024

folkertdev commented Sep 4, 2024

This comment has been minimized.

lcnr Sep 4, 2024

Choose a reason for hiding this comment

folkertdev Sep 4, 2024

Choose a reason for hiding this comment

lcnr commented Sep 5, 2024 • edited Loading

bors commented Sep 5, 2024

fix ICE when `asm_const` and `const_refs_to_static` are combined #129472

fix ICE when `asm_const` and `const_refs_to_static` are combined #129472

folkertdev commented Aug 23, 2024 •

edited

Loading

lcnr commented Sep 5, 2024 •

edited

Loading