Differential D28328
Enable PIE by default on 64-bit architectures Authored by mw on Jan 25 2021, 9:51 AM. Tags Referenced Files
Details This patch adds Position Independent Executables (PIE) The change is limited only for 64-bit architectures. Use bsd.opts.mk instead of the src.opts.mk in order
Diff Detail
Event TimelineComment Actions Hi, Do you have any feedback about this patch (and https://reviews.freebsd.org/D28370 + https://reviews.freebsd.org/D27666 as well)? Comment Actions Yes lets get this into -CURRENT soon. I've asked @imp to weigh in on the __T move. We should also add a note to UPDATING, something like: On 64-bit target architectures binaries are now built as Position-Independent Executables (PIE) by default. A WITHOUT_CLEAN build will not work across the transition so you will need to run make cleanworld, or set WITH_CLEAN for the first build. We'll also want to send a heads-up mail to -current suggesting things that users should watch for; it should be transparent for most things, but things like application launchers might misbehave, and we will want to check for ports build failures. We should also run this through an exp-run.
Comment Actions No. Let's not do this.
Comment Actions Adding a comment that was accidentally unsubmitted for a while
Comment Actions Hi, I have looked at *.mk files and if I am not mistaken it seems that there is currently no clean way of having a separate default option for PIE for base, as the __DEFAULT_YES_OPTIONS from bsd.opts.mk takes precedence and moving this option from bsd.opts.mk will leave us with no MK_PIE defined. The original purpose of this patch was to enable PIE in base and to enable it for ports later, when any issues that arise will be eliminated. Since I was tasked by @mw with fixing this patch, I want to ask how to fix this. Should I try to somehow work around this in src or would it be better to leave it enabled for everything and instead create additional patch which would allow ports to opt out in a clean way? While enabling this for everything was not the original intention, I think that in the long term enabling this for both ports and base as soon as possible could be beneficial. Comment Actions It's only O(20) ports using bmake that are affected, IMO the most straightforward path is going to be to fix that small number of ports. I'd guess the changes are going to fall into one or two different categories. Comment Actions Oh, one issue: when MK_PIE is true we still have non-pie static libs. This is the case with e.g. net/freevrrpd ld: error: can't create dynamic relocation R_X86_64_32 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output >>> defined in /usr/lib/libnetgraph.a(sock.o) >>> referenced by sock.c:78 (/usr/home/emaste/src/freebsd-git/laptop/lib/libnetgraph/sock.c:78) >>> sock.o:(NgMkSockNode) in archive /usr/lib/libnetgraph.a in bsd.lib.mk we create a special libfoo_pie.a if INTERNALLIB but /usr/lib/lib*.a is always non-PIE. Comment Actions That's not what I'm suggesting at all. I'm saying you should not move the TARGET stuff. MK_PIE Will be defined if you (a) always define it and have it marked BROKEN on 32-bit targets or (b) define it based on MACHINE_ARCH (a) seem to be undesirable, which is fine (I didn't know if the selected enabling was done for performance or functional reasons). Doing (b) is fine.
The original patch would not have accomplished that goal. TARGET_ARCH is not something we can test here to differentiate between the two. Comment Actions I agree. Having different defaults for src and ports is something I generally strongly oppose because it causes too much confusion unless there's a compelling reason to have the defaults be different. I think given the few ports that are affected, there's not really a compelling reason: we can do an exp run to clear this up.
Comment Actions Update the patch by:
The next step is to go over failing ports as listed in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253275 Comment Actions Thanks @imp ! Regarding the logistics, 2 questions:
Comment Actions Affected ports from the exp-run have been fixed, or have patches available and are waiting on maintainer approval. So, this should be ready to commit any time, although I think we should have a comment explaining which architectures have been opted-out (32-bit) and a brief sentence why.
Comment Actions Update revision:
Comment Actions
Ah, right - I was looking at a number of ports patches from dgr@semihalf and forgot this one was yours.
We don't have a dependency on resolving every last issue; only a couple of leaf ports are affected and patches are available for all of them. We can make sure they're all resolved before MFC though.
I think we should, but this is probably a separate discussion to have in a little while based on what we see after this is turned on in main. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||