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

CFI: Monomorphize transparent ADTs before typeid #106232

Merged
merged 1 commit into from
Dec 31, 2022
Merged

CFI: Monomorphize transparent ADTs before typeid #106232

merged 1 commit into from
Dec 31, 2022

Conversation

maurer
Copy link
Contributor

@maurer maurer commented Dec 28, 2022

Monomorphise #[repr(transparent)] parameterized ADTs before turning them into an Itanium mangled String.

#[repr(transparent)] ADTs currently use the single field to represent them in their CFI type ID to ensure that they are compatible. However, if that type involves a type parameter instantiated at the ADT level, as in ManuallyDrop, this will currently ICE as the Parameter type cannot be mangled. Since this happens at lowering time, it should always be concrete after substitution.

Fixes #106230

@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2022

r? @Nilstrieb

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

@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 Dec 28, 2022
@maurer
Copy link
Contributor Author

maurer commented Dec 28, 2022

cc @rcvalle

@compiler-errors
Copy link
Member

One change and this can be r+'d.

@rcvalle
Copy link
Member

rcvalle commented Dec 29, 2022

Thanks for reporting and also working on a fix/solution for it, @maurer! Much appreciated.

@rcvalle
Copy link
Member

rcvalle commented Dec 29, 2022

(I also just wanted to let you know that I’m currently on vacation so my responses may be delayed.)

@Noratrieb
Copy link
Member

@bors delegate=rcvalle

@bors
Copy link
Contributor

bors commented Dec 29, 2022

✌️ @rcvalle can now approve this pull request

@maurer
CFI: Monomorphize transparent ADTs before typeid
fb2c27d 
Monomorphise `#[repr(transparent)]` parameterized ADTs before turning
them into an Itanium mangled String.

`#[repr(transparent)]` ADTs currently use the single field to represent
them in their CFI type ID to ensure that they are compatible. However,
if that type involves a type parameter instantiated at the ADT level, as
in `ManuallyDrop`, this will currently ICE as the `Parameter` type
cannot be mangled. Since this happens at lowering time, it should always
be concrete after substitution.

Fixes rust-lang#106230
@rcvalle
Copy link
Member

rcvalle commented Dec 30, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Dec 30, 2022

📌 Commit fb2c27d has been approved by rcvalle

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 Dec 30, 2022
jyn514 added a commit to jyn514/rust that referenced this pull request Dec 30, 2022
@jyn514
Rollup merge of rust-lang#106232 - maurer:transparent-subst, r=rcvalle
266f92c 
CFI: Monomorphize transparent ADTs before typeid

Monomorphise `#[repr(transparent)]` parameterized ADTs before turning them into an Itanium mangled String.

`#[repr(transparent)]` ADTs currently use the single field to represent them in their CFI type ID to ensure that they are compatible. However, if that type involves a type parameter instantiated at the ADT level, as in `ManuallyDrop`, this will currently ICE as the `Parameter` type cannot be mangled. Since this happens at lowering time, it should always be concrete after substitution.

Fixes rust-lang#106230
@jyn514 jyn514 mentioned this pull request Dec 30, 2022
Closed
jyn514 added a commit to jyn514/rust that referenced this pull request Dec 31, 2022
@jyn514
Rollup merge of rust-lang#106232 - maurer:transparent-subst, r=rcvalle
b9d6d17 
CFI: Monomorphize transparent ADTs before typeid

Monomorphise `#[repr(transparent)]` parameterized ADTs before turning them into an Itanium mangled String.

`#[repr(transparent)]` ADTs currently use the single field to represent them in their CFI type ID to ensure that they are compatible. However, if that type involves a type parameter instantiated at the ADT level, as in `ManuallyDrop`, this will currently ICE as the `Parameter` type cannot be mangled. Since this happens at lowering time, it should always be concrete after substitution.

Fixes rust-lang#106230
@jyn514 jyn514 mentioned this pull request Dec 31, 2022
Closed
@bors
Copy link
Contributor

