GCC 4.8.4 4.9.2 nifti_swap_2bytes optimization bug

[Martin Sebor]

On 04/01/2015 08:49 AM, Andrew Haley wrote:
> On 04/01/2015 03:36 PM, Andreas Schuh wrote:
>
>> I believe I have encountered a bug in the optimization at least in
>> versions 4.8.4 and 4.9.2. The problem is not present in version
>> 4.6.3. The attached complete example code can be used to reproduce
>> the error (my system is Ubuntu 12.04). This code is crucial in
>> medical image processing where the NIfTI-1 file format is commonly
>> used. The respective function that is optimized incorrectly by most
>> recent C/C++ compilers of GCC was copied from the NIfTI-1 C library
>> and is used by many medical image processing tools. It is
>> responsible for swapping the bytes if necessary when the image data
>> was written on a system with differing endianness.
>
> Sure, but it's not standard C.  You really don't need to alias a
> struct of two bytes and a short, and these are not compatible types.
> (I can give you chapter and verse of the C standard if you really want
> it, but the rule is simple: don't expect a cast between pointers to
> different types to work.  Doing so via a hidden void* doesn't help.)

The C requirement is that "An object shall have its stored value
accessed only by an lvalue expression of ... a character type."
The original program accesses the value object via the tb[ii].a
and tb[ii].b expressions, which are both unsigned char lvalues.
That the program derives those lvalues from pointers to some
other type (*) doesn't violate the aliasing rule.

[*] As if by:

     unsigned char *b0 = &((twobytes*)ar + ii)->a;
     unsigned char *b1 = &((twobytes*)ar + ii)->b;
     tt = *b0;
     *b0 = *b1;
     *b1 = tt;

Martin

>
> It'd be perfectly legal to do this swap with an array of bytes,
> because byte arrays can alias anything.
>
> Like this:
>
>     char *p = ar;
>     for( ii=0 ; ii < 2*n ; ii +=2 ){
>       tt = p[ii]; p[ii] = p[ii+1] ; p[ii+1] = tt ;
>     }
>
> If you really need to compile stuff like this you can use
> -fno-strict-aliasing, but I'd fix the code if I were you.
>
> Andrew.
>