Test failures for python-isal on aarch64

[ellert]

When compiling https://github.com/pycompression/python-isal on aarch64 there are some test failures.

The following tests from python-isal fail on aarch64:

=========================== short test summary info ============================
FAILED tests/test_compat.py::test_decompress_wbits[131072-1-15-1] - igzip_lib.IsalError: Error -6 Incorrect checksum found
FAILED tests/test_compat.py::test_decompress_wbits[131072-1-31-1] - igzip_lib.IsalError: Error -6 Incorrect checksum found
FAILED tests/test_compat.py::test_decompress_decompressobj[131072-1-15-1] - igzip_lib.IsalError: Error -6 Incorrect checksum found
FAILED tests/test_compat.py::test_decompress_decompressobj[131072-1-31-1] - igzip_lib.IsalError: Error -6 Incorrect checksum found
============ 4 failed, 9437 passed, 6 skipped, 2 warnings in 26.12s ============

The failures are not related to the python-isal code, but are related to the aarch64 assembler version of decode_huffman_code_block_stateless in ISA-L. If this is not used the tests do not fail.

diff --git a/igzip/aarch64/igzip_multibinary_aarch64_dispatcher.c b/igzip/aarch64/igzip_multibinary_aarch64_dispatcher.c
index cadec0c..71f61a2 100644
--- a/igzip/aarch64/igzip_multibinary_aarch64_dispatcher.c
+++ b/igzip/aarch64/igzip_multibinary_aarch64_dispatcher.c
@@ -234,9 +234,9 @@ DEFINE_INTERFACE_DISPATCHER(isal_deflate_hash_lvl3)
 DEFINE_INTERFACE_DISPATCHER(decode_huffman_code_block_stateless)
 {
 #if defined(__linux__)
-        unsigned long auxval = getauxval(AT_HWCAP);
-        if (auxval & HWCAP_CRC32)
-                return PROVIDER_INFO(decode_huffman_code_block_stateless_aarch64);
+        /* unsigned long auxval = getauxval(AT_HWCAP); */
+        /* if (auxval & HWCAP_CRC32) */
+        /*         return PROVIDER_INFO(decode_huffman_code_block_stateless_aarch64); */
 #elif defined(__APPLE__)
         if (sysctlEnabled(SYSCTL_CRC32_KEY))
                 return PROVIDER_INFO(decode_huffman_code_block_stateless_aarch64);

The above is obviously not the proper patch, the assembler code should be debugged and fixed instead. I don't know enough assembler to debug this. Since the change above is just a workaround I didn't create a pull request and filed an issue instead.

The long test failure log is copied below:

=================================== FAILURES ===================================
_____________________ test_decompress_wbits[131072-1-15-1] _____________________
data_size = 131072, level = 1, wbits = 15, memLevel = 1
    @pytest.mark.parametrize(["data_size", "level", "wbits", "memLevel"],
                             itertools.product([128 * 1024], range(4),
                                               WBITS_RANGE, range(1, 10)))
    def test_decompress_wbits(data_size, level, wbits, memLevel):
        data = DATA[:data_size]
        compressobj = zlib.compressobj(level=level, wbits=wbits, memLevel=memLevel)
        compressed = compressobj.compress(data) + compressobj.flush()
>       decompressed = isal_zlib.decompress(compressed, wbits=wbits)
E       igzip_lib.IsalError: Error -6 Incorrect checksum found
tests/test_compat.py:76: IsalError
_____________________ test_decompress_wbits[131072-1-31-1] _____________________
data_size = 131072, level = 1, wbits = 31, memLevel = 1
    @pytest.mark.parametrize(["data_size", "level", "wbits", "memLevel"],
                             itertools.product([128 * 1024], range(4),
                                               WBITS_RANGE, range(1, 10)))
    def test_decompress_wbits(data_size, level, wbits, memLevel):
        data = DATA[:data_size]
        compressobj = zlib.compressobj(level=level, wbits=wbits, memLevel=memLevel)
        compressed = compressobj.compress(data) + compressobj.flush()
>       decompressed = isal_zlib.decompress(compressed, wbits=wbits)
E       igzip_lib.IsalError: Error -6 Incorrect checksum found
tests/test_compat.py:76: IsalError
_________________ test_decompress_decompressobj[131072-1-15-1] _________________
data_size = 131072, level = 1, wbits = 15, memLevel = 1
    @pytest.mark.parametrize(["data_size", "level", "wbits", "memLevel"],
                             itertools.product([128 * 1024], range(4),
                                               WBITS_RANGE, range(1, 10)))
    def test_decompress_decompressobj(data_size, level, wbits, memLevel):
        data = DATA[:data_size]
        compressobj = zlib.compressobj(level=level, wbits=wbits, memLevel=memLevel)
        compressed = compressobj.compress(data) + compressobj.flush()
        decompressobj = isal_zlib.decompressobj(wbits=wbits)
>       decompressed = decompressobj.decompress(compressed) + decompressobj.flush()
E       igzip_lib.IsalError: Error -6 Incorrect checksum found
tests/test_compat.py:111: IsalError
_________________ test_decompress_decompressobj[131072-1-31-1] _________________
data_size = 131072, level = 1, wbits = 31, memLevel = 1
    @pytest.mark.parametrize(["data_size", "level", "wbits", "memLevel"],
                             itertools.product([128 * 1024], range(4),
                                               WBITS_RANGE, range(1, 10)))
    def test_decompress_decompressobj(data_size, level, wbits, memLevel):
        data = DATA[:data_size]
        compressobj = zlib.compressobj(level=level, wbits=wbits, memLevel=memLevel)
        compressed = compressobj.compress(data) + compressobj.flush()
        decompressobj = isal_zlib.decompressobj(wbits=wbits)
>       decompressed = decompressobj.decompress(compressed) + decompressobj.flush()
E       igzip_lib.IsalError: Error -6 Incorrect checksum found
tests/test_compat.py:111: IsalError

The tests cycles through many sets of possible parameters, but the failure is only seen on two of the many sets:

• data_size = 131072, level = 1, wbits = 15, memLevel = 1
• data_size = 131072, level = 1, wbits = 31, memLevel = 1

[ellert]

I have recreated the failing test from python-isal in C.

https://ellert.se/isa-l-test/isa-l-test.tar.gz

For most architectures the test is successful and ends with:

Summary: 0 failures

However, on aarch64 the test fails:

Summary: 2 failures

The two input files that fail are:

zlib/1-15-1.data
Error -6 Incorrect checksum found

and

gzip/1-15-1.data
Error -6 Incorrect checksum found

I do not have the necessary skills to debug aarch64 assembler code, but I hope that with this reproducer someone can fix it.

[tipabu]

FWIW, I can repro this on OS X, too; both the zlib and gzip files fail with bad checksums. Easiest way to demonstrate might be this repo's igzip, which chokes on the file:

% igzip -cd isa-l-test/gzip/1-15-1.data | wc -c
igzip: Error encountered while decompressing file isa-l-test/gzip/1-15-1.data
       0

Of course, I can bypass the check with something like:

diff --git a/igzip/igzip_inflate.c b/igzip/igzip_inflate.c
index ec788ab..376be0d 100644
--- a/igzip/igzip_inflate.c
+++ b/igzip/igzip_inflate.c
@@ -1901,9 +1901,11 @@ check_gzip_checksum(struct inflate_state *state)
         crc = state->crc;
         total_out = state->total_out;

+/*
         if (trailer != (crc | (total_out << 32)))
                 return ISAL_INCORRECT_CHECKSUM;
         else
+*/
                 return ISAL_DECOMP_OK;
 }

Then decompression produces some valid data, but is interrupted partway:

% diff <( zcat < isa-l-test/gzip/1-15-1.data | \
          head -c $( igzip -cd isa-l-test/gzip/1-15-1.data | wc -c ) ) \
       <( igzip -cd isa-l-test/gzip/1-15-1.data )
% echo $?
0

FWIW, it stops after a pretty suspicious number of bytes:

% igzip -cd isa-l-test/gzip/1-15-1.data | wc -c
   65537

while it should be just under twice that size:

% zcat < isa-l-test/gzip/1-15-1.data | wc -c
  131072

[tipabu]

OK, a little more digging: The trouble seems to be this call to decode_huffman_code_block_stateless. With the C code, we see a bunch of zero returns, followed by ISAL_OUT_OVERFLOW; at that point, the block state is ISAL_BLOCK_NEW_HDR and the rest of the data gets decompressed via the other decode_huffman_code_block_stateless call site.

The ASM has the same zero returns there, but when it returns ISAL_OUT_OVERFLOW, it's left the block state at ISAL_BLOCK_INPUT_DONE -- so decompression halts, and since the CRC of the partial data doesn't match the CRC of all the expected data, we eventually return that ISAL_INCORRECT_CHECKSUM.

FWIW, if I apply

diff --git a/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S b/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
index 1420c88..c7f3741 100644
--- a/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
+++ b/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
@@ -642,7 +642,7 @@ invalid_symbol:
        b exit_func
 isal_out_overflow_1:

-       cmp     bfinal,0
+       cmp     w_bfinal,0
        mov     block_state, ISAL_BLOCK_INPUT_DONE
        csel    block_state, block_state, wzr, ne
        str     block_state, [block_state_adr]

(so it more closely matches next_lit_eq_256) then I can get your failing test passing. I don't feel like I've got enough understanding of the assembly to feel confident proposing it, though -- in part because I can also get it passing with something like

diff --git a/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S b/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
index 1420c88..9ace252 100644
--- a/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
+++ b/igzip/aarch64/igzip_decode_huffman_code_block_aarch64.S
@@ -644,7 +644,7 @@ isal_out_overflow_1:

        cmp     bfinal,0
        mov     block_state, ISAL_BLOCK_INPUT_DONE
-       csel    block_state, block_state, wzr, ne
+       csel    block_state, block_state, wzr, eq
        str     block_state, [block_state_adr]
 isal_out_overflow:
        mov     w0, ISAL_OUT_OVERFLOW

Maybe @liuqinfei or @yuhaoth can help guide us?

[ellert]

I ran the complete test suite from python-isal.

As stated above, the unmodified isa-l gives 4 failures on aarch64:

=========================== short test summary info ============================
FAILED tests/test_compat.py::test_decompress_wbits[131072-1-15-1] - igzip_lib...
FAILED tests/test_compat.py::test_decompress_wbits[131072-1-31-1] - igzip_lib...
FAILED tests/test_compat.py::test_decompress_decompressobj[131072-1-15-1] - i...
FAILED tests/test_compat.py::test_decompress_decompressobj[131072-1-31-1] - i...
============ 4 failed, 9437 passed, 6 skipped, 3 warnings in 26.15s ============

With the first proposed fix (bfinal → w_bfinal), there are no failure:

================= 9441 passed, 6 skipped, 3 warnings in 26.06s =================

With the second proposed fix (ne → eq) there are 2 failures:

=========================== short test summary info ============================
FAILED tests/test_compat.py::test_decompress_wbits[131072-1--15-1] - Assertio...
FAILED tests/test_compat.py::test_decompress_decompressobj[131072-1--15-1] - ...
============ 2 failed, 9439 passed, 6 skipped, 3 warnings in 26.40s ============

So the first one is a potential fix, but not the second.

[liuqinfei]

@ellert Could you provide the command line to reproduce the issue?

[ellert]

@ellert Could you provide the command line to reproduce the issue?

git clone https://github.com/intel/isa-l.git
cd isa-l
autoreconf -i -v -f
./configure --disable-static
make
make install DESTDIR=$PWD/../isa-l-install
cd ..
git clone https://github.com/pycompression/python-isal.git
cd python-isal
PYTHON_ISAL_LINK_DYNAMIC=1 CFLAGS=-I$PWD/../isa-l-install/usr/include LDFLAGS=-L$PWD/../isa-l-install/usr/lib64 python3 -m pip wheel .
LD_LIBRARY_PATH=$PWD/../isa-l-install/usr/lib64 PYTHONPATH=$PWD/build/lib.linux-aarch64-cpython-314 python3 -m pytest tests

Note the . at the end of the second to last line.
The python path on the last line differs due to python version.
Running the tests requires the python modules: pytest, pytest-timeout, test.

[liuqinfei]

I was unable to reproduce the issue you described on kunpeng aarch64 platform; however, I did encounter a different test failure. FAILED tests/test_igzip_threaded.py::test_flush.

isa-l version: commit 73c5044
python-isa-l version: commit 52dba9210f544547c97bced28b31a8e30fcfd2b4

tests/test_compat.py .........=== 6490 passed in 20.63s ===

[ellert]

My test uses the current master of both repos, which currently are the same commits as you state above.
The dispatcher only uses decode_huffman_code_block_stateless_aarch64 if the cpu supports HWCAP_CRC32, otherwise the base implementation is used. Patching the dispatcher to always use the base implementation for this function works around this bug. This is what currently is done in the isa-l package distributen in Fedora.
I do not see the error you report above.

# cat /proc/cpuinfo | grep Features
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp