[Python-Dev] bpo-28055: Fix unaligned accesses in siphash24(). (GH-6123)
?On 5/13/18, 12:44 PM, "Python-Dev on behalf of Christian Heimes" <python-dev-bounces+robb=datalogics.com at python.org on behalf of christian at python.org> wrote:
On 2018-05-13 06:57, Serhiy Storchaka wrote:
> commit: 1e2ec8a996daec65d8d5a3d43b66a9909c6d0653
> branch: master
> author: Rolf Eike Beer <eike at sf-mail.de>
> committer: Serhiy Storchaka <storchaka at gmail.com>
> date: 2018-05-13T13:57:31+03:00
> bpo-28055: Fix unaligned accesses in siphash24(). (GH-6123)
> The hash implementation casts the input pointer to uint64_t* and directly reads
> from this, which may cause unaligned accesses. Use memcpy() instead so this code
> will not crash with SIGBUS on sparc.
> A Misc/NEWS.d/next/Core and Builtins/2018-04-25-20-44-42.bpo-28055.f49kfC.rst
> M Python/pyhash.c
I was against the approach a good reason. The PR adds additional CPU
instructions and changes memory access pattern in a critical path of
CPython. There is no reason to add additional overhead for the majority
of users or X86 and X86_64 architectures. The memcpy() call should only
be used on architectures that do not support unaligned memory access.
See comment https://bugs.python.org/issue28055#msg276257
X86 won't *directly* write misaligned data either, it will intrinsically copy it out to a properly aligned location. In C this is also "undefined behavior", so technically the C implementation can do whatever it wants - like raise an exception - which is will on SPARC. While X86 users may not notice any problems, depending on undefined behavior working in any particular way has many drawbacks. Often C compilers will optimize code in ways that assume there is no undefined behavior in ways that breaks code that does.
At least for latest GCC, the change seems to be fine. GCC emits the same
assembly code for X86_64 before and after your change. Did you check the
output on other CPU architectures as well as clang and MSVC, too?
Python-Dev mailing list
Python-Dev at python.org