Stream: t-lang/wg-unsafe-code-guidelines

Topic: C struct alignment


Elichai Turkel (Nov 18 2019 at 11:18, on Zulip):

Hi, i'm not sure this is the best place to ask, but for FFI does alignment has to match or does it has to be rust_align >= c_align?

Amanieu (Nov 18 2019 at 12:37, on Zulip):

transmute ignores alignment and only cares about size. If you remove the transmute, I think it's still fine, at least in that direction. Note that in the other direction (taking a C paramter in a function called from C) you need to make sure that the alignment on the Rust side is <= the alignment on the C side.

RalfJ (Nov 18 2019 at 19:45, on Zulip):

the reason transmute can ignore alignment is that it does a copy of the data

RalfJ (Nov 18 2019 at 19:45, on Zulip):

alignment is relevant when talking about pointers to data, not about things passed by-value

Elichai Turkel (Nov 18 2019 at 21:04, on Zulip):

@RalfJ You're right. my mistake this is a better example of what I mean: https://play.rust-lang.org/?gist=f19f7904fae3436a143a7fcaeb755f6a (though I'm also interested if transmuting can change the answer here somehow)

RalfJ (Nov 19 2019 at 10:14, on Zulip):

generally the alignment requirement is an "at least requirement"

RalfJ (Nov 19 2019 at 10:14, on Zulip):

so having a 16-aligned pointer to something 4-aligned is perfectly fine

RalfJ (Nov 19 2019 at 10:14, on Zulip):

however, there is one case where you have to be careful, and that is allocators

RalfJ (Nov 19 2019 at 10:15, on Zulip):

allocating memory in rust demands a certain size and alignment; deallocation needs to happen with the same size and alignment

RalfJ (Nov 19 2019 at 10:15, on Zulip):

that's why you cannot transmute a Vec<u32> into a Vec<u8> (of 4 times the length) -- you can make the size match, but not the alignment

gnzlbg (Nov 19 2019 at 11:46, on Zulip):

deallocation needs to happen with the same size and alignment

This isn't fully accurate, you can deallocate with a different size, and a different alignment.

gnzlbg (Nov 19 2019 at 11:47, on Zulip):

The problem with the Vec example, is that the alignment used when deallocating a Vec<u8> is less than the one used by Vec<u32>

gnzlbg (Nov 19 2019 at 11:48, on Zulip):

but one can constructo a MyVec type for which transmuting MyVec<u32> to MyVec<u8> is fine

gnzlbg (Nov 19 2019 at 11:50, on Zulip):

by computing the alignment passed on the deallocation to the allocator from the pointer, instead of just using mem::align_of::<T>()

gnzlbg (Nov 19 2019 at 12:04, on Zulip):

as long as the Layout matches the allocation class used by the allocator, you are fine

RalfJ (Nov 19 2019 at 15:46, on Zulip):

deallocation needs to happen with the same size and alignment

This isn't fully accurate, you can deallocate with a different size, and a different alignment - otherwise the actual allocation size returned by alloc_excess would be useless.

uh, what? the docs say quite clearly that the size+align must match?

RalfJ (Nov 19 2019 at 15:46, on Zulip):

as long as the Layout matches the allocation class used by the allocator, you are fine

that's an allocator-internal thing. for allocator-agnostic code, you have to assume that the allocator has one class per size+alloc combination.

RalfJ (Nov 19 2019 at 15:47, on Zulip):

by computing the alignment passed on the deallocation to the allocator from the pointer, instead of just using mem::align_of::<T>()

I dont think you can compute the alignment from the pointer, without making assumptions beyond what the allocator API provides?

gnzlbg (Nov 19 2019 at 17:23, on Zulip):

uh, what? the docs say quite clearly that the size+align must match?

Then the docs are wrong, because you can do Alloc::alloc_excess and that returns a size that can be larger than the requested size, and this size can be used for deallocation.

gnzlbg (Nov 19 2019 at 17:24, on Zulip):

I dont think you can compute the alignment from the pointer,

You can find the largest power of two that's divisible by the address, for example.

