Re: [boost] [lockfree] _ENABLE_ATOMIC_ALIGNMENT_FIX for VS 2015 Update 2

Subject: Re: [boost] [lockfree] _ENABLE_ATOMIC_ALIGNMENT_FIX for VS 2015 Update 2
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2016-05-10 03:39:23

On Tuesday, 10 May 2016 10:26:15 MSK Stephan T. Lavavej wrote:
> Hi,
> 
> While running Boost.Lockfree's tests, one of our compiler devs encountered
> the following failure:
> 
> 
> freelist_test.cpp
> c:\wcfb01\binaries\x86chk\inc\atomic(659): error C2338: You've instantiated
> std::atomic<T> with sizeof(T) equal to 2/4/8 and alignof(T) < sizeof(T).
> Before VS 2015 Update 2, this would have misbehaved at runtime. VS 2015
> Update 2 was fixed to handle this correctly, but the fix inherently changes
> layout and breaks binary compatibility. Please define
> _ENABLE_ATOMIC_ALIGNMENT_FIX to acknowledge that you understand this, and
> that everything you're linking has been compiled with VS 2015 Update 2 (or
> later). c:\wcfb01\binaries\x86chk\inc\atomic(669): note: see reference to
> class template instantiation 'std::_Atomic_base<_Ty,4>' being compiled with
>         [
>             _Ty=boost::lockfree::detail::tagged_index
>         ]
> e:\boost\boost\boost\lockfree\detail\freelist.hpp(608): note: see reference
> to class template instantiation
> 'std::atomic<boost::lockfree::detail::tagged_index>' being compiled
> e:\boost\boost\boost\lockfree\detail\freelist.hpp(609): note: see reference
> to class template instantiation
> 'boost::lockfree::detail::fixed_size_freelist<T,NodeStorage>' being
> compiled
> 
>     call "C:\WCFB01\binaries\x86chk\bin\i386\vcvarsall.bat" x86 >nul
> cl /Zm800 -nologo
> @"..\bin.v2\libs\lockfree\test\freelist_test.test\msvc-latest\debug\threadi
> ng-multi\freelist_test.obj.rsp"
> 
> ...failed compile-c-c++
> ..\bin.v2\libs\lockfree\test\freelist_test.test\msvc-latest\debug\threading
> -multi\freelist_test.obj...
> 
> 
> with the following type:
> 
> 
> class tagged_index
> {
> public:
>     typedef boost::uint16_t tag_t;
>     typedef boost::uint16_t index_t;
> 
> protected:
>     index_t index;
>     tag_t tag;
> };
> 
> 
> This is by design - your tagged_index has align 2, size 4, triggering the
> need for my atomic alignment fix.  As the static_assert advises, your code
> needs to define _ENABLE_ATOMIC_ALIGNMENT_FIX in order to pass.
> 
> If this has already been fixed in 1.61.0, then that's great (I haven't
> checked).

I think, it's not the library's prerogative to define such macros as this can affect user's 
code. I.e. the library cannot tell if breaking binary compatibility is ok for users. The code 
could have worked previously, if the atomic<> value was suitably aligned because of the 
surrounding code or natural alignment of the memory allocator or if the user specified 
alignment manually. I guess the test could be fixed by defining the macro for the test, 
but this won't fix other uses of the library.

IMHO, it should be fixed in MSVC. The error should be opt-in, probably with an option of 
being a warning instead of an error. Or (more preferably) just fix the bug by default, with 
no errors or warnings, and provide a way to revert to the old behavior on request.