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

Fix unaligned read in fun.c siphash #4603

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dipinhora
Copy link
Contributor

ubsan doesn't like it when unaligned reads happen. this commit fixes things to make ubsan happy.

NOTE: fix uses memcpy which at least some compilers optimize well.
See: https://blog.quarkslab.com/unaligned-accesses-in-cc-what-why-and-solutions-to-do-it-properly.html

Makes ubsan runtime errors such as the following go away:

runtime error: load of misaligned address 0xb2af29174143 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 6, 2025
@SeanTAllen SeanTAllen changed the title ubsan: fix unaligned read in fun.c siphash Fix unaligned read in fun.c siphash Feb 6, 2025
@dipinhora
Copy link
Contributor Author

the cycle-detector test timed out in the use runtimestats build..

can someone poke at it to run again?

@SeanTAllen
Copy link
Member

@dipinhora rebase against main. that will cause a new run and it will pick up your cycle detector test change.

@dipinhora dipinhora force-pushed the happyubsan_unaligned_read branch from aa98996 to 7734a55 Compare February 6, 2025 15:41
@dipinhora
Copy link
Contributor Author

@dipinhora rebase against main. that will cause a new run and it will pick up your cycle detector test change.

but i was being lazy.. 8*/

.

.

.

.

.

done..

@dipinhora dipinhora force-pushed the happyubsan_unaligned_read branch from 7734a55 to 116a7cd Compare February 8, 2025 06:26
ubsan doesn't like it when unaligned reads happen. this commit fixes
things to make ubsan happy.

NOTE: fix uses `memcpy` which at least some compilers optimize well.
See: https://blog.quarkslab.com/unaligned-accesses-in-cc-what-why-and-solutions-to-do-it-properly.html

Makes ubsan runtime errors such as the following go away:

`runtime error: load of misaligned address 0xb2af29174143 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment`
@dipinhora dipinhora force-pushed the happyubsan_unaligned_read branch from 116a7cd to fc768c1 Compare February 10, 2025 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss during sync Should be discussed during an upcoming sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants