object_rep: use `aligned_storage` when possible

[timblechmann]

the culprit here is that boost::aligned_storage has an alignof(int128_t) but the memory allocated by lua is only aligned by the standard alignment. std::aligned_storage however has an alignment of 1 so it is safe to allocate.

this fixes ubsan warnings like:

src/object_rep.cpp:263:61: runtime error: constructor call on misaligned address 0x55f9e96fa6a8 for type 'struct object_rep', which requires 16 byte alignment
    0x55f9e96fa6a8: note: pointer points here
     00 00 00 00  12 00 00 00 db 00 00 00  69 00 00 00 74 00 00 00  31 00 00 00 00 00 00 00  80 06 7a ec

[timblechmann]

Note that overalignment is fine, but the pointer printed is aligned to 8 byte while Ubsan wants 16 byte alignment for some reason. I cannot imagine size_t being 16 byte aligned, so I think the problem may be something else.

afaict overalignment is the actual problem here: object_rep is constructed via placement new into memory which is not sufficiently aligned. afaict one would have to over-allocate and manually re-align in push_new_instance

I believe we might have a silly mistake here, see line comment.

ah, i had ported this from an older in-house fix. reverting to boost for now with size_t alignment

[Oberon00]

OK, so the alignment requirements of object_rep itself is ignored? That would need to be fixed then instead of just making UBsan believe that the alignment requirements are less than they really are.

[timblechmann]

OK, so the alignment requirements of object_rep itself is ignored?

placement new does not have any notion of alignment. so when using wrongly aligned placement new in wrongly aligned data (e.g. allocated by malloc), the sanitizer rightfully complains

[Oberon00]

That's clear to me. The question is why the placement new is provided under-aligned storage. It looks like the storage is allocated by lua_newuserdata. Normally such functions that don't have a way to specify alignment (also malloc) allocate with maximum alignment.

With your patch, you simply reduce the alignment requirement from 16 to 8 for the aligned_storage member and presumably also for the whole object_rep. However, if we then placement-new something inside the inner aligned storage, that object might actually have an alignment requirement of 16 (e.g. if it is a long double) and be again under-aligned.

[timblechmann]

Normally such functions that don't have a way to specify alignment (also malloc) allocate with maximum alignment.

iirc malloc on macos is 16byte aligned, but on linux it's 8-byte aligned.

[Oberon00]

Should be 16 on 64 bit systems: https://stackoverflow.com/questions/17380686/malloc-and-unaligned-memory

[timblechmann]

Should be 16 on 64 bit systems: https://stackoverflow.com/questions/17380686/malloc-and-unaligned-memory

hmm, interesting ... though lua uses realloc and i didn't override its allocator ...

[kaetemi]

Alternative fix: https://github.com/Oberon00/luabind/pull/50

If you trace through lua_newuserdata, you'll find that it's aligned based on LUAI_MAXALIGN, not based on the default allocator. The allocation block has some header fields, and the payload only has LUAI_MAXALIGN as alignment guarantee.

Declaring object_rep as 16-byte aligned is simply wrong, since it's stored in a LUAI_MAXALIGN aligned field.

[Oberon00]

This should be solved by #50.