Open
Conversation
* Fix invalid pointer casts in arm64 implementation
It is UB in C and C++ to type-pun pointers in this way, for two reasons.
First, casting to a pointer is forbidden if the pointer is not aligned.
Second, this kind of type-punning is a strict aliasing violation.
Instead, the way to read 8 bytes as a time as a uint64_t is to call
memcpy. Compilers recognize this pattern and optimize it, lowering it to
the same code, but without breaking the language's abstract state
machine.
This is needed to fix some UBSan warnings in Chromium.
* Use crc32c_read_le.h instead
In doing so, I fixed up crc32c_read_le.h a bit:
1. The documentation and tests say the buffers need to be aligned, but
they don't.
2. Add a 16-bit version.
3. Remove an unnecessary static_cast<uint8_t>. The pointer is already
uint8_t.
4. Replace the necessary static_casts with uintN_t{x} per the Google
style guide. https://google.github.io/styleguide/cppguide.html#Casting
This reverts commit 1ac401e.
Member
|
Want to update this to also pull: google#68. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A few more changes have been pulled from upstream:
The google#65 provides an alternative implementation to our #6, so the latter has been reverted.
Additionally, there is a conflict with our a3f2c1c. Should we keep it, or can it be reverted to resolve the conflict?