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.

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

Add PME backstop to detect padding in IntoBytes types #1768

Open
joshlf opened this issue Sep 27, 2024 · 0 comments
Open

Add PME backstop to detect padding in IntoBytes types #1768

joshlf opened this issue Sep 27, 2024 · 0 comments

Comments

@joshlf
Copy link
Member

joshlf commented Sep 27, 2024

As demonstrated in #1764, #1763, #1758, and #1757, reasoning about the presence or absence of padding is subtle.

When a type is Sized and concrete, we can easily emit a padding check using size_of to confirm that the size of the type is equal to the sum of the sizes of its fields (although note that this is still unsound in the presence of niche-filling). However, on unsized or generic types, our job is much harder, and #[derive(IntoBytes)] employs subtle logic to determine whether Rust could in principle emit padding bytes in a particular type.

When the type is generic, the only reason that we can't emit a padding check is that it requires the unstable generic_const_exprs feature. However, we could still use a post-monomorphization error (PME). In particular, I propose the following:

  • Add a #[doc(hidden)] associated const to IntoBytes which evaluates to () if a type has no padding or panics during evaluation if it does have padding
  • Evaluate this const in all IntoBytes methods, and maybe in other places that consume T: IntoBytes

Note that this is a backstop, but it must not be taken as a complete proof that a type is padding-free. In particular, user code is permitted to use T: IntoBytes as a precondition to justify the soundness of unsafe code - we cannot guarantee that such user code will call our methods. Thus, we cannot guarantee that such user code would trigger a PME.

@joshlf joshlf added blocking-next-release This issue should be resolved before we release on crates.io blocking-next-release-publicization and removed blocking-next-release This issue should be resolved before we release on crates.io labels Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant