Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crc32/adler32_combine doesn't work on 32-bit musl systems #12

Closed
rrbq opened this issue Jun 7, 2022 · 6 comments
Closed

crc32/adler32_combine doesn't work on 32-bit musl systems #12

rrbq opened this issue Jun 7, 2022 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@rrbq
Copy link

rrbq commented Jun 7, 2022

Hello, while building Perl 5.36 on Alpine Linux, I noticed that the testsuite for C:R:Z fails the crc32/adler32_combine tests on 32-bit platforms (specifically x86, armv7, and armhf).

Alpine uses musl libc instead of glibc. When building Perl, Alpine's build system sets the environment variable BUILD_ZLIB=0, and removes C:R:Z's zlib-src directory.

From what I could understand of this issue, it has something to do with musl defining off_t to be 64-bit everywhere, even on 32-bit platforms.

This breaks the assumption that off_t always has the same size as long, which while true for 64-bit systems, is no longer the case for 32-bit systems using musl.

On 32-bit Alpine, this results in its system zlib having a 64-bit z_off_t, while C:R:Z expects a 32-bit z_off_t. For those platforms, the crc32/adler32_combine functions are also not functioning properly with the C:R:Z bundled in previous versions of Perl, but as the tests for those functions just appeared in Perl 5.36, Alpine has not noticed this problem until now.

After some discussion, the Alpine developers have decided to provisionally remove -DZ_SOLO from the Makefile.PL of C:R:Z. To reach this decision, it was observed that there are 2 places in zlib's zconf.h where z_off_t could get #defined:

#ifndef Z_SOLO
#  if defined(Z_HAVE_UNISTD_H) || defined(_LARGEFILE64_SOURCE)
#    include <unistd.h>         /* for SEEK_*, off_t, and _LFS64_LARGEFILE */
#    ifdef VMS
#      include <unixio.h>       /* for off_t */
#    endif
#    ifndef z_off_t
#      define z_off_t off_t
#    endif
#  endif
#endif

and

#ifndef z_off_t
#  define z_off_t long
#endif

When Z_SOLO is defined, the first definition of z_off_t (which would give the right size of 64-bit everywhere for systems using musl) is ignored, and instead the second one is used, which causes problems on 32-bit systems.

I have some questions about this:

  1. May I know if it is safe to remove -DZ_SOLO while building C:R:Z? Are there any unexpected behaviors we should be on the look out for?
  2. If it is safe to do so, would it be possible for C:R:Z to stop defining Z_SOLO whenever a pre-built system zlib is used (just like what happens with Z_PREFIX)?

Thank you.

@pmqs pmqs self-assigned this Jun 7, 2022
@pmqs pmqs added the bug Something isn't working label Jun 7, 2022
@pmqs
Copy link
Owner

pmqs commented Jun 7, 2022

Hey @rrbq, thanks for the bug report.

I have some questions about this:

  1. May I know if it is safe to remove -DZ_SOLO while building C:R:Z? Are there any unexpected behaviors we should be on the look out for?

if you aren't building C:R:Z with the zlib source included, than it will be fine.

  1. If it is safe to do so, would it be possible for C:R:Z to stop defining Z_SOLO whenever a pre-built system zlib is used (just like what happens with Z_PREFIX)?

Yes, that looks like the best way forward with this issue. The use of Z_SOLO only makes sense when C:R:Z is building its own copy if zlib.

I'll get an update to my code out

pmqs added a commit that referenced this issue Jun 7, 2022
@pmqs
Copy link
Owner

pmqs commented Jun 7, 2022

@rrbq if you get a chance could you try the latest version of the code I've added to github please? If that works for you I'll spin an official release.

@rrbq
Copy link
Author

rrbq commented Jun 7, 2022

Thanks for the bug fix. I can confirm that the latest c44e0b7 on Github fixes the problem I had with Z_SOLO on 32-bit musl systems.

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jun 8, 2022
removed zlib-test.patch that has been applied upstream.

