Rebase of #14066 by luke-jr.
Let’s try to get PowerPC support in in the beginning of the 22.0 cycle so that it gets some testing, and is not a last-minute decision this time, like for last … 2 or 3 major versions.
The symbol/security tooling-related changes have been dropped since they were part of #20434.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
122- # Workaround for https://bugs.launchpad.net/ubuntu/+source/gcc-8-cross-ports/+bug/1853740
123- # TODO: remove this when no longer needed
124- HOST_LDFLAGS="${HOST_LDFLAGS_BASE} -Wl,-z,noexecstack"
125- else
126- HOST_LDFLAGS="${HOST_LDFLAGS_BASE}"
127- fi
So the reason that this is a workaround is that it’s not 100% safe. Normally the object files are supposed to flag that they support non-executable stack. Due to a bug in that verson of RISC-V gcc, this is not signaled.
This linker flag forces non-executable stack, overriding individual object files’ preference.
I’m not sure it’s a good idea to set this by default. If it’s required for PPC here, we should add a specific exception.
This is missing b74c21f …?
While rebasing it looked to me that that was only a change to the security script. But it seems not. Will add the gitian yml change.
Edit: Done, and removed 25cd539e756f6e2d514e21cfc902ff44abef7d10 “gitian: Always specify noexecstack since we enforce it unconditionally”, replacing it with a commit that extends the workaround for the powerpc architectures.
812@@ -813,6 +813,11 @@ AX_GCC_FUNC_ATTRIBUTE([dllimport])
813 if test x$use_glibc_compat != xno; then
814 AX_CHECK_LINK_FLAG([[-Wl,--wrap=__divmoddi4]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--wrap=__divmoddi4"])
815 AX_CHECK_LINK_FLAG([[-Wl,--wrap=log2f]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--wrap=log2f"])
816+ case $host in
817+ powerpc64* | ppc64*)
818+ AX_CHECK_LINK_FLAG([[-Wl,--no-tls-get-addr-optimize]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--no-tls-get-addr-optimize"])
Does this disable some optimization related to thread local storage? Is there a way to avoid that?
(just wondering why this is here; not an objection if it’s needed)
@luke-jr Yes, of course. I guess my question is: do we know whether that has any performance impact (presumably not much for now, as thread_local variables are only used for thread names AFAIK), and/or has the alternative of requiring a higher minimum glibc been considered?
No problem if the answer is we don’t know, or there are reasons for doing it this way. I just want to make sure we don’t end up years from now chasing a weird performance regression on POWER9 in release binaries.
FWIW in #18851 I had implemented the thread name system without use of TLS, which would have dropped that entire dependency.
That said, I doubt for our use (one query per log message?) a slightly slower TLS makes any noticeable difference in performance.
-mcpu=power9 instead of power8/970 respectively. I strongly suspect that everyone interested in these binaries wants to run them on Talos II workstations. But this also can be changed later, it doesn’t have to be decided in this PR.
129@@ -118,7 +130,7 @@ script: |
130 # Extract the git archive into a dir for each host and build
131 for i in ${HOSTS}; do
132 export PATH=${BASEPREFIX}/${i}/native/bin:${ORIGPATH}
133- if [ "${i}" = "riscv64-linux-gnu" ]; then
134+ if [ "${i}" = "riscv64-linux-gnu" ] || [ "${i}" = "powerpc64-linux-gnu" ] || [ "${i}" = "powerpc64le-linux-gnu" ]; then
Because that is unsafe, see rationale here: #20963 (review)
These lines can go away completely after gcc upgrade that fixes the underlying issue. But no sooner.