gnzlbg (Nov 19 2019 at 17:30, on Zulip):

AFAICT the docs do not say that anywhere, e.g., std::alloc::Alloc docs explicitly say:

if a layout k fits a memory block (denoted by ptr) currently allocated via an allocator a, then it is legal to use that layout to deallocate it, i.e., a.dealloc(ptr, k);

gnzlbg (Nov 19 2019 at 17:30, on Zulip):

The only requirement is that the Layout used for deallocation must fit the actual Layout of the allocation

gnzlbg (Nov 19 2019 at 17:31, on Zulip):

The actual Layout of the allocation can be different than the Layout requested, and can also be different than the Layout returned by the Alloc trait methods

gnzlbg (Nov 19 2019 at 17:31, on Zulip):

But the Layout requested and the Layouts returned must fit the actual Layout, because you can use either during deallocation.

gnzlbg (Nov 19 2019 at 17:33, on Zulip):

Those docs expand on what "fit" means.

gnzlbg (Nov 19 2019 at 17:33, on Zulip):

And in particular for aligned, it says:

The block's starting address must be aligned to layout.align().

gnzlbg (Nov 19 2019 at 17:36, on Zulip):

Ah wait, Alloc::dealloc imposes an extra requirement:

In addition to fitting the block of memory layout, the alignment of the layout must match the alignment used to allocate that block of memory.

gnzlbg (Nov 19 2019 at 17:36, on Zulip):

So the size only needs to "fit", but the alignment needs to exactly match.

RalfJ (Nov 19 2019 at 18:36, on Zulip):

I dont think you can compute the alignment from the pointer,

You can find the largest power of two for which addr % pow2 == 0.

that has no correlation with the alignment requested though, so it is useless

gnzlbg (Nov 21 2019 at 09:51, on Zulip):

Depends on the allocator

gnzlbg (Nov 21 2019 at 09:52, on Zulip):

If your allocator supports deallocating with any alignment that’s satisfied by a pointer, then you can always pass just 1, but the generic API doesn’t require all allocators to support that.

gnzlbg (Nov 21 2019 at 09:53, on Zulip):

A specific impl can support that

RalfJ (Nov 21 2019 at 10:32, on Zulip):

Depends on the allocator

all of this discussion has been for "normal" Rust code. as in, the only thing you know about the allocator is what the allocator API explicitly says. quite clearly that is the default and anyone asking here "may I do this" doesn't mean "does there exist an allocator for which I may do this".
I dont understand why you insist on bringing in guarantees only made by certain allocators when that is irrelevant for the discussion.

gnzlbg (Nov 21 2019 at 12:57, on Zulip):

You claimed that the size and alignment must exactly match

gnzlbg (Nov 21 2019 at 12:57, on Zulip):

I hope I have convinced you that this claim is wrong

gnzlbg (Nov 21 2019 at 12:57, on Zulip):

Only the alignment must match

gnzlbg (Nov 21 2019 at 12:58, on Zulip):

The allocator api is derived from real allocators, I don't know any for which the alignment must actually match

rkruppe (Nov 21 2019 at 12:59, on Zulip):

Note that you quoted the unstable Alloc trait, GlobalAlloc::dealloc explicitly says "layout must be the same layout that was used to allocate that block of memory"

gnzlbg (Nov 21 2019 at 13:00, on Zulip):

If that's the case, the Alloc trait implementations for GlobalAlloc are unsound

rkruppe (Nov 21 2019 at 13:02, on Zulip):

¯\_(ツ)_/¯

gnzlbg (Nov 21 2019 at 13:02, on Zulip):

and rustc violates that requirement

rkruppe (Nov 21 2019 at 13:03, on Zulip):

The easy fix, naturally, is to drop alloc_excess and change the unstable trait to match the requirements of GlobalAlloc

gnzlbg (Nov 21 2019 at 13:04, on Zulip):

The last allocator-wg proposal drops alloc_excess, but it makes alloc return the excess

gnzlbg (Nov 21 2019 at 13:04, on Zulip):

meaning there is no GlobalAlloc compatible way to implement Alloc

rkruppe (Nov 21 2019 at 13:05, on Zulip):

Well, have fun sorting that mess out, I'm not gonna touch that :upside_down:

gnzlbg (Nov 21 2019 at 13:06, on Zulip):

one can probably just require that impls of Alloc that use GlobalAlloc to drop the excess internally, but... i don't think any impl does that

gnzlbg (Nov 21 2019 at 13:06, on Zulip):

kind of defeats the point

gnzlbg (Nov 21 2019 at 13:06, on Zulip):

there is also a proposal to deprecate GlobalAlloc and replace it with Alloc, which would sort this out as well

rkruppe (Nov 21 2019 at 13:08, on Zulip):

IIUC that would cause the same problem as liberalizing the safety requirement on GlobalAlloc::dealloc, it causes previously valid implementations to become unsound

gnzlbg (Nov 21 2019 at 13:09, on Zulip):

Not really, you can only have one #[global_allocator] in a binary, and it has to implement either Alloc or GlobalAlloc. If it implements GlobalAlloc, and Alloc impl shim gets added

gnzlbg (Nov 21 2019 at 13:10, on Zulip):

That Alloc shim would need to be generated in such a way that it is sound, but it can just return the requested size as the excess.

gnzlbg (Nov 21 2019 at 13:10, on Zulip):

meaning users can only dealloc with the requested size

rkruppe (Nov 21 2019 at 13:10, on Zulip):

I guess?

rkruppe (Nov 21 2019 at 13:11, on Zulip):

Anyway I should stick to my word and not get nerdsniped by this

gnzlbg (Nov 21 2019 at 14:26, on Zulip):

This has a much simpler fix.

gnzlbg (Nov 21 2019 at 14:28, on Zulip):

Currently there are no GlobalAlloc APIs that return the allocation actual size. Those APIs can be added as trait methods with a default implementation, that just returns the requested size. This allows relaxing Layout constraints of GlobalAlloc::dealloc to match those of the Alloc trait.

gnzlbg (Nov 21 2019 at 14:28, on Zulip):

Current implementations remain sound, because they do not currently provide an implementation of the default trait method.

gnzlbg (Nov 21 2019 at 14:29, on Zulip):

If a current implementation were to be modified to provide an impl of the _excess methods to return the actual allocation size, then this would require updating GlobalAlloc::dealloc to be able to handle that. But doing this would be entirely opt-in.

RalfJ (Nov 22 2019 at 16:08, on Zulip):

I hope I have convinced you that this claim is wrong

fair. but you also claimed things about alignment that were wrong (at least if they were meant as a response to my question -- and if not one wonders why they were posted in this thread)... no need to continue discussing that but I feet communication was quite suboptimal here.

RalfJ (Nov 22 2019 at 16:08, on Zulip):

if you think the docs of one of the involved traits are wrong, make a PR?

gnzlbg (Nov 25 2019 at 12:35, on Zulip):

The docs of both traits look correct to me, the Alloc trait docs support deallocating with different layouts

gnzlbg (Nov 25 2019 at 12:36, on Zulip):

The GlobalAlloc trait does not, but it does not have any APIs for which doing that would make sense

gnzlbg (Nov 25 2019 at 12:38, on Zulip):

The current state of affairs just means that you cannot mix-match Alloc and GlobalAlloc calls for the same allocator, e.g., alloc with Alloc::alloc and dealloc wit hGlobalAlloc::dealloc, but that's documented already in the part that talks about "the pointer returned must have been produced by calling one of the Self methods.

gnzlbg (Nov 25 2019 at 12:41, on Zulip):

The trait implementation of Alloc and GlobalAlloc themselves can rely on extra semantic guarantees, e.g., have Alloc::alloc do something different than GlobalAlloc::alloc but have Alloc::dealloc just call GlobalAlloc::dealloc.

gnzlbg (Nov 25 2019 at 12:43, on Zulip):

I.e. the implementor of the Alloc and GlobalAlloc traits can violate their generic API.

Last update: Dec 12 2019 at 00:50UTC