TestTypelist tests failing with s390x big-endian arch

[mruprich]

I know that this isn't probably going to get much attention since this only happens on s390x arch but I wanted to share this with you anyway.

I have hit this issue when building frr-7.2 in Fedora. The only arch that failed in all of the TestTypelist tests is s390x which is the only one with big-endian:

lib/test_typelist.py::TestTypelist::test_exit_cleanly FAILED                                          [ 96%]
lib/test_typelist.py::TestTypelist::test_LIST_end FAILED                                              [ 96%]
lib/test_typelist.py::TestTypelist::test_DLIST_end FAILED                                            [ 96%]
lib/test_typelist.py::TestTypelist::test_SORTLIST_UNIQ_end FAILED                          [ 96%]
lib/test_typelist.py::TestTypelist::test_SORTLIST_NONUNIQ_end FAILED                  [ 97%]
lib/test_typelist.py::TestTypelist::test_HASH_end FAILED                                            [ 97%]
lib/test_typelist.py::TestTypelist::test_HASH_collisions_end FAILED                            [ 97%]
lib/test_typelist.py::TestTypelist::test_SKIPLIST_UNIQ_end FAILED                            [ 97%]
lib/test_typelist.py::TestTypelist::test_SKIPLIST_NONUNIQ_end FAILED                    [ 98%]
lib/test_typelist.py::TestTypelist::test_RBTREE_UNIQ_end FAILED                             [ 98%]
lib/test_typelist.py::TestTypelist::test_RBTREE_NONUNIQ_end FAILED                     [ 98%]
lib/test_typelist.py::TestTypelist::test_ATOMSORT_UNIQ_end FAILED                       [ 98%]
lib/test_typelist.py::TestTypelist::test_ATOMSORT_NONUNIQ_end FAILED                [ 99%]
================ FAILURES ==============================
_______________ TestTypelist.test_exit_cleanly _________________

self = <test_typelist.TestTypelist object at 0x3ffa0abd7c0>

    def testfunction(self):
        self._run_tests()
        result = self.testresults[matchfunction]
        if result is not None:
>           frrsix.reraise(*result)

helpers/python/frrtest.py:101: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
helpers/python/frrsix.py:57: in reraise
    raise value
helpers/python/frrtest.py:76: in _run_tests
    test(self)
helpers/python/frrtest.py:94: in matchfunction
    method(self, *args, **kwargs)
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_typelist.TestTypelist object at 0x3ffa0abd7c0>

    def _exit_cleanly(self):
        if self.exitcode != 0:
>           raise MultiTestFailure("Program did not terminate with exit code 0")
E           frrtest.MultiTestFailure: Program did not terminate with exit code 0

helpers/python/frrtest.py:84: MultiTestFailure
------------------------ Captured stderr call --------------
lt-test_typelist: tests/lib/test_typelist.h:118: ts_hash_LIST: Assertion `i < count' failed.
_____________ TestTypelist.test_LIST_end _________

self = <test_typelist.TestTypelist object at 0x3ffa09425e0>

    def testfunction(self):
        self._run_tests()
        result = self.testresults[matchfunction]
        if result is not None:
>           frrsix.reraise(*result)

helpers/python/frrtest.py:101: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
helpers/python/frrsix.py:57: in reraise
    raise value
helpers/python/frrtest.py:76: in _run_tests
    test(self)
helpers/python/frrtest.py:94: in matchfunction
    method(self, *args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_typelist.TestTypelist object at 0x3ffa0abd7c0>, line = b'LIST end'

    def _onesimple(self, line):
        if type(line) is str:
            line = line.encode('utf8')
        idx = self.output.find(line)
        if idx != -1:
            self.output = self.output[idx+len(line):]
        else:
>           raise MultiTestFailure("%r could not be found" % line)
E           frrtest.MultiTestFailure: b'LIST end' could not be found

Other errors are similar to the last one.

I know that the fail is caused by this commit: 8c3d03b because I tried frr-7.1 and I added this commit together with 2214f16 and it triggered the fail.

I am assuming it has to do something with the endianness since I was able to reproduce this exact fail on x86_64 when I removed the htonl in tests/lib/test_typelist.h on line 103:

count = htonl(list_count(&head));

But otherwise, I am unable to make this work on s390x even when I tried to simulate the same behaviour like this:

count = bswap_32(list_count(&head));

I have spent some time on this trying to figure this out but I am out of ideas. If anyone has any insight, I would be more than happy to test any possible solution. For now, I will most likely just turn of these tests for s390x.

Regards,
Michal

(put "x" in "[ ]" if you already tried following)
[x ] Did you check if this is a duplicate issue?
[x ] Did you test it on the latest FRRouting/frr master branch?

[mjstapp]

interesting - the assert in there is comparing a byte-swapped 'count' with un-swapped 'i' - on little-endian, I'll bet that passes, but I think it might be off-by-one for a big-endian arch. could I offer you a patch to try?

[mjstapp]

This retains the un-swapped value of 'list_count', and tweaks the comparison with the index value 'i'.

index f20bbc52d..9039fa8a4 100644
--- a/tests/lib/test_typelist.h
+++ b/tests/lib/test_typelist.h
@@ -98,12 +98,13 @@ static void ts_hash(const char *text, const char *expect)
        unsigned i = 0;
        uint8_t hash[32];
        char hashtext[65];
-       uint32_t count;
+       uint32_t swap_count, count;
 
-       count = htonl(list_count(&head));
+       count = list_count(&head);
+       swap_count = htonl(count);
 
        SHA256_Init(&ctx);
-       SHA256_Update(&ctx, &count, sizeof(count));
+       SHA256_Update(&ctx, &swap_count, sizeof(swap_count));
 
        frr_each (list, &head, item) {
                struct {
@@ -115,7 +116,7 @@ static void ts_hash(const char *text, const char *expect)
                };
                SHA256_Update(&ctx, &hashitem, sizeof(hashitem));
                i++;
-               assert(i < count);
+               assert(i <= count);
        }
        SHA256_Final(hash, &ctx);

[donaldsharp]

@mjstapp -> I agree with your patch. Can you submit a PR?

[mjstapp]

Should be fixed by #5559

[mruprich]

Great, I didn't expect this to be fixed so swiftly. All builds on Fedora are green. Thanks guys.