Revert "[PATCH] RISC-V: Use new linker emulations for glibc ABI."

Message ID 20220615075943.1776786-1-maskray@google.com
State Deferred, archived
Headers
Series Revert "[PATCH] RISC-V: Use new linker emulations for glibc ABI." |

Commit Message

Fangrui Song June 15, 2022, 7:59 a.m. UTC
  This reverts commit 37d57ac9a636f2235f9060e84fb8dd7968abd1dc.

The resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=22962
let GCC pass -m emulation to ld and let the ld emulation configure
default library paths.  This scheme is problematic:

* It's not ld's business to specify default -L.  Different platforms have
different opinions on the hierarchy and all other arches work well without ld's
default -L.
* If some ABI derived library paths are desired, the compiler driver is in a
better position to make the decision and traditionally has done this.
* -m emulation is opaque to the compiler driver.  It doesn't affect -B, so
data files like crt*.o, libasan_preinit.o, and libtsan_preinit.o are not affected.

As is, many platforms just use symlinks to fake the lib64/{ilp32{,f},lp64{,f}}
hierarchies needed by the GNU ld emulation.  They can always specify -L
explicitly if they want some ABI derived library paths.  See also the rejected
https://reviews.llvm.org/D95755

gcc/Changelog:

    * config/riscv/linux.h (LD_EMUL_SUFFIX): Remove.
    (LINK_SPEC): Remove LD_EMUL_SUFFIX.
---
 gcc/config/riscv/linux.h | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
  

Comments

Kito Cheng June 20, 2022, 7:21 a.m. UTC | #1
Generally I agree we should fix that by GCC driver rather than ld
emulation, but I think this should be reverted with the -L path fix,
otherwise that will break multilib on GNU toolchain for linux
immediately?

On Wed, Jun 15, 2022 at 4:00 PM Fangrui Song via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This reverts commit 37d57ac9a636f2235f9060e84fb8dd7968abd1dc.
>
> The resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=22962
> let GCC pass -m emulation to ld and let the ld emulation configure
> default library paths.  This scheme is problematic:
>
> * It's not ld's business to specify default -L.  Different platforms have
> different opinions on the hierarchy and all other arches work well without ld's
> default -L.
> * If some ABI derived library paths are desired, the compiler driver is in a
> better position to make the decision and traditionally has done this.
> * -m emulation is opaque to the compiler driver.  It doesn't affect -B, so
> data files like crt*.o, libasan_preinit.o, and libtsan_preinit.o are not affected.
>
> As is, many platforms just use symlinks to fake the lib64/{ilp32{,f},lp64{,f}}
> hierarchies needed by the GNU ld emulation.  They can always specify -L
> explicitly if they want some ABI derived library paths.  See also the rejected
> https://reviews.llvm.org/D95755
>
> gcc/Changelog:
>
>     * config/riscv/linux.h (LD_EMUL_SUFFIX): Remove.
>     (LINK_SPEC): Remove LD_EMUL_SUFFIX.
> ---
>  gcc/config/riscv/linux.h | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
> index 38803723ba9..e0ff6e6a178 100644
> --- a/gcc/config/riscv/linux.h
> +++ b/gcc/config/riscv/linux.h
> @@ -49,16 +49,8 @@ along with GCC; see the file COPYING3.  If not see
>
>  #define CPP_SPEC "%{pthread:-D_REENTRANT}"
>
> -#define LD_EMUL_SUFFIX \
> -  "%{mabi=lp64d:}" \
> -  "%{mabi=lp64f:_lp64f}" \
> -  "%{mabi=lp64:_lp64}" \
> -  "%{mabi=ilp32d:}" \
> -  "%{mabi=ilp32f:_ilp32f}" \
> -  "%{mabi=ilp32:_ilp32}"
> -
>  #define LINK_SPEC "\
> --melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
> +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
>  %{mno-relax:--no-relax} \
>  %{mbig-endian:-EB} \
>  %{mlittle-endian:-EL} \
> --
> 2.36.1.476.g0c4daa206d-goog
>
  
Fangrui Song June 21, 2022, 3:48 a.m. UTC | #2
On Mon, Jun 20, 2022 at 1:21 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Generally I agree we should fix that by GCC driver rather than ld
> emulation, but I think this should be reverted with the -L path fix,
> otherwise that will break multilib on GNU toolchain for linux
> immediately?

Thanks for the good consideration. That said, I am unsure any distro
uses this currently.
I think some just work around the possibly non-existent paths by
creating symlinks.
Perhaps we should prioritize on fixing the scheme before distros start
to rely on the behavior.

> On Wed, Jun 15, 2022 at 4:00 PM Fangrui Song via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > This reverts commit 37d57ac9a636f2235f9060e84fb8dd7968abd1dc.
> >
> > The resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=22962
> > let GCC pass -m emulation to ld and let the ld emulation configure
> > default library paths.  This scheme is problematic:
> >
> > * It's not ld's business to specify default -L.  Different platforms have
> > different opinions on the hierarchy and all other arches work well without ld's
> > default -L.
> > * If some ABI derived library paths are desired, the compiler driver is in a
> > better position to make the decision and traditionally has done this.
> > * -m emulation is opaque to the compiler driver.  It doesn't affect -B, so
> > data files like crt*.o, libasan_preinit.o, and libtsan_preinit.o are not affected.
> >
> > As is, many platforms just use symlinks to fake the lib64/{ilp32{,f},lp64{,f}}
> > hierarchies needed by the GNU ld emulation.  They can always specify -L
> > explicitly if they want some ABI derived library paths.  See also the rejected
> > https://reviews.llvm.org/D95755
> >
> > gcc/Changelog:
> >
> >     * config/riscv/linux.h (LD_EMUL_SUFFIX): Remove.
> >     (LINK_SPEC): Remove LD_EMUL_SUFFIX.
> > ---
> >  gcc/config/riscv/linux.h | 10 +---------
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
> > index 38803723ba9..e0ff6e6a178 100644
> > --- a/gcc/config/riscv/linux.h
> > +++ b/gcc/config/riscv/linux.h
> > @@ -49,16 +49,8 @@ along with GCC; see the file COPYING3.  If not see
> >
> >  #define CPP_SPEC "%{pthread:-D_REENTRANT}"
> >
> > -#define LD_EMUL_SUFFIX \
> > -  "%{mabi=lp64d:}" \
> > -  "%{mabi=lp64f:_lp64f}" \
> > -  "%{mabi=lp64:_lp64}" \
> > -  "%{mabi=ilp32d:}" \
> > -  "%{mabi=ilp32f:_ilp32f}" \
> > -  "%{mabi=ilp32:_ilp32}"
> > -
> >  #define LINK_SPEC "\
> > --melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
> > +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
> >  %{mno-relax:--no-relax} \
> >  %{mbig-endian:-EB} \
> >  %{mlittle-endian:-EL} \
> > --
> > 2.36.1.476.g0c4daa206d-goog
> >
  
Palmer Dabbelt July 14, 2022, 8:41 p.m. UTC | #3
On Mon, 20 Jun 2022 20:48:50 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> On Mon, Jun 20, 2022 at 1:21 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>>
>> Generally I agree we should fix that by GCC driver rather than ld
>> emulation, but I think this should be reverted with the -L path fix,
>> otherwise that will break multilib on GNU toolchain for linux
>> immediately?

IIUC just changing this will break even non-multlib systems, though it's 
possible that the symlinks work around that sufficiently.

> Thanks for the good consideration. That said, I am unsure any distro
> uses this currently.
> I think some just work around the possibly non-existent paths by
> creating symlinks.
> Perhaps we should prioritize on fixing the scheme before distros start
> to rely on the behavior.

I'm kind of torn on this one: this has been around for a while and 
dropping it would be an ABI break, but the feedback from distro folks is 
pretty consistently that multlib is broken on RISC-V.  If it's really 
unusably broken then I could buy the argument that there's no binaries 
(and thus no ABI to break), but there's at some base multilib 
functionality working -- I build multilib cross toolchains regularly, 
for example, and they can build simple stuff.

I always find making that "nobody's used it" argument really hard, 
there's just too many users to try and track everyone down.  We're in 
kind of a weird spot with RISC-V in general when it comes to ABI stuff: 
we were probably a bit overly optimistic about how fast any of this was 
going to get used when we committed to the ABI freeze, but any ABi break 
has such a huge potential for user headaches that I'm not sure it's 
going to be possible.  It means we're stuck with some baggage, and while 
it's a headache to keep around stuff that's probably not all that useful 
I think it's just what we've got to live with.

If multlib really is so broken it's not fixable without an ABI break 
then I guess there's no other option, but I think in this case we have 
some:

One option would be to add an ld argument that says to turn off the 
emulation-specific path resolution, which we could then add to LINK_SPEC 
when we get the library paths sorted out?  We'd still have the 
emulations and the subdirs, but at least we wouldn't need a flag day.

Another option would be to add new multlib paths that don't have the 
subdirectories, as last I checked that was an issue for distros 
(violates FHS, breaks build systems, etc).  If we're going to do that 
anyway then we could piggyback the new behavior on it and deprecate the old 
paths along with whatever behavior is associated with them.

>> On Wed, Jun 15, 2022 at 4:00 PM Fangrui Song via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>> >
>> > This reverts commit 37d57ac9a636f2235f9060e84fb8dd7968abd1dc.
>> >
>> > The resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=22962
>> > let GCC pass -m emulation to ld and let the ld emulation configure
>> > default library paths.  This scheme is problematic:
>> >
>> > * It's not ld's business to specify default -L.  Different platforms have
>> > different opinions on the hierarchy and all other arches work well without ld's
>> > default -L.
>> > * If some ABI derived library paths are desired, the compiler driver is in a
>> > better position to make the decision and traditionally has done this.
>> > * -m emulation is opaque to the compiler driver.  It doesn't affect -B, so
>> > data files like crt*.o, libasan_preinit.o, and libtsan_preinit.o are not affected.
>> >
>> > As is, many platforms just use symlinks to fake the lib64/{ilp32{,f},lp64{,f}}
>> > hierarchies needed by the GNU ld emulation.  They can always specify -L
>> > explicitly if they want some ABI derived library paths.  See also the rejected
>> > https://reviews.llvm.org/D95755

I don't do a lot of LLVM stuff, but that has a green check mark that 
says "accepted" at the top.  Does that mean it was merged somewhere, or 
just that it was acked/reviewed and then dropped?

>> >
>> > gcc/Changelog:
>> >
>> >     * config/riscv/linux.h (LD_EMUL_SUFFIX): Remove.
>> >     (LINK_SPEC): Remove LD_EMUL_SUFFIX.
>> > ---
>> >  gcc/config/riscv/linux.h | 10 +---------
>> >  1 file changed, 1 insertion(+), 9 deletions(-)
>> >
>> > diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
>> > index 38803723ba9..e0ff6e6a178 100644
>> > --- a/gcc/config/riscv/linux.h
>> > +++ b/gcc/config/riscv/linux.h
>> > @@ -49,16 +49,8 @@ along with GCC; see the file COPYING3.  If not see
>> >
>> >  #define CPP_SPEC "%{pthread:-D_REENTRANT}"
>> >
>> > -#define LD_EMUL_SUFFIX \
>> > -  "%{mabi=lp64d:}" \
>> > -  "%{mabi=lp64f:_lp64f}" \
>> > -  "%{mabi=lp64:_lp64}" \
>> > -  "%{mabi=ilp32d:}" \
>> > -  "%{mabi=ilp32f:_ilp32f}" \
>> > -  "%{mabi=ilp32:_ilp32}"
>> > -
>> >  #define LINK_SPEC "\
>> > --melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
>> > +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
>> >  %{mno-relax:--no-relax} \
>> >  %{mbig-endian:-EB} \
>> >  %{mlittle-endian:-EL} \
>> > --
>> > 2.36.1.476.g0c4daa206d-goog
>> >
>
>
>
> -- 
> 宋方睿
  
Fangrui Song July 14, 2022, 9:02 p.m. UTC | #4
On 2022-07-14, Palmer Dabbelt wrote:
>On Mon, 20 Jun 2022 20:48:50 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
>>On Mon, Jun 20, 2022 at 1:21 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>>>
>>>Generally I agree we should fix that by GCC driver rather than ld
>>>emulation, but I think this should be reverted with the -L path fix,
>>>otherwise that will break multilib on GNU toolchain for linux
>>>immediately?
>
>IIUC just changing this will break even non-multlib systems, though 
>it's possible that the symlinks work around that sufficiently.
>
>>Thanks for the good consideration. That said, I am unsure any distro
>>uses this currently.
>>I think some just work around the possibly non-existent paths by
>>creating symlinks.
>>Perhaps we should prioritize on fixing the scheme before distros start
>>to rely on the behavior.
>
>I'm kind of torn on this one: this has been around for a while and 
>dropping it would be an ABI break, but the feedback from distro folks 
>is pretty consistently that multlib is broken on RISC-V.  If it's 
>really unusably broken then I could buy the argument that there's no 
>binaries (and thus no ABI to break), but there's at some base multilib 
>functionality working -- I build multilib cross toolchains regularly, 
>for example, and they can build simple stuff.
>
>I always find making that "nobody's used it" argument really hard, 
>there's just too many users to try and track everyone down.  We're in 
>kind of a weird spot with RISC-V in general when it comes to ABI 
>stuff: we were probably a bit overly optimistic about how fast any of 
>this was going to get used when we committed to the ABI freeze, but 
>any ABi break has such a huge potential for user headaches that I'm 
>not sure it's going to be possible.  It means we're stuck with some 
>baggage, and while it's a headache to keep around stuff that's 
>probably not all that useful I think it's just what we've got to live 
>with.
>
>If multlib really is so broken it's not fixable without an ABI break 
>then I guess there's no other option, but I think in this case we have 
>some:
>
>One option would be to add an ld argument that says to turn off the 
>emulation-specific path resolution, which we could then add to 
>LINK_SPEC when we get the library paths sorted out?  We'd still have 
>the emulations and the subdirs, but at least we wouldn't need a flag 
>day.
>
>Another option would be to add new multlib paths that don't have the 
>subdirectories, as last I checked that was an issue for distros 
>(violates FHS, breaks build systems, etc).  If we're going to do that 
>anyway then we could piggyback the new behavior on it and deprecate 
>the old paths along with whatever behavior is associated with them.

Thanks for chiming in and providing the good consideration.  I don't do
much GNU development.  I just spotted something and think it should be
fixed.  If dropping -L is considered a risk, an alternative scheme is
that someone:

* create an alternative patch making GCC driver pass the -L to ld and use the plain emulation name.
* a configure option can possibly be added to guard whether the -L is added at all
* drop the excessive emulations in GNU ld.

This way an alternative linker implementation doesn't have to add a
compatibility no-op option.

I appreciate anyone who wants to step up and helps removing
eelf64lriscv_{lp64,ilp32f,...}. We will have "*64briscv*" variants and
it will then look really ugly.

>>>On Wed, Jun 15, 2022 at 4:00 PM Fangrui Song via Gcc-patches
>>><gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>> This reverts commit 37d57ac9a636f2235f9060e84fb8dd7968abd1dc.
>>>>
>>>> The resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=22962
>>>> let GCC pass -m emulation to ld and let the ld emulation configure
>>>> default library paths.  This scheme is problematic:
>>>>
>>>> * It's not ld's business to specify default -L.  Different platforms have
>>>> different opinions on the hierarchy and all other arches work well without ld's
>>>> default -L.
>>>> * If some ABI derived library paths are desired, the compiler driver is in a
>>>> better position to make the decision and traditionally has done this.
>>>> * -m emulation is opaque to the compiler driver.  It doesn't affect -B, so
>>>> data files like crt*.o, libasan_preinit.o, and libtsan_preinit.o are not affected.
>>>>
>>>> As is, many platforms just use symlinks to fake the lib64/{ilp32{,f},lp64{,f}}
>>>> hierarchies needed by the GNU ld emulation.  They can always specify -L
>>>> explicitly if they want some ABI derived library paths.  See also the rejected
>>>> https://reviews.llvm.org/D95755
>
>I don't do a lot of LLVM stuff, but that has a green check mark that 
>says "accepted" at the top.  Does that mean it was merged somewhere, 
>or just that it was acked/reviewed and then dropped?

I could land it, but I realized that would be a mistake so I stopped.
There has since been no real need for this feature, either.  I wished
that some RISC-V folks stepped up and fixed it.  It took so long so I
ended up sending this patch myself, but I have no bandwidth to change this
patch to a form providing better compatibility....

