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

Aligned allocation of userdata block (data section) for Test failed #1192

Closed
Smertig opened this issue May 10, 2021 · 0 comments · Fixed by #1193
Closed

Aligned allocation of userdata block (data section) for Test failed #1192

Smertig opened this issue May 10, 2021 · 0 comments · Fixed by #1193

Comments

@Smertig
Copy link
Contributor

Smertig commented May 10, 2021

Hello!

I've found a non-obvious bug in alignment and memory size calculations. Let's take a simple type (assuming x86 build mode):

struct Test {
	std::uint64_t dummy;
};
static_assert(sizeof(Test) == 8);
static_assert(alignof(Test) == 8);
static_assert(sizeof(Test*) == 4);
static_assert(alignof(Test*) == 4);

The following piece of code leads to panic (sometimes):

sol::state lua;
lua["test"] = Test {};
// [sol2] An error occurred and panic has been invoked: aligned allocation of userdata block (data section) for 'Test' failed

I've made some research and found the following


The first problem is that misaligned_size can be less than aligned_size:

static const std::size_t initial_size = aligned_space_for<T*, T>(nullptr);
static const std::size_t misaligned_size = aligned_space_for<T*, T>(reinterpret_cast<void*>(0x1));

where align_space_for calculates memory size required to place correctly aligned types in memory block starting at specific address.

  • align_space_for<Test*, Test>((void*)0x0) == 16 (or even 12, see below)
  • align_space_for<Test*, Test>((void*)0x1) == 15

Technically, 0x0 is the best alignment for Test* (first type in pack). However, the second type (Test) would be placed at minimal address equal to 0x4 or 0x05 but requires 0x8 alignment so there would be a 3-4 byte gap. This means that align_space_for<...>((void*)0x0) doesn't calculate required space for best™ allocator and align_space_for<..>((void*)0x1) - for the worst™ one. More specifically, the worst case for Test is reached when allocator returns pointer with representation 0xXXXXX5: align_space_for<Test*, Test>((void*)0x5) == 19.


The second problem is the attempt to avoid UB, leading to randomization of the result.

template <typename... Args>
std::size_t aligned_space_for(void* alignment = nullptr) {
// use temporary storage to prevent strict UB shenanigans
char alignment_shim[(std::max)({ sizeof(Args)... }) + (std::max)({ alignof(Args)... })] {};
char* start = alignment != nullptr ? static_cast<char*>(alignment) : alignment_shim;
alignment = start;
(void)detail::swallow { int {}, (align_one(std::alignment_of_v<Args>, sizeof(Args), alignment), int {})... };
return static_cast<std::size_t>(static_cast<char*>(alignment) - start);
}

First of all, there is an UB for (void*)0x1 anyway. Just because (void*)0x1 is invalid address and is not nullptr so alignment_shim won't help.
Secondly, align_space_for<Test*, Test>(nullptr) may return 16 or 12 or something else depending on address of a local variable alignment_shim:

  • reinterpret_cast<std::uintptr_t>(&alignment_shim[0]) % 16 == 0 => result is 16
  • reinterpret_cast<std::uintptr_t>(&alignment_shim[0]) % 16 == 8 => result is 12
  • and so on..

In my case initial_size is 12, misaligned_size is 15 but allocator returns pointer with alignment equal to 16 requiring at least 16 bytes to place Test* and Test types.


Tested sol2 version: trunk (f92c28b)

P.S. I'm already working on PR. Hope, you'll accept it.

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 a pull request may close this issue.

1 participant