Skip to content

Unsound transmute in lib #26

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

Closed
MolotovCherry opened this issue Feb 2, 2023 · 1 comment
Closed

Unsound transmute in lib #26

MolotovCherry opened this issue Feb 2, 2023 · 1 comment

Comments

@MolotovCherry
Copy link

Just a small note, but the repr(Rust) memory layout does not guarantee that two similarly defined structs are laid out the exact same way in memory. Thus, transmutes between repr(Rust) types are UB.

For reference, please see this nomicon article here:
https://doc.rust-lang.org/nomicon/repr-rust.html

Rust does guarantee that two instances of A have their data laid out in exactly the same way. However Rust does not currently guarantee that an instance of A has the same field ordering or padding as an instance of B.

The struct in question requires a repr(transparent) or repr(C) to be safe. However, ThreadId does not have any of these on it, so putting it only on one will do little good.

std::mem::transmute::<std::thread::ThreadId, FakeThreadId>(thread::current().id()).0

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 2, 2023

Thanks for the note!

I agree that it's not safe by the book, but I do have those const checks that make sure they're both u64, which I think means that this should be fine in practice... Also even if the two u64 are laid out differently, that would likely still work for this application, the specific value here isn't super important.

With that said, if there's some safe code that works on stable, let me know. As I mention in the comment rust-lang/rust#67939 is nightly only and I don't want to deal with the headache of nightly toolchain. I used to have something that did more explicit hashing, but that was slower and resulted in collisions which made perf scale worse with thread count.

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2023
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

2 participants