Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARMv6 unaligned hack ends up in bus-error if running under a 64bit kernel #2632

Closed
bugfinder opened this issue May 11, 2021 · 2 comments · Fixed by #2633
Closed

ARMv6 unaligned hack ends up in bus-error if running under a 64bit kernel #2632

bugfinder opened this issue May 11, 2021 · 2 comments · Fixed by #2633

Comments

@bugfinder
Copy link

the comment in the source code already points out that the unaligned access violates the C standard.
just for reference if you run this code with a 32bit userspace on a 64bit armv8/aarch64 kernel the unaligned access is no longer caught and handled by the kernel but leads directly to a bus error.
not sure if this could/should make it into the documentation somewhere

@bmwiedemann
Copy link
Contributor

https://github.com/facebook/zstd/blob/v1.4.9/lib/common/mem.h#L139 to be precise with the special ARMV6 default of 2 below.

@bmwiedemann
Copy link
Contributor

related http://fastcompression.blogspot.com/2015/08/accessing-unaligned-memory.html says

Of course, a better solution would be for all compilers, and gcc specifically, to properly translate memcpy() into efficient assembly for all targets. But that's wishful thinking, clearly outside of our responsibility. Even if it does improve some day, we nonetheless need an efficient solution now, for current crop of compilers.

bmwiedemann added a commit to bmwiedemann/zstd that referenced this issue May 11, 2021
When running armv6 userspace on armv8 hardware with a 64 bit Linux kernel,
the mode 2 caused SIGBUS (unaligned memory access).
Running all our arm builds in the build farm
only on armv8 simplifies administration a lot.

Depending on compiler and environment, this change might slow down
memory accesses (did not benchmark it). The original analysis is 6 years old.

Fixes facebook#2632
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants