Skip to content
New issue

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

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

Already on GitHub? # to your account

zsnes: fix build against zlib-1.3 #263444

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Oct 25, 2023

Without the change builda fails on master as
https://hydra.nixos.org/build/238481288/nixlog/3:

checking for zlib - version >= 1.2.3... 1.3, bad version string given
    by zlib, sometimes due to very old zlibs that didnt correctly
    define their version. Please upgrade if you are running an
    old zlib... no

The failure happens due to 2-digit zlib version string. Add a trivial change to allow 2-digit support in addition to existing 3-digit ones..

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Without the change builda fails on `master` as
https://hydra.nixos.org/build/238481288/nixlog/3:

    checking for zlib - version >= 1.2.3... 1.3, bad version string given
        by zlib, sometimes due to very old zlibs that didnt correctly
        define their version. Please upgrade if you are running an
        old zlib... no

The failure happens due to 2-digit zlib version string.
Add a trivial change to allow 2-digit support in addition to existing
3-digit ones..
@trofi trofi mentioned this pull request Oct 25, 2023
14 tasks
@ofborg ofborg bot requested a review from svanderburg October 26, 2023 00:30
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Oct 26, 2023
@pbsds
Copy link
Member

pbsds commented Nov 1, 2023

Fix LGTM, upstream has been unresponsive since 2013 so there is no upstreaming it.
I can't get this to run however, nor the version on unstable pre zlib-1.3

$ nix run github:nixos/nixpkgs/0bffda19b8af722f8069d09d8b6a24594c80b352#zsnes
...
*** buffer overflow detected ***: terminated
Aborted (core dumped)

It seems to happen when initializing audio, judging by the output from zsnes on 23.05.
Is this an actual breakage, or does this happen because i'm running on a stable system?

@trofi
Copy link
Contributor Author

trofi commented Nov 2, 2023

Fix LGTM, upstream has been unresponsive since 2013 so there is no upstreaming it. I can't get this to run however, nor the version on unstable pre zlib-1.3

$ nix run github:nixos/nixpkgs/0bffda19b8af722f8069d09d8b6a24594c80b352#zsnes
...
*** buffer overflow detected ***: terminated
Aborted (core dumped)

It seems to happen when initializing audio, judging by the output from zsnes on 23.05. Is this an actual breakage, or does this happen because i'm running on a stable system?

Ah, that one is the result of fortify3 crash. Startup crashes for me as well.

Pushed zsnes: fix buffer size marking to avoid crash on _FORTIFY_SOURCE=3 patch to fix that. That fixes the run for me.

Without the change `zsnesn` startup crashes with:

    $ gdb zsnes
    *** buffer overflow detected ***: terminated
    ...
    #7  0x08057c14 in memset (__len=2, __ch=255, __dest=<optimized out>) at
      ...-glibc-2.38-23-dev/include/bits/string_fortified.h:59
@trofi
Copy link
Contributor Author

trofi commented Nov 2, 2023

Forgot to actually push the patch. Pushed zsnes: fix buffer size marking to avoid crash on _FORTIFY_SOURCE=3 now.

@pbsds
Copy link
Member

pbsds commented Nov 2, 2023

Works now, you're a wizard

@pbsds pbsds merged commit 5628bfe into NixOS:master Nov 2, 2023
@trofi trofi deleted the zsnes-zlib-1.3-fix branch November 2, 2023 13:57
@trofi
Copy link
Contributor Author

trofi commented Nov 2, 2023

I was too eager trying to fix bounds access in fortify3.patch. Proposed the amendment: #265039

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants