#1080 closed defect (fixed)
strict-aliasing rules build failures on some platforms
Reported by: | Thomas Klausner | Owned by: | Eli Schwartz |
---|---|---|---|
Priority: | major | Milestone: | 0.17 |
Component: | core | Version: | 0.16.x |
Keywords: | Cc: | eschwartz@… |
Description
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.
Attachments (2)
Change History (26)
Changed 6 years ago by
Attachment: | silence-cython-strict-aliasing-warning.patch added |
---|
comment:1 Changed 6 years ago by
Owner: | changed from Antoine Martin to Thomas Klausner |
---|
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)
comment:2 Changed 6 years ago by
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.
comment:3 Changed 6 years ago by
Yeah, the patch is wrong I think, try moving it above the check for 4.4, or merging it into that section instead.
Changed 6 years ago by
Attachment: | silence-cython-strict-aliasing-warning.2.patch added |
---|
better patch - checks for netbsd
comment:4 Changed 6 years ago by
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.
comment:5 Changed 6 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
comment:6 Changed 6 years ago by
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.
comment:7 Changed 6 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:8 Changed 6 years ago by
Owner: | changed from Thomas Klausner to Antony Lee |
---|---|
Status: | reopened → new |
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!)
comment:9 Changed 6 years ago by
Summary: | xpra-0.16.0 does not build → strict-aliasing rules build failures on some platforms |
---|
(editing ticket title)
comment:10 Changed 6 years ago by
<edited out> sorry, silly misread.
I edited the Arch Linux build file to solve the issue.
comment:11 Changed 6 years ago by
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)
comment:12 Changed 6 years ago by
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.
comment:13 Changed 6 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Thanks!
Note to _wiz_: please see the comments above, I may remove the netbsd hack, please update your cflags.
comment:14 Changed 6 years ago by
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...
comment:15 Changed 6 years ago by
_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.
comment:16 Changed 4 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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.
comment:17 Changed 4 years ago by
Cc: | eschwartz@… added |
---|
comment:18 Changed 4 years ago by
Owner: | changed from Antony Lee to Eli Schwartz |
---|---|
Status: | reopened → new |
@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
comment:19 Changed 4 years ago by
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.
comment:20 follow-up: 21 Changed 4 years ago by
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.
comment:21 Changed 4 years ago by
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 addfrom 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.
comment:23 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
The patch mentioned above works on trunk and 2.3.
Thanks, applied in r19327. (will backport to 2.3)
comment:24 Changed 16 months ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1080
silence the problematic warnings as per cython ticket