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

Implement FromBytes and AsBytes for raw pointers #171

Closed
wants to merge 1 commit into from
Closed

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented May 20, 2023

EDIT: This is now blocked on rust-lang/unsafe-code-guidelines#401

Previously, we omitted these impls out of a fear of creating a footgun in which it would be too easy to mistakenly transmute into a type with internal safety invariants. Recently, this omission meant that I was unable to get rid of a number of lines of unsafe code in a codebase that used raw pointers, and it made me realize that it is more important to enable this use case than to avoid a theoretical footgun.

Closes #170

Previously, we omitted these impls out of a fear of creating a footgun
in which it would be too easy to mistakenly transmute into a type with
internal safety invariants. Recently, this omission meant that I was
unable to get rid of a number of lines of `unsafe` code in a codebase
that used raw pointers, and it made me realize that it is more important
to enable this use case than to avoid a theoretical footgun.

Closes #170
@djkoloski
Copy link
Member

djkoloski commented May 20, 2023

Per tests, looks like tests/ui/transmute-illegal.stderr needs to be updated now that pointers may be transmuted. The soundness looks fine to me. 🙂

@joshlf
Copy link
Member Author

joshlf commented May 20, 2023

Oh damn, that may actually be a clue to a soundness hole. For those following along, here's the error I get:

error[E0080]: evaluation of constant value failed
  --> tests/ui/transmute-illegal.rs:10:1
   |
10 | const POINTER_VALUE: usize = zerocopy::transmute!(&0usize as *const usize);
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
   |
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported

Are we guaranteed that rustc's const eval will catch every instance of turning a pointer into raw bytes? If so, this should be okay - it'll just result in a potentially confusing compile-time error for the user. If this is a best effort analysis, though, then introducing those trait impls may be unsound.

@djkoloski
Copy link
Member

AFAIK the compiler will catch all cases where a pointer is transmuted to bytes at compile time. If you want to guarantee that we can't do that in const expressions, the macro could expand to a non const function that is immediately invoked. I don't think the guarantees around const UB are well defined.

@joshlf
Copy link
Member Author

joshlf commented May 20, 2023

Submitted rust-lang/unsafe-code-guidelines#401 to get a clearer answer; if the answer is that it's guaranteed, I'll follow up with a change to the Rust reference to make it clear that it's guaranteed.

This also brings up another question, which is: None of our safety comments justifying FromBytes, AsBytes, and Unaligned impls mention the const context. If there are other operations which are sound in non-const code but unsound in const code, then those safety arguments may be incomplete. Do we need to come up with a general strategy for reasoning about const soundness?

@djkoloski
Copy link
Member

I anticipate the answer will be that any time const soundness is more restrictive than runtime soundness, the compiler is guaranteed to emit a compile error. If that's not the case, then it's difficult to make a single coherent strategy

@joshlf
Copy link
Member Author

joshlf commented May 21, 2023

Closing for now; I'll reopen depending on the resolution of rust-lang/unsafe-code-guidelines#401

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

Successfully merging this pull request may close these issues.

Implement FromBytes and AsBytes for raw pointers
2 participants