fix: ensure -zlazy for systems that default to -znow#1729
Conversation
Adds -zlazy to the go build wrapper and the 'go run' command for generating completions. Some systems like Gentoo default to znow if it is left unset. See: containers#1729 Signed-off-by: Alexandra Parker <alex.iris.parker@gmail.com>
xandris
force-pushed
the
bugfix/ensure-z-lazy
branch
from
59be97f to
73ed56e
Compare
|
Build failed. ❌ unit-test FAILURE in 1m 37s |
|
Ping. |
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for looking into this, @xandris , and my apologies for the delay! I always shudder at this linker hackery. :)
|
recheck |
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
It looks like this fixes #1706 Could you please link to the issue instead of the PR in the commit message?
xandris
force-pushed
the
bugfix/ensure-z-lazy
branch
from
73ed56e to
8811838
Compare
It's not necessary to prefix each external linker flag with its own -Wl option. One -Wl option can carry everything because the commas will be used to split the string. containers#1706
|
Thanks for updating this PR, @xandris ! Let me try to rebase this against |
debarshiray
force-pushed
the
bugfix/ensure-z-lazy
branch
from
8811838 to
ac96de9
Compare
Many operating systems default to '-z now' in their linker flags to harden binaries using Relocation Read-Only (or RELRO) [1]. Such as, Fedora [2,3] and Gentoo [4,5,6,7,8,9]. Some, like Fedora, pass it through the LDFLAGS environment variable that's usually injected into the clang(1) and gcc(1) compiler drivers' command line by build systems like Meson. Some, like Gentoo, use a Clang configuration file or GCC specs instead. The reference Go toolchain (ie., gc, not gccgo) in its external linking mode always uses the clang(1) or gcc(1) compiler drivers [10], and whether '-z now' gets used or not depends on which of the above mechanisms are in use. The Meson adapter scripts in Toolbx to connect Meson to 'go build' and 'go run', since Meson doesn't natively support Go [11], don't respect the LDFLAGS environment variable. So, anything passed through it gets filtered out from the linker flags. However, the default Clang configuration files or the built-in GCC specs can't be avoided. So, if '-z now' is specified there, then it gets used. This leads to a broken toolbox(1) binary and fails to generate the shell completions. [1] https://www.redhat.com/en/blog/hardening-elf-binaries-using-relocation-read-only-relro [2] Fedora redhat-rpm-config commit 796b80f2f49f2301 https://src.fedoraproject.org/rpms/redhat-rpm-config/c/796b80f2f49f2301 [3] Fedora redhat-rpm-config commit d9235d2d90873ff6 https://src.fedoraproject.org/rpms/redhat-rpm-config/c/d9235d2d90873ff6 https://bugzilla.redhat.com/show_bug.cgi?id=1192183 https://fedoraproject.org/wiki/Changes/Harden_All_Packages [4] Gentoo ebuild commit 8bfd8afef6dd8c66 https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8bfd8afef6dd8c66 https://bugs.gentoo.org/876923 [5] Gentoo gcc-patches commit 8ffd428773f8e1e6 gentoo/gcc-patches@8ffd428773f8e1e6 [6] Gentoo ebuild commit 718448f923ae9302 https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=718448f923ae9302 [7] Gentoo gcc-patches commit 98c1d96dbf03d704 gentoo/gcc-patches@98c1d96dbf03d704 https://bugs.gentoo.org/876923 [8] Gentoo ebuild commit adf44514cf590a86 https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=adf44514cf590a86 https://bugs.gentoo.org/876923 [9] Gentoo ebuild commit 975678507aa3cebb https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=975678507aa3cebb https://bugs.gentoo.org/876923 [10] https://pkg.go.dev/cmd/link [11] mesonbuild/meson#123 containers#1706 Signed-off-by: Alexandra Parker <alex.iris.parker@gmail.com>
|
I took the liberty to dump all the references you dug up for me in #1706 in the commit message for the sake of my future self. |
This should have been part of commit 83f28c5. containers#1706
|
👀 |
These are the same CI failures that showed up recently and is being debugged. See #1802 and #1805 So, let's ignore them here. |
|
Thanks for all the insight and investigation, @xandris ! |
3 participants
I see #1706 and #1722. this is more minimal change for systems that default to znow