Software crc32c produces wrong results on big endian CPUs about crc32c HOT 7 CLOSED

I ran all the tests on my side and looks great.

from crc32c.

I am pretty sure the issue comes from some edge case with endianess. PowerPC has 3 different architectures:

• PowerPC, big endian 32 bit
• PowerPC 64, big endian 32 bit
• PowerPC 64le, little endian 64 bit and 64

Tests against PowerPC 64le are passing:

❯❯❯ ~/github/crc32c ❯ cross test --target powerpc64le-unknown-linux-gnu  
warning: unused manifest key: target.aarch64-unknown-linux-gnu.linker
    Finished test [unoptimized + debuginfo] target(s) in 0.16s
     Running unittests src/lib.rs (/target/powerpc64le-unknown-linux-gnu/debug/deps/crc32c-27d21e5a67b97edc)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/simple.rs (/target/powerpc64le-unknown-linux-gnu/debug/deps/simple-5fe61d7c8ac92f28)

running 6 tests
test crc ... ok
test crc_append ... ok
test crc_combine ... ok
test long_string ... ok
test very_big ... ok
test very_small ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

from crc32c.

might the issue be here?:

util::split takes a &[u8], and one of its return values is a &[u64], meaning it's transmuting the two without considering endianness.

edit: (this is called in the software impl here:

from crc32c.

Hum this is a good hunch.. Writing unsafe rust is hard :) I am not sure how to try to fix this thou.. There are other people lamenting that mem::transmute is of course not doing the right thing on BE: https://users.rust-lang.org/t/std-mem-transmute-returns-different-values-between-architectures/1589

from crc32c.

if this is the case, then some change needs to be made either in this function or at callsites (or both).
u64::from_le may be useful here; if callsites using this slice, whenever they need elements, pass them through that, it may be correct?

expanding further on the previous points, util::split could return &[U64Le] (fixing this on the type-level) so it wouldn't have to be a manual conversion:

#[repr(transparent)]
#[derive(Clone, Copy)]
struct U64Le(u64);

impl U64Le {
    #[inline]
    pub const fn get(self) -> u64 {
        u64::from_le(self.0)
    }
}

which would be safe to transmute to in the original function because it has the same layout as a u64 with the repr(transparent).

from crc32c.

im trying this fix out, could you see if it works correctly on those big endian targets?
link: https://github.com/nissaofthesea/crc32c/tree/231fed8cb605ae4db02ab91ac164400ccf09a6ad

EDIT: didn't see you used cross when i wrote this, so i installed that and tested it on this commit.

$ cross test --target powerpc-unknown-linux-gnu
warning: unused manifest key: target.aarch64-unknown-linux-gnu.linker
    Finished test [unoptimized + debuginfo] target(s) in 0.09s
     Running unittests src/lib.rs (/target/powerpc-unknown-linux-gnu/debug/deps/crc32c-52b9b02f194c0936)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/simple.rs (/target/powerpc-unknown-linux-gnu/debug/deps/simple-0ffe959d1ae05e6c)

running 6 tests
test crc ... ok
test crc_append ... ok
test crc_combine ... ok
test long_string ... ok
test very_big ... ok
test very_small ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s

seems to work, nice! i'll open a pull request then

from crc32c.

This is neat. Tomorrow I'll rerun all the tests I have on my side!

from crc32c.