strict-aliasing rules build failures on some platforms

[Thomas Klausner]

I've tried compiling xpra-0.16.0 with python-2.7.11 on NetBSD, but it fails for me with:

gcc -DNDEBUG -O2 -D_FORTIFY_SOURCE=2 -fstack-protector -DHAVE_DB_185_H -pthread -I/usr/include -I/usr/pkg/include -O2 -D_FORTIFY_SOURCE=2 -fstack-protector -I/usr/pkg/include -I/usr/include -I/usr/pkg/include/ffmpeg1 -I/usr/pkg/include/freetype2 -I/usr/pkg/include/libdrm -I/usr/pkg/include -I/usr/include -I/usr/pkg/include/ffmpeg1 -I/usr/pkg/include/freetype2 -I/usr/pkg/include/libdrm -fPIC -I/usr/pkg/include -I/usr/pkg/include/python2.7 -c xpra/x11/bindings/randr_bindings.c -o build/temp.netbsd-7.99.25-amd64-2.7/xpra/x11/bindings/randr_bindings.o -Wall -Werror -fPIC
xpra/x11/bindings/randr_bindings.c: In function '__pyx_pf_4xpra_3x11_8bindings_14randr_bindings_13RandRBindings_4check_randr':
xpra/x11/bindings/randr_bindings.c:1140:9: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
         __Pyx_INCREF(Py_True);
         ^
xpra/x11/bindings/randr_bindings.c:1179:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   __Pyx_INCREF(Py_False);
   ^