also run tests in parallel, this cuts down the time
required to finish building Perl from 20 to 4
minutes (on x86_64). Test options are modified from
Perl's documentation (the "Parallel tests" section
of `perldoc perlhack`), by using HARNESS_OPTIONS
instead of TEST_JOBS, as this provides an additional
check that $JOBS is a number, and handles the case
when $JOBS is not set, by giving it a default of 9.

a new zlib-no-zsolo.patch has been added that removes
"-DZ_SOLO" from Compress::Raw::Zlib's Makefile.PL. This
issue has been reported upstream and will be fixed soon:
pmqs/Compress-Raw-Zlib#12 .
@pmqs
Copy link
Owner

pmqs commented Jun 14, 2022

Change 5338226 adds a 32-bit Alpine workflow file, alpine-32bit.yml, that will guard against regressions with this issue.

@pmqs
Copy link
Owner

pmqs commented Jun 14, 2022

Thanks for the bug fix. I can confirm that the latest c44e0b7 on Github fixes the problem I had with Z_SOLO on 32-bit musl systems.

Thanks @rrbq. Just getting some infrastructure in place to test for regressions with this fix in place before I push out an official release.

@pmqs
Copy link
Owner

pmqs commented Jun 21, 2022

Issue fixed in Compress::Raw::Zlib 2.200. Uploaded to CPAN (https://metacpan.org/dist/Compress-Raw-Zlib)

@pmqs pmqs closed this as completed Jun 21, 2022
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Jun 26, 2022
Fixes test failures on musl.

Restored based on 6b1b54851d9b8525f5981592138dc35ca6d2380a.

Bug: pmqs/Compress-Raw-Zlib#12
Signed-off-by: Sam James <[email protected]>
atoomic added a commit to atoomic/perl5 that referenced this issue Jul 20, 2022
From ChangeLog:

  2.202 27 June 2022

      * Z_NULL should be 'UV' rather than 'PV'
        pmqs/Compress-Raw-Zlib#17
        Sun Jun 26 22:02:04 2022 +0100
        de28f0335d3d605d696b19d43fc48de42272455c

  2.201 25 June 2022

      * 2.021
        Sat Jun 25 08:42:46 2022 +0100
        85416cab509c18c5fa3f923de7b45b6c7c0f7a6f

      * 2.201
        Sat Jun 25 08:39:26 2022 +0100
        b3d63862b2ff4ac9d28e23be500c0d32ad69dd11

      * More zlib-ng updates
        Thu Jun 23 22:42:13 2022 +0100
        313f626425181702b5fc80af2b6ea7eed41d5a9d

      * Fix test count regression in t/07bufsize.t (Perl#16)
        Wed Jun 22 09:45:11 2022 +0100
        98dc5b4a2b30c26752b6f686462b06b8db72a5e4

  2.200 21 June 2022

      * Added zlib-ng support
        pmqs/Compress-Raw-Zlib#9

      * Only set Z_SOLO when building zlib sources
      * pmqs/Compress-Raw-Zlib#12
        Tue Jun 7 10:13:00 2022 +0100
        c44e0b732e214b7f77d42a3af6ae64ef944cee90

  2.105 14 April 2022

      * Add Compress::Raw::Zlib::VERSION to output
        Sat May 14 15:16:57 2022 +0100
        3e22c93169a67986017f64d9a2e5085c417d8624

      * Dump version info when running test harness
        Sat May 14 15:10:17 2022 +0100
        ca9f33ba0323d0abc91a83800636f180b2b44162

      * Fix use of ZLIB_INCLUDE/LIB
        Sat May 14 09:01:38 2022 +0100
        8a7d4a97d7441b61a8a888342766419044fa5a33

      * More fixes for BUILD_ZLIB off
        Sat May 14 08:54:04 2022 +0100
        2d9650094dab90858ef58bfbda62f3bc60e159e4

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:31:54 2022 +0100
        b61b92fc9d06bf04f1adec337357ffbd39535901

      * Merge branch 'master' of
      * https://github.com/pmqs/Compress-Raw-Zlib
        Sat May 14 08:27:14 2022 +0100
        3ac7d0d3d45ae263402fab1ebb3835e2ae16c5a6

      * Fix for BUILD_ZLIB disabled
        Sat May 14 08:25:34 2022 +0100
        b0f04e37fb58a34ef01767ad16a8f63ca868eec6

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:22:56 2022 +0100
        aa8f5ff981c7305c995d4e2f798ae0d7d45866a5

  2.104 13 April 2022

      * Merge pull request #11 from monkburger/symbol_fix_2
        Fri May 13 07:17:19 2022 +0100
        64aea2d3f78946d7df4096eadfa0d7267f4439a5

      * perl_crz -> Perl_crz
        Tue May 3 18:19:24 2022 +0000
        20502e6c2eba8ddcad80b20574e840457c0cb369

      * This is a slightly different way to fix
      * pmqs/Compress-Raw-Zlib#8
        Tue May 3 18:06:48 2022 +0000
        d9cd27fb212da7455b6ba44729ca11bb441f3950

      * add tests for crc32/adler32_combine
        Mon May 2 16:18:13 2022 +0100
        dcfe9ef439790f1a4fae81cf3eac38cfeb848294
atoomic added a commit to Perl/perl5 that referenced this issue Jul 20, 2022
From ChangeLog:

  2.202 27 June 2022

      * Z_NULL should be 'UV' rather than 'PV'
        pmqs/Compress-Raw-Zlib#17
        Sun Jun 26 22:02:04 2022 +0100
        de28f0335d3d605d696b19d43fc48de42272455c

  2.201 25 June 2022

      * 2.021
        Sat Jun 25 08:42:46 2022 +0100
        85416cab509c18c5fa3f923de7b45b6c7c0f7a6f

      * 2.201
        Sat Jun 25 08:39:26 2022 +0100
        b3d63862b2ff4ac9d28e23be500c0d32ad69dd11

      * More zlib-ng updates
        Thu Jun 23 22:42:13 2022 +0100
        313f626425181702b5fc80af2b6ea7eed41d5a9d

      * Fix test count regression in t/07bufsize.t (#16)
        Wed Jun 22 09:45:11 2022 +0100
        98dc5b4a2b30c26752b6f686462b06b8db72a5e4

  2.200 21 June 2022

      * Added zlib-ng support
        pmqs/Compress-Raw-Zlib#9

      * Only set Z_SOLO when building zlib sources
      * pmqs/Compress-Raw-Zlib#12
        Tue Jun 7 10:13:00 2022 +0100
        c44e0b732e214b7f77d42a3af6ae64ef944cee90

  2.105 14 April 2022

      * Add Compress::Raw::Zlib::VERSION to output
        Sat May 14 15:16:57 2022 +0100
        3e22c93169a67986017f64d9a2e5085c417d8624

      * Dump version info when running test harness
        Sat May 14 15:10:17 2022 +0100
        ca9f33ba0323d0abc91a83800636f180b2b44162

      * Fix use of ZLIB_INCLUDE/LIB
        Sat May 14 09:01:38 2022 +0100
        8a7d4a97d7441b61a8a888342766419044fa5a33

      * More fixes for BUILD_ZLIB off
        Sat May 14 08:54:04 2022 +0100
        2d9650094dab90858ef58bfbda62f3bc60e159e4

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:31:54 2022 +0100
        b61b92fc9d06bf04f1adec337357ffbd39535901

      * Merge branch 'master' of
      * https://github.com/pmqs/Compress-Raw-Zlib
        Sat May 14 08:27:14 2022 +0100
        3ac7d0d3d45ae263402fab1ebb3835e2ae16c5a6

      * Fix for BUILD_ZLIB disabled
        Sat May 14 08:25:34 2022 +0100
        b0f04e37fb58a34ef01767ad16a8f63ca868eec6

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:22:56 2022 +0100
        aa8f5ff981c7305c995d4e2f798ae0d7d45866a5

  2.104 13 April 2022

      * Merge pull request #11 from monkburger/symbol_fix_2
        Fri May 13 07:17:19 2022 +0100
        64aea2d3f78946d7df4096eadfa0d7267f4439a5

      * perl_crz -> Perl_crz
        Tue May 3 18:19:24 2022 +0000
        20502e6c2eba8ddcad80b20574e840457c0cb369

      * This is a slightly different way to fix
      * pmqs/Compress-Raw-Zlib#8
        Tue May 3 18:06:48 2022 +0000
        d9cd27fb212da7455b6ba44729ca11bb441f3950

      * add tests for crc32/adler32_combine
        Mon May 2 16:18:13 2022 +0100
        dcfe9ef439790f1a4fae81cf3eac38cfeb848294
scottchiefbaker pushed a commit to scottchiefbaker/perl5 that referenced this issue Nov 3, 2022
From ChangeLog:

  2.202 27 June 2022

      * Z_NULL should be 'UV' rather than 'PV'
        pmqs/Compress-Raw-Zlib#17
        Sun Jun 26 22:02:04 2022 +0100
        de28f0335d3d605d696b19d43fc48de42272455c

  2.201 25 June 2022

      * 2.021
        Sat Jun 25 08:42:46 2022 +0100
        85416cab509c18c5fa3f923de7b45b6c7c0f7a6f

      * 2.201
        Sat Jun 25 08:39:26 2022 +0100
        b3d63862b2ff4ac9d28e23be500c0d32ad69dd11

      * More zlib-ng updates
        Thu Jun 23 22:42:13 2022 +0100
        313f626425181702b5fc80af2b6ea7eed41d5a9d

      * Fix test count regression in t/07bufsize.t (Perl#16)
        Wed Jun 22 09:45:11 2022 +0100
        98dc5b4a2b30c26752b6f686462b06b8db72a5e4

  2.200 21 June 2022

      * Added zlib-ng support
        pmqs/Compress-Raw-Zlib#9

      * Only set Z_SOLO when building zlib sources
      * pmqs/Compress-Raw-Zlib#12
        Tue Jun 7 10:13:00 2022 +0100
        c44e0b732e214b7f77d42a3af6ae64ef944cee90

  2.105 14 April 2022

      * Add Compress::Raw::Zlib::VERSION to output
        Sat May 14 15:16:57 2022 +0100
        3e22c93169a67986017f64d9a2e5085c417d8624

      * Dump version info when running test harness
        Sat May 14 15:10:17 2022 +0100
        ca9f33ba0323d0abc91a83800636f180b2b44162

      * Fix use of ZLIB_INCLUDE/LIB
        Sat May 14 09:01:38 2022 +0100
        8a7d4a97d7441b61a8a888342766419044fa5a33

      * More fixes for BUILD_ZLIB off
        Sat May 14 08:54:04 2022 +0100
        2d9650094dab90858ef58bfbda62f3bc60e159e4

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:31:54 2022 +0100
        b61b92fc9d06bf04f1adec337357ffbd39535901

      * Merge branch 'master' of
      * https://github.com/pmqs/Compress-Raw-Zlib
        Sat May 14 08:27:14 2022 +0100
        3ac7d0d3d45ae263402fab1ebb3835e2ae16c5a6

      * Fix for BUILD_ZLIB disabled
        Sat May 14 08:25:34 2022 +0100
        b0f04e37fb58a34ef01767ad16a8f63ca868eec6

      * Add BUILD_ZLIB to the matrix
        Sat May 14 08:22:56 2022 +0100
        aa8f5ff981c7305c995d4e2f798ae0d7d45866a5

  2.104 13 April 2022

      * Merge pull request Perl#11 from monkburger/symbol_fix_2
        Fri May 13 07:17:19 2022 +0100
        64aea2d3f78946d7df4096eadfa0d7267f4439a5

      * perl_crz -> Perl_crz
        Tue May 3 18:19:24 2022 +0000
        20502e6c2eba8ddcad80b20574e840457c0cb369

      * This is a slightly different way to fix
      * pmqs/Compress-Raw-Zlib#8
        Tue May 3 18:06:48 2022 +0000
        d9cd27fb212da7455b6ba44729ca11bb441f3950

      * add tests for crc32/adler32_combine
        Mon May 2 16:18:13 2022 +0100
        dcfe9ef439790f1a4fae81cf3eac38cfeb848294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants