zowens/crc32c

Software crc32c produces wrong results on big endian CPUs

Closed this issue · 7 comments

I am trying to run some code which uses crc32c on powerpc 32 and 64 bit and I noticed it produces incorrect results.

I have tried to run the test suite of this crate using cross and they indeed fail:

❯❯❯ ~/github/crc32c ❯ 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.15s
     Running unittests src/lib.rs (/target/powerpc-unknown-linux-gnu/debug/deps/crc32c-06d60a5bbd004a3e)

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-63febc81630ac4ff)

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

failures:

---- crc_combine stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `689410915`,
 right: `3475076554`', tests/simple.rs:25:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- long_string stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `550182489`,
 right: `372323125`', tests/simple.rs:55:5

---- very_big stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `314413457`,
 right: `1410821608`', tests/simple.rs:63:5


failures:
    crc_combine
    long_string
    very_big

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

error: test failed, to rerun pass '--test simple'
❯❯❯ ~/github/crc32c ❯ cross test --target powerpc64-unknown-linux-gnu
warning: unused manifest key: target.aarch64-unknown-linux-gnu.linker
    Finished test [unoptimized + debuginfo] target(s) in 0.14s
     Running unittests src/lib.rs (/target/powerpc64-unknown-linux-gnu/debug/deps/crc32c-d3c4704dfac2e462)

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/powerpc64-unknown-linux-gnu/debug/deps/simple-7936969e39a66f78)

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

failures:

---- crc_combine stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `2861399345`,
 right: `3645618169`', tests/simple.rs:25:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- long_string stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `550182489`,
 right: `372323125`', tests/simple.rs:55:5

---- very_big stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `314413457`,
 right: `1410821608`', tests/simple.rs:63:5


failures:
    crc_combine
    long_string
    very_big

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

error: test failed, to rerun pass '--test simple'

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

might the issue be here?:

let ptr = mem::transmute(mid.as_ptr());

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:

let (start, mid, end) = util::split(buffer);
)

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

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).

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

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

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