Fedora Account System
Red Hat Associate
Red Hat Customer
isa-l-2.31.1-3.fc42 compiled with gcc 15 on Fedora rawhide: https://koji.fedoraproject.org/koji/buildinfo?buildID=2638214 The build fails on s390x due to a failure in the unit tests. isa-l-2.31.1-3.fc41 compiled with gcc 14 on Fedora 41: https://koji.fedoraproject.org/koji/buildinfo?buildID=2638220 This is the same code as above, and the unit tests succeed. isa-l-2.31.1-1.fc42 was successfully built on Fedora rawhide before the gcc 15 was introduced. There are no compiler warnings in the build log in koji. It is difficult to investigate a problem that affects only s390x. Multiple attempts to build the package in koji fail the same way. Reproducible: Always Steps to Reproduce: 1. Build the isa-l package in Fedora rawhide for s390x (with gcc 15) Current version at time of bug submission: gcc-0:15.0.1-0.3.fc42.s390x Actual Results: FAIL: igzip/igzip_rand_test =========================== igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: error: invalid header Fail igzip rand test finished: Some tests failed FAIL igzip/igzip_rand_test (exit status: 1) ============================================================================ Testsuite summary for libisal 2.31.1 ============================================================================ # TOTAL: 16 # PASS: 15 # SKIP: 0 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 ============================================================================ Expected Results: As on the other architectures: PASS: igzip/igzip_rand_test ============================================================================ Testsuite summary for libisal 2.31.1 ============================================================================ # TOTAL: 16 # PASS: 16 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 This started failing when gcc 15 was introduced in Fedora rawhide.
Are you sure it is gcc? I've built isa-l in rawhide mock (where it failed with the given test) and f41 (where it passed). In the rawhide chroot, I've run igzip_rand_test against the f41 libisal.so.2 (i.e. built with gcc 14.2.1), it still failed. In the 41 chroot, I've run igzip_rand_test against the rawhide libisal.so.2 (i.e. built with gcc 15.0.1), it still succeeded. That would point to the igzip_rand_test binary. But, cross-copying those binaries doesn't change anything, even rawhide built igzip_rand_test with LD_LIBRARY_PATH against rawhide built libisal.so.2 in the 41 chroot succeeds, while 41 built igzip_rand_test with LD_LIBRARY_PATH against 41 built libisal.so.2 fails.
So, in particular I've copied the .libs/libisal.so* and igzip/.libs/igzip_rand_test from both buildroots to /tmp/ of both buildroots. In 41 buildroot [mockbuild@da1a45a5b1e94ac9b1f689925e3c5204 ~]$ LD_LIBRARY_PATH=/tmp/rawhide/ /tmp/rawhide/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: Pass igzip_rand_test stateful NO_FLUSH: Pass igzip_rand_test stateful SYNC_FLUSH: Pass igzip_rand_test stateful FULL_FLUSH: Pass igzip_rand_test stateful Change Flush: Pass igzip_rand_test large input Pass igzip_rand_test inflate Std Vectors: Pass igzip rand test finished: All tests passed [mockbuild@da1a45a5b1e94ac9b1f689925e3c5204 ~]$ LD_LIBRARY_PATH=/tmp/41/ /tmp/41/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: Pass igzip_rand_test stateful NO_FLUSH: Pass igzip_rand_test stateful SYNC_FLUSH: Pass igzip_rand_test stateful FULL_FLUSH: Pass igzip_rand_test stateful Change Flush: Pass igzip_rand_test large input Pass igzip_rand_test inflate Std Vectors: Pass igzip rand test finished: All tests passed In the rawhide chroot [mockbuild@1310715f95354bd5b8749b91e252348f ~]$ LD_LIBRARY_PATH=/tmp/rawhide /tmp/rawhide/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: error: invalid header Fail igzip rand test finished: Some tests failed [mockbuild@1310715f95354bd5b8749b91e252348f ~]$ LD_LIBRARY_PATH=/tmp/41 /tmp/41/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: error: invalid header Fail igzip rand test finished: Some tests failed strace tells all it opens are the libisal.so.2 and glibc. openat(AT_FDCWD, "/tmp/41/libisal.so.2", O_RDONLY|O_CLOEXEC) = 3 openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3 openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 So the only thing that changed is glibc. And now same buildroot, /tmp/old is the fc41 ld64.so.1 and libc.so.6 and /tmp/new fc64 ld64.so.1 and libc.so.6: [mockbuild@1310715f95354bd5b8749b91e252348f ~]$ /tmp/old/ld64.so.1 --library-path /tmp/old:/tmp/41 /tmp/41/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: Pass igzip_rand_test stateful NO_FLUSH: Pass igzip_rand_test stateful SYNC_FLUSH: Pass igzip_rand_test stateful FULL_FLUSH: Pass igzip_rand_test stateful Change Flush: Pass igzip_rand_test large input Pass igzip_rand_test inflate Std Vectors: Pass igzip rand test finished: All tests passed [mockbuild@1310715f95354bd5b8749b91e252348f ~]$ /tmp/new/ld64.so.1 --library-path /tmp/new:/tmp/41 /tmp/41/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: error: invalid header Fail igzip rand test finished: Some tests failed and [mockbuild@3950c92442f34105bfd9728c2fe4f87b tmp]$ /tmp/old/ld64.so.1 --library-path /tmp/old:/tmp/rawhide /tmp/rawhide/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: Pass igzip_rand_test stateful NO_FLUSH: Pass igzip_rand_test stateful SYNC_FLUSH: Pass igzip_rand_test stateful FULL_FLUSH: Pass igzip_rand_test stateful Change Flush: Pass igzip_rand_test large input Pass igzip_rand_test inflate Std Vectors: Pass igzip rand test finished: All tests passed [mockbuild@3950c92442f34105bfd9728c2fe4f87b tmp]$ /tmp/new/ld64.so.1 --library-path /tmp/new:/tmp/rawhide /tmp/rawhide/igzip_rand_test igzip_rand_test stateless: Pass igzip_rand_test stateless FULL_FLUSH: error: invalid header Fail igzip rand test finished: Some tests failed in the other buildroot. So, it is glibc 2.40-17.s390x vs. glibc-2.40.9000-28.fc42 that changes things, not the version of gcc. And, both glibc 2.40-17.s390x and glibc 2.40.900-28.fc42.s390x have been built with gcc 14.2.1. Of course it might be some UB in the package, but I don't see how gcc is involved here at all.
I'll try to bisect it on the glibc side.
Surprisingly, it's not one of my TLS changes. I suspect there is an integer size mismatch somewhere in this change, and that breaks things on big endian: commit d5bceac99d24af1131b90027dab267e437b65cd1 (HEAD) Author: Sam James <sam> Date: Tue Dec 10 01:21:46 2024 +0000 stdlib: random_r: fix unaligned access in initstate and initstate_r [BZ #30584] The initstate{,_r} interfaces are documented in BSD as needing an aligned array of 32-bit values, but neither POSIX nor glibc's own documentation require it to be aligned. glibc's documentation says it "should" be a power of 2, but not must. Use memcpy to read and write to `state` to handle such an unaligned argument. Co-authored-by: Adhemerval Zanella <adhemerval.zanella> Reviewed-by: Florian Weimer <fweimer>
This fixes it: diff --git a/stdlib/random_r.c b/stdlib/random_r.c index 605e96983c..64f3e402aa 100644 --- a/stdlib/random_r.c +++ b/stdlib/random_r.c @@ -390,9 +390,8 @@ __random_r (struct random_data *buf, int32_t *result) int32_t *end_ptr = buf->end_ptr; uint32_t val; - val = read_state (rptr, 0); - int32_t t = read_state (fptr, 0); - write_state (fptr, 0, t + val); + val = read_state (fptr, 0) + read_state (rptr, 0); + write_state (fptr, 0, val); /* Chucking least random bit. */ *result = val >> 1; ++fptr; It's clearly a bug in the original commit, which made this change: - val = *fptr += (uint32_t) *rptr; + val = read_state (rptr, 0); + int32_t t = read_state (fptr, 0); + write_state (fptr, 0, t + val); I would like to better understand why this makes a difference, and why only on s390x.
(In reply to Florian Weimer from comment #5) > + val = read_state (fptr, 0) + read_state (rptr, 0); Does it care which read_state happens first?
(In reply to Jakub Jelinek from comment #6) > (In reply to Florian Weimer from comment #5) > > + val = read_state (fptr, 0) + read_state (rptr, 0); > > Does it care which read_state happens first? No, I don't think the original code had a sequence point violation or race condition. The issue is that the sequence of random numbers changes due to this bug, and the test case igzip/igzip_rand_test.c now selects level 3 here instead of level 0: int test_compress_stateless(uint8_t *in_data, uint32_t in_size, uint32_t flush_type) { int ret = IGZIP_COMP_OK; uint32_t z_size, overflow, gzip_flag, level, hist_bits; uint8_t *z_buf = NULL; uint8_t *in_buf = NULL; gzip_flag = rand() % 5; hist_bits = rand() % 16; level = get_rand_level(); … } If I change the level to 3 manually on s390x, the test fails with the old glibc, too. But forcing this change into the x86-64 build doesn't make the test fail on that architecture. The s390x gzip accelerator does not seem to be used. Maybe it's the over way round: the x86-64 assembler pieces deal with this test change better than the C implementation that is used on s390x.
I think val = read_state (fptr, 0) + read_state (rptr, 0); is bad, because the function returns int32_t and I think the point of the old code was to do the addition in unsigned type to avoid UB on overflow. So better either val = read_state (fptr, 0); val += read_state (rptr, 0); or the other way around, or explicit (uint32_t) cast somewhere. As for isa-l, isn't there a bug then in the testcase as well? Give me any (pseudo)random number you like as long as it is not this and that because I don't handle that.
(In reply to Jakub Jelinek from comment #8) > I think val = read_state (fptr, 0) + read_state (rptr, 0); is bad, because > the function returns int32_t and I think the point of the old code was to do > the addition in unsigned type to avoid UB on overflow. > So better either val = read_state (fptr, 0); val += read_state (rptr, 0); or > the other way around, or explicit (uint32_t) cast somewhere. > > As for isa-l, isn't there a bug then in the testcase as well? Give me any > (pseudo)random number you like as long as it is not this and that because I > don't handle that. Fair point, I'll tweak it.
Posted upstream: [PATCH 1/2] stdlib: Fix unintended change to the random_r implementation <https://inbox.sourceware.org/libc-alpha/76b56012252d98de3ffacb5f8e3b2adfd6ee6ede.1737403568.git.fweimer@redhat.com/>
(In reply to Jakub Jelinek from comment #8) > As for isa-l, isn't there a bug then in the testcase as well? Give me any > (pseudo)random number you like as long as it is not this and that because I > don't handle that. The isa-l issue has been fixed uosream: https://github.com/intel/isa-l/issues/311