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

mingw: Disalbe FORTIFY_SOURCE by default. #133479

Merged
merged 2 commits into from
Aug 12, 2021
Merged

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Aug 11, 2021

In newer versions of mingw, programs compiled with FORTIFY_SOURCE need
to link to libssp or they will have link-time errors.

gmp has been broken since @pstn updated mingw-64 in c60a0b0

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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.

In newer versions of mingw, programs compiled with FORTIFY_SOURCE need
to link to libssp or they will have link-time errors.

gmp has been broken since @pstn updated mingw-64 in c60a0b0
@SuperSandro2000
Copy link
Member

In newer versions of mingw, programs compiled with FORTIFY_SOURCE need
to link to libssp or they will have link-time errors.

Would an alternative be to just add libssp or am I missing something?

@shlevy
Copy link
Member Author

shlevy commented Aug 11, 2021

@SuperSandro2000 We'd have to add it to all links with fortify enabled, but just adding -lssp when fortify is given results in failed autoconf checks. Would need to track down the proper fix.

@Ericson2314
Copy link
Member

@alexfmpe and I were looking into this (see #132199). I think this is a good temp fix. I hope to fix it more properly with #132343 (which has been a long time coming!).

@alexfmpe
Copy link
Member

@SuperSandro2000 We'd have to add it to all links with fortify enabled, but just adding -lssp when fortify is given results in failed autoconf checks. Would need to track down the proper fix.

That's more or less the conclusion I reached in #132188 (comment)

@shlevy shlevy merged commit fcf5efa into NixOS:master Aug 12, 2021
@shlevy shlevy deleted the mingw-no-fortify branch August 12, 2021 01:24
@pstn
Copy link
Contributor

pstn commented Aug 12, 2021

Just for the record: I did not break pkgs.gmp as would be discoverable by nixpkgs-review, as @shlevy seems to feel the need to imply constantly. I assume some cross package he uses broke.

@shlevy
Copy link
Member Author

shlevy commented Aug 12, 2021

@pstn Apologies for the lack of precision, I did mean gmp cross-compiled to mingw in that commit.

I don't know what you mean by "feel the need to imply constantly." I just put that in one commit message to explain why this was needed now and not before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants