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.

Sign up for GitHub

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

Jump to bottom

WIP: improve support for crosscompiling to musl32 #62817

Closed
wants to merge 8 commits into from
Closed

WIP: improve support for crosscompiling to musl32 #62817

wants to merge 8 commits into from

Conversation

wfranzini
Copy link
Contributor

@wfranzini wfranzini commented Jun 7, 2019 •
edited
Loading

Motivation for this change

Crosscompiling packages to musl32 lead to compilation errors due to the missing __stack_chk_fail_local symbol. Even building nix fails. This PR disable stackprotector for musl32.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

wfranzini added 2 commits June 7, 2019 10:07
@wfranzini
disable stackprotector on target platform musl32
db5d5e2
@wfranzini
libexecinfo: enables stackprotector through hardeningEnable
ee2ad96 
The makefile patch pulled from alpinelinux used to enable
stackprotector unconditionally.  This change use the nixpkgs flag
hardeningEnable.

This is required to support platform where stackprotector has
problems.
@wfranzini wfranzini requested a review from Ericson2314 as a code owner June 7, 2019 08:28
@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/crosscompilation-to-musl32-problems/3110/5

@wfranzini
audit: backport from upstream support for system w/o strndupa/rawmemchr
3a163f9 
The patches from alpinelinux failed to apply to v2.8.5, on the other
hand upstream has added support for musl based system in the master
branch on github [1].

The patch has been added to the nixpkgs repo since it needs to be
modified slightly, a copyright notice has been removed, to apply
cleanly.

1. linux-audit/audit-userspace@d579a08
@ofborg ofborg bot requested review from dtzWill, 7c6f434c and Fuuzetsu June 7, 2019 12:35
@wfranzini
bison2: add perl in nativeBuildInputs
63762fc 
Without this change crosscompiling fails because perl is not found
during configure.
@wfranzini
sox: musl does not support pipe rewind
b509a5f 
Disable the pipe rewind related feature since musl has no support for it.
matthewbauer
matthewbauer reviewed Jun 7, 2019
View reviewed changes
pkgs/development/libraries/libexecinfo/default.nix
url = https://git.alpinelinux.org/cgit/aports/plain/main/libexecinfo/30-linux-makefile.patch?id=730cdcef6901750f4029d4c3b8639ce02ee3ead1;
sha256 = "1jwjz22z5cjy5h2bfghn62yl9ar8jiqhdvbwrcfavv17ihbhwcaf";
})
./30-linux-makefile.patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if alpine has a way to get ssp working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewbauer Alpinelinux creates a small libssp_nonshared library in the musl-dev package.
https://git.alpinelinux.org/aports/tree/main/musl?h=master

I've recreated it in my local nixpkgs repo, however I'm not sure how to automatically link with libssp_nonshared.a.

Do you have a hint?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to put it in musl's $out/lib/libssp_nonshared.a. Unfortunately we also need to apply this patch to GCC:

https://git.alpinelinux.org/aports/tree/main/gcc/gcc-6.1-musl-libssp.patch

That would probably be okay to apply to GCC, but maybe only on i686-musl builds.

@FRidh FRidh added the 2.status: work-in-progress This PR isn't done label Jul 17, 2019
@7c6f434c
Copy link
Member

For context: is this problem irrelevant in 64-bit case?

@matthewbauer
Copy link
Member

For context: is this problem irrelevant in 64-bit case?

I have never seen it on x86_64. I think it could be possible to hit it with some messy linking, but I would prefer to limit the patching to just i*86 for now.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 23, 2019
@wfranzini
add stackprotector support on musl32
8b7219b 
In order to support stackprotector on musl32, this change import a
couple of patches from alpinelinux:
1. libssp_nonshared.a is built alongside musl's libc
2. the above library is automatically linked when compiling with gcc6
   or gcc7
@matthewbauer matthewbauer mentioned this pull request Aug 28, 2019
Closed
@symphorien
Copy link
Member

I successfully used this PR to compile openssl.

@Ericson2314
Copy link
Member

Can we submit some of these patches upstream and fetchpatch them?

@wfranzini
Copy link
Contributor Author

Can we submit some of these patches upstream and fetchpatch them?

@Ericson2314 I'll look into this the next week.

I would like to have this PR merged somewhere in the future, but I need guidance about the correct branch to use since I fear it will cause a massive rebuild.

@flokli
Copy link
Contributor

flokli commented Nov 8, 2019

@wfranzini this should target the staging branch.

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@symphorien
Copy link
Member

I would really like to see this merged in one form or another.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
7c6f434c
7c6f434c reviewed Jun 3, 2020
View reviewed changes
pkgs/os-specific/linux/audit/audit-strndupa-rawmemchr-compat.patch
From d579a08bb1cde71f939c13ac6b2261052ae9f77e Mon Sep 17 00:00:00 2001
From: Steve Grubb <[email protected]>
Date: Tue, 26 Feb 2019 18:33:33 -0500
Subject: [PATCH] Add substitue functions for strndupa & rawmemchr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Subject: [PATCH] Add substitue functions for strndupa & rawmemchr
Subject: [PATCH] Add substitute functions for strndupa & rawmemchr

@wfranzini
Copy link
Contributor Author

Later today, I'm going to open a new PR starting from staging.

@wfranzini wfranzini mentioned this pull request Jun 4, 2020
Closed
10 tasks
@wfranzini
Copy link
Contributor Author

I'll continue working on the cross compilation to musl32 in PR #89471

@wfranzini wfranzini closed this Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli left review comments

@matthewbauer matthewbauer matthewbauer left review comments

@7c6f434c 7c6f434c 7c6f434c approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@dtzWill dtzWill Awaiting requested review from dtzWill

@Fuuzetsu Fuuzetsu Awaiting requested review from Fuuzetsu

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: work-in-progress This PR isn't done 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@wfranzini @nixos-discourse @7c6f434c @matthewbauer @symphorien @Ericson2314 @flokli @FRidh