[Python-Dev] bpo-28055: Fix unaligned accesses in siphash24(). (GH-6123)

[Christian Heimes]

On 2018-05-13 06:57, Serhiy Storchaka wrote:
> https://github.com/python/cpython/commit/1e2ec8a996daec65d8d5a3d43b66a9909c6d0653
> 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
> summary:
> 
> 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.
> 
> https://bugs.gentoo.org/show_bug.cgi?id=636400
> 
> files:
> A Misc/NEWS.d/next/Core and Builtins/2018-04-25-20-44-42.bpo-28055.f49kfC.rst
> M Python/pyhash.c

Hi Serhiy,

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

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?

Christian