[issue30124] Fix C aliasing issue in Python/dtoa.c to use strict aliasing on Clang 4.0
Dimitry Andric added the comment: STINNER Victor writes: > Would it be technically possible to completely get ride of the union? Probably, it's just more work, and it has to be done pretty carefully to avoid regressions. It seems Python already does some exercising of these dtoa functions in its test suite, but ideally you would want to check against upstream's full tests, if those exist. There are probably many edge cases for a set of tricky functions like this... -- ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue30124> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue30124] Fix C aliasing issue in Python/dtoa.c to use strict aliasing on Clang 4.0
Dimitry Andric added the comment: There is a "#pragma clang optimize", but it only has the options "on" or "off", for specific source locations. I guess that would defeat the purpose a little bit. :( As an experiment, and to show what would be needed (at minimum), I have attempted to make Python/dtoa.c completely aliasing-safe here: https://github.com/DimitryAndric/cpython/commit/29c3f6f5cd771fce5630f127b9e7054593e3160c This allowed to remove the -fno-strict-aliasing flag again, and it succeeded all tests, even with clang 5.0.0. It basically replaces the direct union member accesses with getters and setters, which do the right thing for clang. Note that even though those getters and setters use memcpy(), this is actually completely optimized away in the resulting assembly. (Old compilers might not fare that well, though.) In any case, while these are mostly mechanical changes, it is still a lot of code churn, and it should really be reviewed by the original maintainer of dtoa, David M. Gay. I have no idea whether he is still active, though... -- ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue30124> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue30124] Fix C aliasing issue in Python/dtoa.c to use strict aliasing on Clang 4.0
Dimitry Andric added the comment:
Note that gcc has documented accessing union members in this way as an
"implementation defined" feature:
https://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html#Structures-unions-enumerations-and-bit-fields-implementation
They specifically mention an example that is very similar to the dtoa pattern
as being allowed under *their* implementation, here:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning
However, whether that means that it is generally allowed by all standards is
unfortunately still rather vague. I have seen more than one complaint that the
standards committees should make this very explicit, instead of weaseling it
into post-release footnotes.
That said, I'd like to reply to some other posters here.
Eric V. Smith writes:
> At most, I think Mark's idea to use -fno-strict-aliasing only on dtoa.c and
> nowhere else would be the better approach.
Indeed, this is exactly the solution I chose for FreeBSD. Just the one file
that needs it is compiled with -fno-strict-aliasing. The caveat is that this
might not work when link-time optimization is in effect.
Serhiy Storchaka writes:
> Could we use Clang specific pragma in dtoa.c rather than a compiler option?
Unfortunately, as far as I know, clang still does not support function-level
optimization pragmas. Maybe it was implemented recently, but then you would
still have to have a workaround for older versions.
STINNER Victor writes:
> If we decide to go for the -fno-strict-aliasing only for dtoa.c, I suggest to
> use it also for GCC. GCC might decide to also optimize dtoa.c further in the
> future.
Theoretically they could, but as I pointed out above, they explicitly document
this as a feature of their union implementation. I estimate the probability of
them dropping this in the future to be near zero.
> I don't think that the flag has a major impact on performance if it's
> restricted to dtoa.c, and it would simplify the build system to only have
> "per compiler" flags. (Ex: Does ICC also "miscompile" dtoa?)
Disabling strict aliasing for just that file, or even just the one function it
applies to (by splitting it off to a separate file, for instance) should not
result in different assembly output, unless the compiler is very old and/or
dumb.
> FreeBSD uses the following syntax to only add the flag on a specific C file.
> Does it work with GNU and BSD make? (is it a "portable" syntax?)
>
>CFLAGS.gdtoa_${src}+=-fno-strict-aliasing
No, this is specifically a feature of BSD's bsd.sys.mk infrastructure. It's
most likely not compatible with GNU make.
--
nosy: +dim
___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue30124>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue30104] Float rounding errors on AMD64 FreeBSD CURRENT Debug 3.x buildbot
Dimitry Andric added the comment: This is most likely the same issue we found in https://bugs.freebsd.org/216770, which was reported upstream to LLVM here: https://bugs.llvm.org//show_bug.cgi?id=31928, and resulted in a very long and not really productive discussion about whether type punning in this way is officially allowed, or a GNU extension. I will gladly leave it to language lawyers. :) In our case, easy fix was to use -fno-strict-aliasing, as we did here: https://svnweb.freebsd.org/changeset/base/313706 -- nosy: +dim ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue30104> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10910] pyport.h FreeBSD/Mac OS X "fix" causes errors in C++ compilation
Dimitry Andric added the comment: I am a FreeBSD committer, and I recently ran into this issue too, since I am working on an update of libc++ in the FreeBSD base system. As part of this work, we attempt to recompile all ports with the proposed change (see [1]). During such a recompile, we get many errors in ports that include pyport.h in C++ mode, similar to (see [2]): In file included from scipy/interpolate/src/_interpolate.cpp:4: In file included from scipy/interpolate/src/interpolate.h:3: In file included from /usr/include/c++/v1/iostream:38: In file included from /usr/include/c++/v1/ios:216: /usr/include/c++/v1/__locale:468:15: error: C++ requires a type specifier for all declarations char_type toupper(char_type __c) const ^ /usr/local/include/python2.7/pyport.h:731:29: note: expanded from macro 'toupper' #define toupper(c) towupper(btowc(c)) ^ I think Ronald's proposed workaround is fine, since messing with ctype macros is risky in C++ mode. E.g. the libc++ header explicitly undefines all ctype macros, and replaces them with inline functions. Additionally, the original issue mentioned in changeset 32950 [3] was fixed in October 2007 for FreeBSD 8.x [4], and subsequently the fix was merged back to FreeBSD 6.x and 7.x [5]. I'm adding an additional diff that corrects the __FreeBSD_version number checks. This will also be submitted to the FreeBSD ports bug tracker. [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208158 [2] http://package18.nyi.freebsd.org/data/headamd64PR208158-default/2016-03-22_18h30m05s/logs/errors/py27-scipy-0.16.1.log [3] https://hg.python.org/cpython/rev/adfe7d39a049 [4] https://svnweb.freebsd.org/base?view=revision=172619 [5] https://svnweb.freebsd.org/base?view=revision=172929 -- nosy: +dim Added file: http://bugs.python.org/file42359/issue10910-fix-versionchecks-1.diff ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue10910> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
