Skip to content

Use read_unaligned when reading u64#257

Open
illia-bobyr wants to merge 1 commit into
blueshift-gg:masterfrom
illia-bobyr:patch-1
Open

Use read_unaligned when reading u64#257
illia-bobyr wants to merge 1 commit into
blueshift-gg:masterfrom
illia-bobyr:patch-1

Conversation

@illia-bobyr

Copy link
Copy Markdown

Casting an arbitrary pointer to u64 is an undefined behavior.

Not sure if casting of the discriminator value is really necessary, as it is just a u8 in this example. But using the same *ptr.cast::<uXX>() pattern in there seems wrong. It is a noop for u8, but if a u64 is used, it becomes undefined behavior as well.

Also, the unsafe usage is somewhat unclear. Dereferencing a pointer is an unsafe operation, but the first two examples do not have unsafe. Yet in the last example, when a slice is constructed, unsafe is present. I assume that the first two examples are expected to be in an unsafe context already?

@febo

febo commented Oct 25, 2025

Copy link
Copy Markdown
Contributor

Casting an arbitrary pointer to u64 is an undefined behavior.

Sorry to jump into this PR, but I am not sure I follow the reason for this being undefined behaviour. The runtime serialization guarantees that the input pointer will be aligned to an u64 value at that offset.

@illia-bobyr

Copy link
Copy Markdown
Author

Casting an arbitrary pointer to u64 is an undefined behavior.

Sorry to jump into this PR, but I am not sure I follow the reason for this being undefined behaviour. The runtime serialization guarantees that the input pointer will be aligned to an u64 value at that offset.

No worries - your comment is welcome :)

Yes, if we include the ABI behavior in the calculation, then these reads are actually safe.
I didn't know that ABI provides these guarantees when I wrote this comment.
And, to be honest, I do not see it the ABI docs - I only found it in the code a bit later.

I read this tutorial up to this point, and while it is possible that I've missed it, I did not see the VM ABI alignment behavior stated anywhere.

So if I consider just the content of the tutorial, it is unsafe to do an unaligned read here.
And if we want to rely on the VM ABI behavior, it would probably be good to state it explicitly?
ptr::read is an unsafe operation, and alignment is one of the things that the programmer need to check manually when using it.
So, good practice is then to state it explicitly in a preceding // SAFETY: comment.
At the same time, considering the number of reads though a pointer, it would probably be overly verbose.

Casting an arbitrary pointer to `u64` is an undefined behavior.

Not sure if casting of the discriminator value is really necessary, as
it is just a `u8` in this example.  But using the same
`*ptr.cast::<uXX>()` pattern in there seems wrong.  It is a noop for
`u8`, but if a `u64` is used, it becomes undefined behavior as well.

Also, the `unsafe` usage is somewhat unclear.  Dereferencing a pointer
is an unsafe operation, but the first two examples do not have `unsafe`.
Yet in the last example, when a slice is constructed, `unsafe` is
present.  I assume that the first two examples are expected to be in an
unsafe context already?
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.

2 participants