libnxz / power-gzip

POWER NX zlib compliant library
23 stars 18 forks source link

Add support for powerpc64-linux-gnu #191

Open ghost opened 2 years ago

ghost commented 2 years ago

Running

./configure
make

fails with

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I../inc_nx -DZLIB_PATH=\"libz.so\" -O3 -Wall -Werror -g -mcpu=power9 -MT gzip_vas.lo -MD -MP -MF .deps/gzip_vas.Tpo -c gzip_vas.c  -fPIC -DPIC -o .libs/gzip_vas.o
In file included from gzip_vas.c:68:
nx_zlib.h:132:32: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘OF’
  132 | extern const char *zlibVersion OF((void));
      |                                ^~

This is on a big endian system. I'm able to build this fine under a little endian system. If I add #define OF(x) () to various places to get the build to succeed then some of the tests fail.

tuliom commented 2 years ago

To the best of my knowledge, nobody has ever tested this project on a powerpc64-linux system. @amock do you have /dev/crypto/nx-gzip in this big endian machine?

ghost commented 2 years ago

@tuliom I do have /dev/crypto/nx-gzip and I'm running kernel 5.18.

Do you know where that macro is supposed to come from? I would at least expect the code to compile successfully on a big endian machine.

ghost commented 2 years ago

I found that test_crc32 fails when more than 30 bytes are processed, which lead me to the line https://github.com/libnxz/power-gzip/blob/e1d0b1c7b0df47acdbcc8e3c5fb0ca4d9f28f018/lib/crc32_ppc.c#L70 so __crc32_vpmsum is broken. For some tests I tried (strings of 'a' of length up to 100) changing https://github.com/libnxz/power-gzip/blob/b589fb633e2b13eb1f2429ca2853ca87c2747c5f/lib/crc32_power.c#L506 to

return v0[0];

makes the tests pass. That doesn't fix all of the existing test cases and I don't understand the code so I don't know why it works sometimes.

ghost commented 2 years ago

The test summary:

PASS: test_adler32
PASS: test_buf_error
FAIL: test_crc32
../test-driver: line 107: 114513 Aborted                 "$@" > $log_file 2>&1
FAIL: test_inflatesyncpoint
FAIL: test_multithread_stress
PASS: test_pid_reuse
SKIP: test_abi
PASS: test_stress.auto
FAIL: test_deflate.auto
FAIL: test_inflate.auto
FAIL: test_dict.auto
PASS: test_gz.auto
PASS: test_stress.sw
FAIL: test_deflate.sw
FAIL: test_inflate.sw
FAIL: test_dict.sw
PASS: test_gz.sw
PASS: test_stress.nx
FAIL: test_deflate.nx
FAIL: test_inflate.nx
FAIL: test_dict.nx
PASS: test_gz.nx
PASS: test_stress.mix
FAIL: test_deflate.mix
FAIL: test_inflate.mix
FAIL: test_dict.mix
PASS: test_gz.mix
PASS: test_stress.mix2
FAIL: test_deflate.mix2
FAIL: test_inflate.mix2
FAIL: test_dict.mix2
PASS: test_gz.mix2
PASS: test_zeroinput.nx
============================================================================
Testsuite summary for libnxz 0.63
============================================================================
# TOTAL: 33
# PASS:  14
# SKIP:  1
# XFAIL: 0
# FAIL:  18
# XPASS: 0
# ERROR: 0
mscastanho commented 2 years ago

@amock Our version of __crc32_vpmsum indeed only supports LE. But it was adapted from this one, which supports BE as well, so it would be a matter of re-adding the parts containing BE logic.

ghost commented 2 years ago

@mscastanho Thanks for the link. I'll get started with that and see how fixing the crc gets me.