unaligned atomic variables

[TA]

Hi Anthony,

It might be useful to have assertion for unaligned atomic variables. For example:

Code: [Select]

#pragma pack(1)

struct X
{
std::atomic_char a;
std::atomic_long b;
};
As plain loads and stores are guaranteed to be atomic on x86 only in case of aligned read/write this structure may cause problems. In example above atomic_long is not properly aligned (I guess internal representation too), so I think it will be good idea to notify user (at least in debug mode) that library is not going to work as expected. Or alternatively you can explicitly align internal variable to the required boundary (I think this one is a better solution).
What do you think?

Here is the example that produces data race with atomic variables. In the loop I'm trying to place one variable into two different cache lines. I changed alignment explicitly, however it might be changed from compiler settings and not be so obvious (although even problems caused by explicit change might look unobvious for people that are not aware of memory alignment requirement for atomic operations).

Code: [Select]

#include <iostream>
#include <thread>
#include <atomic>

void thread1(std::atomic_long& x)
{
for (int i = 0; i < 1000000; ++i)
{
x.store(0);
long value = x.load();
assert(value == 0 || value == ~0);
}
}

void thread2(std::atomic_long& x)
{
for (int i = 0; i < 1000000; ++i)
{
x.store(~0);
long value = x.load();
assert(value == 0 || value == ~0);
}
}

#pragma pack(push)
#pragma pack(1)
struct X
{
char alignment;
std::atomic_long x;
};
#pragma pack(pop)

int main()
{
X arr[100];
for (int i = 0; i < 100; ++i)
{
std::cout << i << std::endl;
arr[i].x = 0;
std::thread thread(thread1, std::ref(arr[i].x));
thread2(arr[i].x);
thread.join();
}
}

Thanks.

[Anthony Williams]

Are you suggesting putting assert(variable_correctly_aligned()); statements inside the atomic operations? If so, then that seems a reasonable idea.

[TA]

Yes. Alternatively you can do something like this

Code: [Select]

template <typename T>
class atomic_base {
   // T data;
   __declspec (align(4)) T data;
};


[Anthony Williams]

Yes, I guess you have a base class where the actual data is stored. The constructor might do that check. Alternatively you can do something like this

Code: [Select]

template <typename T>
class atomic_base {
   // T data;
   __declspec (align(4)) T data;
};


The code already does that, so everything will be correctly aligned by default. However, if you use #pragma pack(1) then you're deliberately asking the compiler to ignore such alignment specifications, so it doesn't help.

[TA]

Yes, I guess you have a base class where the actual data is stored. The constructor might do that check. Alternatively you can do something like this

Code: [Select]

template <typename T>
class atomic_base {
   // T data;
   __declspec (align(4)) T data;
};


The code already does that, so everything will be correctly aligned by default. However, if you use #pragma pack(1) then you're deliberately asking the compiler to ignore such alignment specifications, so it doesn't help.

hmmm..
#pragma pack affects default alignment, but it shouldn't affect explicit alignment.
This code outputs 8 for the size and true for is_aligned call on my machine inspite of #pragma pack(1).

Code: [Select]

#include <iostream>

#pragma pack(1)

struct X
{
char a;
__declspec (align(4)) long b;
};

bool is_aligned(void *ptr, int boundary)
{
return ((uintptr_t)ptr & (boundary - 1)) == 0;
}


int main()
{
X x;
std::cout << sizeof(X) << std::endl;
std::cout << is_aligned(&x.b, 4) << std::endl;
}
using this structure as amember of another unaligned structure will still keep our variable b properly aligned.
However, I'm not sure about gcc.

If this is not portable enough then assert is still good solution too.

[TA]

Another thing that can be done is putting atomic classes between

Code: [Select]

#pragma pack(push)
#pragma pack(4)
// class atomic ...
#pragma pack(pop)


[Anthony Williams]

hmmm..
#pragma pack affects default alignment, but it shouldn't affect explicit alignment.
This code outputs 8 for the size and true for is_aligned call on my machine inspite of #pragma pack(1).

Code: [Select]

#include <iostream>

#pragma pack(1)

struct X
{
char a;
__declspec (align(4)) long b;
};

bool is_aligned(void *ptr, int boundary)
{
return ((uintptr_t)ptr & (boundary - 1)) == 0;
}


int main()
{
X x;
std::cout << sizeof(X) << std::endl;
std::cout << is_aligned(&x.b, 4) << std::endl;
}
I'm not sure about gcc.
Anyway, if this is not portable enough then assert is good solution too.

MSVC is inconsistent: with #pragma pack(1) it obeys the alignment requirements sometimes, but not others.

gcc always ignores alignment requirements with #pragma pack(1).

I'll put asserts in the atomic ops.

[Anthony Williams]

Another thing that can be done is putting atomic classes between

Code: [Select]

#pragma pack(push)
#pragma pack(4)
// class atomic ...
#pragma pack(pop)


That wouldn't help: it just affects the layout of the atomic objects, not the alignment of the whole object.

[TA]

MSVC is inconsistent: with #pragma pack(1) it obeys the alignment requirements sometimes, but not others.

gcc always ignores alignment requirements with #pragma pack(1).

I'll put asserts in the atomic ops.

Thanks.

[TA]

Another thing that can be done is putting atomic classes between

Code: [Select]

#pragma pack(push)
#pragma pack(4)
// class atomic ...
#pragma pack(pop)


That wouldn't help: it just affects the layout of the atomic objects, not the alignment of the whole object.

Yes, that's right. Didn't think about that. Thank you.