bors commented Dec 31, 2022

⌛ Testing commit fb2c27d with merge 8007b106aa929d53fed4e9826b7c74ce405b0341...

@bors
Copy link
Contributor

bors commented Dec 31, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 31, 2022
@jyn514
Copy link
Member

jyn514 commented Dec 31, 2022

@bors treeclosed=500 tidy merge check is breaking both here and in #106319 (comment)

@jyn514
Copy link
Member

jyn514 commented Dec 31, 2022

@bors r=Nilstrieb

@bors
Copy link
Contributor

bors commented Dec 31, 2022

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Dec 31, 2022

📌 Commit fb2c27d has been approved by Nilstrieb

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 31, 2022

🌲 The tree is currently closed for pull requests below priority 500. This pull request will be tested once the tree is reopened.

@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 Dec 31, 2022
This was referenced Dec 31, 2022
Merged
Merged
@bors bors mentioned this pull request Dec 31, 2022
Merged
@bors bors closed this in 5570cda Dec 31, 2022
@compiler-errors
Copy link
Member

Lol, bors closed this because of the "fixes" comment.

@compiler-errors
Copy link
Member

@bors r=rcvalle just in case the rollup that this is a part of fails

@bors
Copy link
Contributor

bors commented Dec 31, 2022

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Rollup of 7 pull requests #106319

@bors
Copy link
Contributor

bors commented Dec 31, 2022

📌 Commit fb2c27d has been approved by rcvalle

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 31, 2022

🌲 The tree is currently closed for pull requests below priority 500. This pull request will be tested once the tree is reopened.

@compiler-errors
Copy link
Member

@bors treeclosed-

@compiler-errors compiler-errors mentioned this pull request Dec 31, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 31, 2022
@bors
Auto merge of rust-lang#106324 - compiler-errors:rollup-2m9njin, r=co…
247e44e 
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#105903 (Unify id-based thread parking implementations)
 - rust-lang#106232 (CFI: Monomorphize transparent ADTs before typeid)
 - rust-lang#106248 (Revert "Implement allow-by-default `multiple_supertrait_upcastable` lint")
 - rust-lang#106286 (Make tidy errors red)
 - rust-lang#106295 (Extend scraped examples layout GUI test for position of buttons)
 - rust-lang#106305 ( bootstrap: Get rid of tail_args in stream_cargo)
 - rust-lang#106310 (Dont use `--merge-base` during bootstrap formatting subcommand)
 - rust-lang#106314 (Fix panic on `x build --help`)
 - rust-lang#106317 (Only deduplicate stack traces for good path bugs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
* highest error code: E0791
Found 507 error codes
Found 0 error(s) in error codes
Done!
thread '<unnamed>' panicked at 'Encountered error while testing Git status: "failed to execute git log command: fatal: ambiguous argument ':!src/tools/miri': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"', src/tools/tidy/src/no_merge.rs:23:25
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Any { .. }', src/tools/tidy/src/main.rs:42:61
* 393 features
Build completed unsuccessfully in 0:00:26

@bors bors merged commit fad7339 into rust-lang:master Dec 31, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 31, 2022
@rcvalle rcvalle mentioned this pull request Jan 14, 2023
Closed
@rcvalle rcvalle added the PG-exploit-mitigations Project group: Exploit mitigations label Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcvalle rcvalle rcvalle left review comments

@compiler-errors compiler-errors compiler-errors left review comments

Assignees

@Noratrieb Noratrieb

Labels
PG-exploit-mitigations Project group: Exploit mitigations 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.68.0
Development

Successfully merging this pull request may close these issues.

ICE: CFI dies on #[repr(transparent)] on a parameterized ADT (e.g. ManuallyDrop).
8 participants
@maurer @rustbot @compiler-errors @rcvalle @Noratrieb @bors @jyn514 @rust-log-analyzer