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

abi: unsized field in union - assert to delay bug #114060

Merged
merged 2 commits into from
Jul 25, 2023
Merged

abi: unsized field in union - assert to delay bug #114060

merged 2 commits into from
Jul 25, 2023

Conversation

davidtwco
Copy link
Member

Fixes #113279.

Unions cannot have unsized fields, and as such, layout computation for
unions asserts that each union field is sized (as this would normally
have halted compilation earlier).

However, if a generator ends up with an unsized local - a circumstance
in which an error will always have been emitted earlier, for example, if
attempting to dereference a &str - then the generator transform will
produce a union with an unsized field.

Since #110107, later passes will be run, such as constant propagation,
and can attempt layout computation on the generator, which will result
in layout computation of str in the context of it being a field of a
union - and so the aforementioned assertion would cause an ICE.

It didn't seem appropriate to try and detect this case in the MIR body
and skip this specific pass; tainting the MIR body or delaying a bug
from the generator transform (or elsewhere) wouldn't prevent this either
(as neither would prevent the later pass from running); and tainting when
the deref of &str is reported, if that's possible, would unnecessarily
prevent potential other errors from being reported later in compilation,
and is very tailored to this specific case of getting a unsized type in
a generator.

Given that this circumstance can only happen when an error should have
already been reported, the correct fix appears to be just changing the
assert to a delayed bug. This will still assert if there is some
circumstance where this occurs and no error has been reported, but it
won't crash the compiler in this instance.

While debugging this, I noticed a translation ICE in a delayed bug, so I fixed that too:

During borrowck, the MultiSpan from a buffered diagnostic is cloned and
used to emit a delayed bug indicating a diagnostic was buffered - when
the buffered diagnostic is translated, then the cloned MultiSpan may
contain labels which can only render with the diagnostic's arguments, but
the delayed bug being emitted won't have those arguments. Adds a function
which clones MultiSpan without also cloning the contained labels, and
use this function when creating the buffered diagnostic delayed bug.

@davidtwco
borrowck/errors: fix i18n error in delayed bug
3857d9c 
During borrowck, the `MultiSpan` from a buffered diagnostic is cloned and
used to emit a delayed bug indicating a diagnostic was buffered - when
the buffered diagnostic is translated, then the cloned `MultiSpan` may
contain labels which can only render with the diagnostic's arguments, but
the delayed bug being emitted won't have those arguments. Adds a function
which clones `MultiSpan` without also cloning the contained labels, and
use this function when creating the buffered diagnostic delayed bug.

Signed-off-by: David Wood <david@davidtw.co>
@davidtwco
abi: unsized field in union - assert to delay bug
037b274 
Unions cannot have unsized fields, and as such, layout computation for
unions asserts that each union field is sized (as this would normally
have halted compilation earlier).

However, if a generator ends up with an unsized local - a circumstance
in which an error will always have been emitted earlier, for example, if
attempting to dereference a `&str` - then the generator transform will
produce a union with an unsized field.

Since rust-lang#110107, later passes will be run, such as constant propagation,
and can attempt layout computation on the generator, which will result
in layout computation of `str` in the context of it being a field of a
union - and so the aforementioned assertion would cause an ICE.

It didn't seem appropriate to try and detect this case in the MIR body
and skip this specific pass; tainting the MIR body or delaying a bug
from the generator transform (or elsewhere) wouldn't prevent this either
(as neither would prevent the later pass from running); and tainting when
the deref of `&str` is reported, if that's possible, would unnecessarily
prevent potential other errors from being reported later in compilation,
and is very tailored to this specific case of getting a unsized type in
a generator.

Given that this circumstance can only happen when an error should have
already been reported, the correct fix appears to be just changing the
assert to a delayed bug. This will still assert if there is some
circumstance where this occurs and no error has been reported, but it
won't crash the compiler in this instance.

Signed-off-by: David Wood <david@davidtw.co>
@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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 Jul 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

@davidtwco davidtwco changed the title abi: abi: unsized field in union - assert to delay bug Jul 25, 2023
@wesleywiser
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 25, 2023

📌 Commit 037b274 has been approved by wesleywiser

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 Jul 25, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 25, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2023
@bors
Auto merge of rust-lang#114063 - matthiaskrgr:rollup-c90czu6, r=matth…
18fa7b9 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114008 (coverage: Obtain the `__llvm_covfun` section name outside a per-function loop)
 - rust-lang#114014 (builtin_macros: expect raw strings too)
 - rust-lang#114043 (docs(LazyLock): add example pass local LazyLock variable to struct)
 - rust-lang#114051 (Add regression test for invalid "unused const" in method)
 - rust-lang#114052 (Suggest `{Option,Result}::as_ref()` instead of `cloned()` in some cases)
 - rust-lang#114058 (Add help for crate arg when crate name is invalid)
 - rust-lang#114060 (abi: unsized field in union - assert to delay bug )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ba6982b into rust-lang:master Jul 25, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Jul 25, 2023
@davidtwco davidtwco deleted the issue-113279 branch July 26, 2023 13:15
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 31, 2023
@bors
Auto merge of rust-lang#114063 - matthiaskrgr:rollup-c90czu6, r=matth…
61e0aac 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114008 (coverage: Obtain the `__llvm_covfun` section name outside a per-function loop)
 - rust-lang#114014 (builtin_macros: expect raw strings too)
 - rust-lang#114043 (docs(LazyLock): add example pass local LazyLock variable to struct)
 - rust-lang#114051 (Add regression test for invalid "unused const" in method)
 - rust-lang#114052 (Suggest `{Option,Result}::as_ref()` instead of `cloned()` in some cases)
 - rust-lang#114058 (Add help for crate arg when crate name is invalid)
 - rust-lang#114060 (abi: unsized field in union - assert to delay bug )

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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.73.0
Development

Successfully merging this pull request may close these issues.

ICE: generators: assertion failed: field.0.is_sized()
4 participants
@davidtwco @rustbot @wesleywiser @bors