xpra/x11/bindings/randr_bindings.c: In function '__pyx_pf_4xpra_3x11_8bindings_14randr_bindings_13RandRBindings_6has_randr':
xpra/x11/bindings/randr_bindings.c:1250:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   __pyx_t_1 = __Pyx_PyBool_FromLong((!(!__pyx_t_2))); if (unlikely(!__pyx_t_1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 98; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
   ^
xpra/x11/bindings/randr_bindings.c:1250:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
xpra/x11/bindings/randr_bindings.c: In function '__pyx_f_4xpra_3x11_8bindings_14randr_bindings_13RandRBindings__set_screen_size':
xpra/x11/bindings/randr_bindings.c:1774:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
       __Pyx_INCREF(Py_False);
       ^
xpra/x11/bindings/randr_bindings.c:1867:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
       __Pyx_INCREF(Py_False);
       ^
xpra/x11/bindings/randr_bindings.c:1888:5: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
     __Pyx_INCREF(Py_True);
     ^
cc1: all warnings being treated as errors
error: command 'gcc' failed with exit status 1
*** Error code 1

That's when building against the gtk2 backend because I don't know how to build against the gtk3 one instead.

[Antoine Martin]

silence the problematic warnings as per cython ticket

[Antoine Martin]

Someone else reported this but I have never seen it.
Which gcc and Cython version are you using?

You may want to add -fno-strict-aliasing to ignore this problem as per ​http://trac.cython.org/ticket/395 and ​Strict aliasing warnings on generated code.

Does the patch attached fix things? (you may need to edit the gcc version check)

[Thomas Klausner]

I'm using gcc-4.8.5 (nb3 20151015) on NetBSD-7.99.25/amd64 with python-2.7.11 and cython 0.23.4.

I tried your patch and changed 5,2 to 4,8, but I still see the same failure; the additional flag does not end up on the relevant command line.

[Antoine Martin]

Yeah, the patch is wrong I think, try moving it above the check for 4.4, or merging it into that section instead.

[Antoine Martin]

better patch - checks for netbsd

[Thomas Klausner]

I've tested your netbsd-patch and it works for me. Thank you!

I don't think that testing for NetBSD is right though, it's a compiler (flags) issue, not one of the operating system.

[Antoine Martin]

Merged in r11614, if and when others report what platforms and toolchains hit this strict aliasing warning, we can change the platform check to something else. Closing.

[Antony Lee]

Kindly requesting this to be reopened, I also see this issue with Python 2.7.11, Cython 0.23.4, gcc 5.3.0 on Arch Linux (standard packages) with a current svn checkout.

[Antoine Martin]

I have gcc 5.3.1, cython 0.23.4, Python 2.7.11 and I am not seeing this warning:

gcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions \
    -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 \
    -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv \
    -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions \
    -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 \
    -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv \
    -fPIC -I/usr/include/python2.7 -c xpra/x11/bindings/randr_bindings.c \
    -o build/temp.linux-x86_64-2.7/xpra/x11/bindings/randr_bindings.o -Wall -Werror -fPIC

Just to be clear: this code is generated by Cython, and looks correct too.
So.. until this warning starts showing up in Fedora / Debian, I will not change the code to accommodate Arch Linux, I can only suggest that you patch the build file to silence the warning when building on Arch.
Unless someone can come up with a way to detect the systems that will barf on this and turn off the warning only when needed. (but not using autoconf!)

[Antoine Martin]

(editing ticket title)

[Antony Lee]

<edited out> sorry, silly misread.
I edited the Arch Linux build file to solve the issue.

[Antoine Martin]

I edited the Arch Linux build file to solve the issue.

Can I close this ticket?
Can you please link to the change you made?

My understanding is that python is meant to be built with -fno-strict-aliasing, and the cython extensions should use the same flags. I would expect this to work out of the box without us tweaking our build file (and I am tempted to undo the change in r11614 too)

[Antony Lee]

The Arch Linux build file for xpra/svn is
​https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=xpra-winswitch-svn
(format should be self-evident)

The build file for the latest xpra release is
​https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=xpra-winswitch

so I just added CFLAGS=... to the first one.
I agree that Python2 is "known" to require -fno-strict-aliasing (​http://legacy.python.org/dev/peps/pep-3123/) so you can assume it's there. Feel free to close this.

[Antoine Martin]

Thanks!

Note to _wiz_: please see the comments above, I may remove the netbsd hack, please update your cflags.

[Thomas Klausner]

I'm confused about who is supposed to set -fno-strict-aliasing.
If it is a known problem that a part of xpra won't build without that flag, why doesn't xpra set it?

This should at least be mentioned in a README...

[Antoine Martin]

_wiz_: The cython extensions need to be built with the same flags as python, every other distro gets it right without us guessing what flags should or should not be used.

[Eli Schwartz]

This happens because ​https://bugs.python.org/issue969718

Building any python extension when the CFLAGS are set from the environment will completely ignore the CFLAGS python itself was built with.

Fedora locally patches this:
​https://src.fedoraproject.org/rpms/python2/blob/master/f/00168-distutils-cflags.patch

Debian locally patches this too:
​https://bazaar.launchpad.net/~doko/python/pkg2.7-debian/view/head:/patches/distutils-sysconfig.diff

Arch Linux does not, and possibly many other distros. People installing python directly from source will certainly not have these patches.

I think the best idea is to take the value of

import sysconfig
sysconfig.get_config_var('CFLAGS')

and add it to the CFLAGS used in your setup.py until the PSF can fix this 13-year-old bug.

[Antoine Martin]

@eschwartz: does this patch work for you?

Index: setup.py
===================================================================
--- setup.py	(revision 19079)
+++ setup.py	(working copy)
@@ -624,6 +624,14 @@
     ignored_flags = kw.pop("ignored_flags", [])
     ignored_tokens = kw.pop("ignored_tokens", [])
 
+    #for distros that don't patch distutils,
+    #we have to add the python cflags:
+    if not (is_Fedora() or is_Debian() or is_CentOS()):
+        import shlex
+        import sysconfig
+        for x in shlex.split(sysconfig.get_config_var('CFLAGS') or ''):
+            add_to_keywords(kw, 'extra_compile_args', x)
+
     if len(pkgs_options)>0:
         package_names = []
         #find out which package name to use from potentially many options

[Eli Schwartz]

Original:

gcc -pthread -DNDEBUG -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -march=native -ggdb -D_FORTIFY_SOURCE=2 -fPIC -Ixpra/buffers -I/usr/include/python2.7 -c xpra/buffers/membuf.c -o build/temp.linux-x86_64-2.7/xpra/buffers/membuf.o -O3 -Wall -Werror -fPIC

With the patch:

Traceback (most recent call last):
  File "setup.py", line 1672, in <module>
    buffers_pkgconfig = pkgconfig(optimize=3)
  File "setup.py", line 628, in exec_pkgconfig
    if not (is_Fedora() or is_Debian() or is_CentOS()):
NameError: global name 'is_Fedora' is not defined

Note I'm using the PKGBUILD for the stable 2.2.6 release, so possibly this has been added since then?
However... it shouldn't hurt to add these twice to the gcc command line, except for making the build logs somewhat messier. Up to you if you want to code special detection logic for that use case.

Unconditionally setting it:

gcc -pthread -DNDEBUG -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -march=native -ggdb -D_FORTIFY_SOURCE=2 -fPIC -Ixpra/buffers -I/usr/include/python2.7 -c xpra/buffers/membuf.c -o build/temp.linux-x86_64-2.7/xpra/buffers/membuf.o -O3 -fno-strict-aliasing -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -DNDEBUG -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -Wall -Werror -fPIC

This "works" in the sense of preventing compiler errors, but only by clobbering the environment CFLAGS with the python CFLAGS. The options need to be set *before* the environment CFLAGS, to ensure they are properly overridden by anything the user explicitly sets. e.g. -march=native.

[Antoine Martin]

Note I'm using the PKGBUILD for the stable 2.2.6 release, so possibly this has been added since then?

All development is always done against trunk.
In this case, for the 2.2.x branch, you can use add from xpra.os_util import is_Fedora above the if statement.

[Giancarlo Razzolini]

Replying to Antoine Martin:

Note I'm using the PKGBUILD for the stable 2.2.6 release, so possibly this has been added since then?

All development is always done against trunk.
In this case, for the 2.2.x branch, you can use add from xpra.os_util import is_Fedora above the if statement.

I'm a developer on Arch Linux and I'm trying to bring xpra from our unofficial repository:

​https://aur.archlinux.org/packages/xpra/

To the main repos. I have based my PKGBUILD on that one and I'm currently building it like this:

​https://paste.xinu.at/ZGcX32/

Eli was kind to help me review this and this is the reason why he re-opened this issue.

[bug]

The patch mentioned above works on trunk and 2.3.

[Antoine Martin]

The patch mentioned above works on trunk and 2.3.

Thanks, applied in r19327. (will backport to 2.3)