Buid fails with -Werror=strict-aliasing (lto)

[orlitzky]

When using CFLAGS="-Werror=strict-aliasing, the build fails:

oleprop.cpp: In member function ‘virtual OLEProperty::operator FILETIME() const’:
oleprop.cpp:179:59: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
  179 | OLEProperty::operator FILETIME() const          { return *(FILETIME *)(&V_CY(&val)); }
      |                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~
oleprop.cpp:179:59: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
oleprop.cpp: In member function ‘virtual FILETIME& OLEProperty::operator=(const FILETIME&)’:
oleprop.cpp:260:99: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
  260 |  FILETIME& v) { Clear(); V_CY(&val) = *((CY *)&v); return *((FILETIME *)&V_CY(&val)); }
      |                                                            ~^~~~~~~~~~~~~~~~~~~~~~~~

olestrm.cpp: In member function ‘Boolean OLEStream::VTtoString(VARIANT*, char**)’:
olestrm.cpp:2228:13: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
 2228 |       li = *(LARGE_INTEGER *)&V_CY(variant);
olestrm.cpp:2228:13: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
olestrm.cpp:2238:13: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
 2238 |       li = *(LARGE_INTEGER *)&V_CY(variant);
olestrm.cpp:2238:13: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]

This was reported to us in Gentoo bug 859913.

Why would we enable this if it makes the build fail? Newer compilers are becoming more strict about what they will accept, and especially with link-time optimization (LTO), are beginning to make some more assumptions about the code. Gentoo is a source-based distribution and one of the main benefits of that is that users are free to enable link-time optimization themselves. But, as a safety net, it's a good idea to enable -Werror=strict-aliasing when doing so. You want the build to fail if the optimization pass might break the code; this prevents our users from winding up with a broken package installed.

[dlemstra]

We have change our build flags to -Wall -Wextra -Werror -Wno-error=deprecated-copy -Wno-error=class-memaccess and are not seeing this error. But feel free to open a PR to resolve this.

[orlitzky]

I was able to reproduce the problem with,

 $ export CXXFLAGS="$CXXFLAGS -Werror=strict-aliasing"
 $ ./configure && make

The first two strict-aliasing failures in ole/oleprop.cpp can be commented out, allowing you to proceed to the next (easier) set of strict-aliasing errors in ole/olestrm.cpp. This fails,

LARGE_INTEGER li;
li = *(LARGE_INTEGER *)&V_CY(variant);

which leads us to the definitions of LARGE_INTEGER and tagCY in oless/h/ref.hxx and oless/h/props.h respectively:

typedef struct _LARGE_INTEGER {
        DWORD LowPart;
        LONG  HighPart;
} LARGE_INTEGER

// FIXME: portability                                                           
typedef struct tagCY {
  #if BIGENDIAN
    int32_t Hi;
    uint32_t Lo;
  #else
    uint32_t Lo;
    int32_t Hi;
  #endif
    int64_t int64;
} CY;

While the types in the first two fields are compatible, the structs themselves are not, at least not in their entirety. I presume this is what the compiler is complaining about. In this case there's an easy workaround though:

// No!
// li = *(LARGE_INTEGER *)&V_CY(variant);

// Yes!
li.LowPart  = V_CY(variant).Lo;
li.HighPart = V_CY(variant).Hi;

But that brings us back to the first error in oleprop.cpp. The FILETIME struct is essentially the same as LARGE_INTEGER,

typedef struct FARSTRUCT tagFILETIME
{
        DWORD dwLowDateTime;
	DWORD dwHighDateTime;
} FILETIME;

and we have the same sort of problem on line 179 of ole/oleprop.cpp with the same sort of solution: create a new FILETIME, set the high/low fields, and then return it.

But on line 260 of ole/oleprop.cpp, we have

FILETIME& OLEProperty::operator=(const FILETIME& v) {
    Clear();
    V_CY(&val) = *((CY *)&v);
    return *((FILETIME *)&V_CY(&val));
}

I don't see an obvious solution to this one because we actually want to return a reference. And I've been away from C++ too long to know intuitively what happens when we cast a dereferenced pointer back to a reference. I guess it makes sense for chains of overloaded operators like x = y = z?

Anyway, the build succeeds with the two bad lines in ole/oleprop.cpp commented out. Maybe nobody cares about them at all and that's the fix :)