Experimental Physics and
Industrial Control System

Michael Davidsaver <[email protected]> · Thu, 30 Nov 2017 10:30:42 -0600

On 11/30/2017 06:33 AM, Dirk Zimoch wrote:
> On 29.11.2017 20:51, Michael Davidsaver wrote:
>> On 11/29/2017 12:43 PM, Dirk Zimoch wrote:
>>> The file anyscalar.h generates many warnings of the form
>>>
>>> /afs/psi.ch/group/8431/dirk/git/BASE/epics-base-master/include/pv/anyscalar.h:98: warning: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
>>
>> The construct used (std::aligned_storage) is well defined in c++11.
>>
>> http://en.cppreference.com/w/cpp/types/aligned_storage
>>
>>> I am a bit concerned about that because it may lead to wrong optimization. I think this should either be fixed or -fno-strict-aliasing should be added to the compiler flags for recent gccs (i.e. not old vxWorks :-) )
>>
>> GCC >=4.8.4 (at latest) doesn't warn about this even with -std=c++98.  So if anything, this is only need for older compilers.
>>
> 
> I checked the code and found that older compilers (those who complain) see a different piece of code. (#if __cplusplus>=201103L ... #else). And not all of them complain.

Not quite "older compilers", rather older language versions.  Building with "-std=c++98" sets the __cplusplus macro appropriately.
This is why I mention explicitly that gcc 4.8.4 doesn't complain even with "-std=c++98".

This isn't so surprising as the "compatibility code" is almost identical to the g++ implementation of std::aligned_storage.

>   template<std::size_t _Len, std::size_t _Align =
> 	   __alignof__(typename __aligned_storage_msa<_Len>::__type)>
>     struct aligned_storage
>     { 
>       union type
>       {
> 	unsigned char __data[_Len];
> 	struct __attribute__((__aligned__((_Align)))) { } __align; 
>       };
>     };

vs. my

>     struct wrap_t {
>         union blob_t {
>             char data[sizeof(std::string)];
>             double align_f; // assume std::string alignment <= 8
>         } blob[1];
>     } _wrap;

So the only difference is that I'm assuming (yes assuming) that alignment requirement of 'std::string' is not larger than 'double'.

> I will do some tests to find out if I can remove the warnings. If not, I will add the -fno-strict-aliasing compiler option to my architectures which use older compilers and to the vxWorks configuration in EPICS.

Fair enough for PSI, though to be clear I wouldn't want this in Base without some identified mis-behavior.
I've read a lot of people worrying theoretically about -fstrict-aliasing, but the only specific cases of actual mis-behavior
I can find have to do with cases where a function has two pointer arguments of different types,
which happen to point to the same memory.

so eg. a function like:

> unsigned fn1(dbCommon *prec, aiRecord *pai)
> {
>   prec->disv = 5;
>   return pai->disv;
> }

generates (gcc 6.3 w/ -O3 or -Os)

>        movl    $5, (%rdi)
>        movl    (%rsi), %eax

which happens to work, but when compared with

> int fn2(dbCommon *prec)
> {
>   aiRecord *pai = (aiRecord*)prec;
>   prec->disv = 5;
>   return pai->disv;
> }

which generates

>         movl    $5, (%rdi)
>         movl    $5, %eax

It's clear that the strict aliasing assumption is being made.

wrt. anyscalar, the potential problem I can see is the _as<>() method not being being inline'd.
This could might result in the compiler not realizing that the references returned
by eg. as<std::string>() and as<double>() are aliases of '_wrap.blob".

If you are inclined to look for places where -fno-strict-aliasing leads to different generated code,
then anyscalar::swap() is where I would start looking (see test_swap() in testanyscalar.cpp).

> But this is no showstopper for the release.
> 

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System