Skip to content

Fix invalid pointer casts in arm64 implementation #65

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 6, 2024
Merged

Fix invalid pointer casts in arm64 implementation #65

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0e4203
Fix invalid pointer casts in arm64 implementation
davidben Sep 6, 2024
1b4eb1a
Use crc32c_read_le.h instead
davidben Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 29 additions & 14 deletions src/crc32c_arm64.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <cstddef>
#include <cstdint>
#include <cstring>

#include "./crc32c_internal.h"
#include "crc32c/crc32c_config.h"
Expand All @@ -24,17 +25,31 @@
#define KBYTES 1032
#define SEGMENTBYTES 256

static uint64_t LoadU64(const uint8_t in[sizeof(uint64_t)]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use and extend crc32c_read_le.h?

I think that ReadUint32LE() and ReadUint64LE() there work as you need them to, and we checked that they're compiled as efficiently as memcpy on little-endian architectures.

You can add a ReadUint16LE() with tests, and then use it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice! Done. See commit message of 1b4eb1a for some notes on what I changed there.

uint64_t ret;
memcpy(&ret, in, sizeof(uint64_t));
return ret;
}

static uint32_t LoadU32(const uint8_t in[sizeof(uint32_t)]) {
uint32_t ret;
memcpy(&ret, in, sizeof(uint32_t));
return ret;
}

static uint16_t LoadU16(const uint8_t in[sizeof(uint16_t)]) {
uint16_t ret;
memcpy(&ret, in, sizeof(uint16_t));
return ret;
}

// compute 8bytes for each segment parallelly
#define CRC32C32BYTES(P, IND) \
do { \
crc1 = __crc32cd( \
crc1, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 1 + (IND))); \
crc2 = __crc32cd( \
crc2, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 2 + (IND))); \
crc3 = __crc32cd( \
crc3, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 3 + (IND))); \
crc0 = __crc32cd( \
crc0, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 0 + (IND))); \
#define CRC32C32BYTES(P, IND) \
do { \
crc1 = __crc32cd(crc1, LoadU64((P) + SEGMENTBYTES * 1 + (IND)*8)); \
crc2 = __crc32cd(crc2, LoadU64((P) + SEGMENTBYTES * 2 + (IND)*8)); \
crc3 = __crc32cd(crc3, LoadU64((P) + SEGMENTBYTES * 3 + (IND)*8)); \
crc0 = __crc32cd(crc0, LoadU64((P) + SEGMENTBYTES * 0 + (IND)*8)); \
} while (0);

// compute 8*8 bytes for each segment parallelly
Expand Down Expand Up @@ -86,7 +101,7 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) {
t2 = (uint64_t)vmull_p64(crc2, k2);
t1 = (uint64_t)vmull_p64(crc1, k1);
t0 = (uint64_t)vmull_p64(crc0, k0);
crc = __crc32cd(crc3, *(uint64_t *)data);
crc = __crc32cd(crc3, LoadU64(data));
data += sizeof(uint64_t);
crc ^= __crc32cd(0, t2);
crc ^= __crc32cd(0, t1);
Expand All @@ -96,18 +111,18 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) {
}

while (length >= 8) {
crc = __crc32cd(crc, *(uint64_t *)data);
crc = __crc32cd(crc, LoadU64(data));
data += 8;
length -= 8;
}

if (length & 4) {
crc = __crc32cw(crc, *(uint32_t *)data);
crc = __crc32cw(crc, LoadU32(data));
data += 4;
}

if (length & 2) {
crc = __crc32ch(crc, *(uint16_t *)data);
crc = __crc32ch(crc, LoadU16(data));
data += 2;
}

Expand Down