>>>>
>>>> gcc/Changelog:
>>>>
>>>>     * config/riscv/linux.h (LD_EMUL_SUFFIX): Remove.
>>>>     (LINK_SPEC): Remove LD_EMUL_SUFFIX.
>>>> ---
>>>>  gcc/config/riscv/linux.h | 10 +---------
>>>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>>>
>>>> diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
>>>> index 38803723ba9..e0ff6e6a178 100644
>>>> --- a/gcc/config/riscv/linux.h
>>>> +++ b/gcc/config/riscv/linux.h
>>>> @@ -49,16 +49,8 @@ along with GCC; see the file COPYING3.  If not see
>>>>
>>>>  #define CPP_SPEC "%{pthread:-D_REENTRANT}"
>>>>
>>>> -#define LD_EMUL_SUFFIX \
>>>> -  "%{mabi=lp64d:}" \
>>>> -  "%{mabi=lp64f:_lp64f}" \
>>>> -  "%{mabi=lp64:_lp64}" \
>>>> -  "%{mabi=ilp32d:}" \
>>>> -  "%{mabi=ilp32f:_ilp32f}" \
>>>> -  "%{mabi=ilp32:_ilp32}"
>>>> -
>>>>  #define LINK_SPEC "\
>>>> --melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
>>>> +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
>>>>  %{mno-relax:--no-relax} \
>>>>  %{mbig-endian:-EB} \
>>>>  %{mlittle-endian:-EL} \
>>>> --
>>>> 2.36.1.476.g0c4daa206d-goog
>>>>
>>
>>
>>
>>-- 
>>宋方睿
  
Andreas K. Huettel July 17, 2022, 1:57 p.m. UTC | #5
> I'm kind of torn on this one: this has been around for a while and 
> dropping it would be an ABI break, but the feedback from distro folks is 
> pretty consistently that multlib is broken on RISC-V.  If it's really 
> unusably broken then I could buy the argument that there's no binaries 
> (and thus no ABI to break), but there's at some base multilib 
> functionality working -- I build multilib cross toolchains regularly, 
> for example, and they can build simple stuff.

So as the one who initially bootstrapped rv64 for Gentoo in full multilib 
mode...

1) Multilib works in principle.

2) The main problem with the lib64/lp64d etc paths is that software authors
out there assume that the "libdir" is a single-depth directory. 
In Gentoo specifically, "normally" $(get_libdir) echos "lib64" and people
assume that "$(get_libdir)/../" ends up at a specific location. Now if 
$(get_libdir) echos "lib64/lp64" ...
That is not limited to the Gentoo side though.

3) The second problem is that binary-distributed software, built for
non-multilib lp64d (hey, rust, I'm talking of you), of course use
"lib64" and not "lib64/lp64d".

4) The third problem (more of an oddity) is that for rv32 the non-multilib
fallback for the "lib32/ilp32d" directories is not "lib32" but "lib".

With incompatible changes some time ago (the 17.0 to 20.0 profile change), 
2) and 3) was "fixed" in Gentoo (where we officially only do rv64 so far
because usermode qemu-riscv32 is b0rk b0rk b0rk). What I did:

* The main ABI (normally lp64d) was moved to lib64
  https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/multilib.eclass#n419
* Compat symlinks, e.g. /usr/lib64/lp64d -> . were installed.

This 
* *mostly* solves 2) because only a small subset of libraries is
  installed for the non-default ABI, 
* and solves 3) because the default ABI is in the fallback non-multilib
  location (but also accessible via the symlink if something insists).

https://gentoo.osuosl.org/releases/riscv/autobuilds/

Given these hacks, our multilib stages are not linked on the website, 
but they are built just fine and should work just fine. Feel free to
try in a chroot / with qemu / ...


> I always find making that "nobody's used it" argument really hard, 
> there's just too many users to try and track everyone down.  We're in 
> kind of a weird spot with RISC-V in general when it comes to ABI stuff: 
> we were probably a bit overly optimistic about how fast any of this was 
> going to get used when we committed to the ABI freeze, but any ABi break 
> has such a huge potential for user headaches that I'm not sure it's 
> going to be possible.  It means we're stuck with some baggage, and while 
> it's a headache to keep around stuff that's probably not all that useful 
> I think it's just what we've got to live with.
  

Patch

diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
index 38803723ba9..e0ff6e6a178 100644
--- a/gcc/config/riscv/linux.h
+++ b/gcc/config/riscv/linux.h
@@ -49,16 +49,8 @@  along with GCC; see the file COPYING3.  If not see
 
 #define CPP_SPEC "%{pthread:-D_REENTRANT}"
 
-#define LD_EMUL_SUFFIX \
-  "%{mabi=lp64d:}" \
-  "%{mabi=lp64f:_lp64f}" \
-  "%{mabi=lp64:_lp64}" \
-  "%{mabi=ilp32d:}" \
-  "%{mabi=ilp32f:_ilp32f}" \
-  "%{mabi=ilp32:_ilp32}"
-
 #define LINK_SPEC "\
--melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
+-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
 %{mno-relax:--no-relax} \
 %{mbig-endian:-EB} \
 %{mlittle-endian:-EL} \