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.

Sign up for GitHub

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

Jump to bottom

Usage in dynamic objects fails due to alignment #1

Open
lahwaacz opened this issue Dec 30, 2020 · 0 comments · May be fixed by #2
Open

Usage in dynamic objects fails due to alignment #1

lahwaacz opened this issue Dec 30, 2020 · 0 comments · May be fixed by #2

Comments

@lahwaacz
Copy link

async::queue and all classes using it have alignment requirements, which does not play nice with dynamic objects in C++.

Consider the following code:

#include <iostream>
#include <cstddef>  // std::max_align_t
#include "async/async/threadpool.h"

struct A
{
    A() : tp(1) {}

    async::threadpool tp;
};

int main()
{
    std::cout << "alignof(std::max_align_t) = " << alignof(std::max_align_t) << std::endl;
    std::cout << "sizeof(async::threadpool) = " << sizeof(async::threadpool) << std::endl;
    std::cout << "alignof(async::threadpool) = " << alignof(async::threadpool) << std::endl;

    std::vector<std::shared_ptr<A>> vec;
    for (int i = 0; i < 1000; i++)
        vec.push_back(std::make_shared<A>());
}

Compile it with g++ -pthread -g -fsanitize=undefined code.cpp, then the typical output may look like

alignof(std::max_align_t) = 16
sizeof(async::threadpool) = 9152
alignof(async::threadpool) = 64
/usr/include/c++/10.2.0/bits/shared_ptr_base.h:682:16: runtime error: constructor call on misaligned address 0x55d39b50cf70 for type 'struct _Sp_cp_type', which requires 64 byte alignment
0x55d39b50cf70: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
/usr/include/c++/10.2.0/bits/shared_ptr_base.h:546:2: runtime error: member access within misaligned address 0x55d39b50cf70 for type 'struct _Sp_counted_ptr_inplace', which requires 64 byte alignment
0x55d39b50cf70: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
...

(The error output is actually much longer.)

This is because the dynamic allocation in C++ is not required to satisfy alignment requirements stricter than the std::max_align_t type (which requires only 16 bytes compared to 64 bytes of the async::threadpool). See https://stackoverflow.com/a/53485295 and https://stackoverflow.com/a/16510895/4180822 for details.
lahwaacz added a commit to lahwaacz/async that referenced this issue Dec 31, 2020
@lahwaacz
added CachelineAlignment to the queue traits
8378bbe 
The purpose of the cacheline-size padding between class members is to
avoid false sharing when threads modify the atomic members, but they do
not have to be aligned exactly to the cacheline-size. The atomic members
are very small (just 4 or 8 bytes) compared to the cacheline. As far as
I can tell, their explicit alignment does not matter, they can be placed
anywhere on the cacheline - we just need to ensure they are mapped to
different cachelines.

To avoid problems with misaligned dynamic allocation, the alignment must
not be stricter than alignof(std::max_align_t), which is 16 bytes on
x86_64 Linux. The value was added to the traits structs.

Fixes d36u9#1
@lahwaacz lahwaacz linked a pull request Dec 31, 2020 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Decouple alignment from cacheline size lahwaacz/async
1 participant
@lahwaacz