[gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[Theo Anderson]

Hello, please see the below patch to support disabling ld.lld like
ld.gold. This has not been split into a separate function
such as tc-ld-disable-lld(), as I do not believe there is a use case
where ld.gold is supported and ld.lld is not.

Thanks.

Pull-request: https://github.com/gentoo/gentoo/pull/19116

From c0894e304cbd209ab2cf6b3754f75d9bfd93634a Mon Sep 17 00:00:00 2001
From: Theo Anderson <telans@posteo.de>
Date: Thu, 21 Jan 2021 09:59:09 +1300
Subject: [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

No functions currently force bfd usage when lld is active.
This function takes over tc-ld-disable-gold so that any current
 calls to tc-ld-disable-gold will also disable lld. With ld.ldd
 this fixes configure for packages like sys-libs/db where
 --default-symver is added to ldflags.
tc-ld-disable-gold has been marked as deprecated and should be
 replaced by tc-ld-force-bfd within ebuilds.

Package-Manager: Portage-3.0.14, Repoman-3.0.2
Signed-off-by: Theo Anderson <telans@posteo.de>
---
 eclass/toolchain-funcs.eclass | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/eclass/toolchain-funcs.eclass
b/eclass/toolchain-funcs.eclass index 4a4bb27fc08..2cf7ddfb790 100644
--- a/eclass/toolchain-funcs.eclass
+++ b/eclass/toolchain-funcs.eclass
@@ -1,4 +1,4 @@
-# Copyright 2002-2019 Gentoo Authors
+# Copyright 2002-2021 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: toolchain-funcs.eclass
@@ -502,15 +502,26 @@ tc-ld-is-lld() {
 # @FUNCTION: tc-ld-disable-gold
 # @USAGE: [toolchain prefix]
 # @DESCRIPTION:
+# Deprecated in favor of tc-ld-force-bfd.
+#
 # If the gold linker is currently selected, configure the compilation
 # settings so that we use the older bfd linker instead.
 tc-ld-disable-gold() {
-	if ! tc-ld-is-gold "$@" ; then
-		# They aren't using gold, so nothing to do!
+	tc-ld-force-bfd "$@"
+}
+
+# @FUNCTION: tc-ld-force-bfd
+# @USAGE: [toolchain prefix]
+# @DESCRIPTION:
+# If the gold or lld linker is currently selected, configure the
compilation +# settings so that we use the bfd linker instead.
+tc-ld-force-bfd() {
+	if ! tc-ld-is-gold "$@" && ! tc-ld-is-lld "$@" ; then
+		# They aren't using gold or lld, so nothing to do!
 		return
 	fi
 
-	ewarn "Forcing usage of the BFD linker instead of GOLD"
+	ewarn "Forcing usage of the BFD linker"
 
 	# Set up LD to point directly to bfd if it's available.
 	# We need to extract the first word in case there are flags
appended @@ -520,7 +531,7 @@ tc-ld-disable-gold() {
 	local path_ld=$(which "${bfd_ld}" 2>/dev/null)
 	[[ -e ${path_ld} ]] && export LD=${bfd_ld}
 
-	# Set up LDFLAGS to select gold based on the gcc / clang
version.
+	# Set up LDFLAGS to select bfd based on the gcc / clang
version. local fallback="true"
 	if tc-is-gcc; then
 		local major=$(gcc-major-version "$@")
@@ -548,7 +559,7 @@ tc-ld-disable-gold() {
 			ln -sf "${path_ld}" "${d}"/ld
 			export LDFLAGS="${LDFLAGS} -B${d}"
 		else
-			die "unable to locate a BFD linker to bypass
gold"
+			die "unable to locate a BFD linker"
 		fi
 	fi
 }
-- 
2.30.0

Re: [gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[Manoj Gupta]

[-- Attachment #1: Type: text/plain, Size: 4350 bytes --]

On Wed, Jan 20, 2021 at 1:01 PM Theo Anderson <telans@posteo.de> wrote:

> Hello, please see the below patch to support disabling ld.lld like
> ld.gold. This has not been split into a separate function
> such as tc-ld-disable-lld(), as I do not believe there is a use case
> where ld.gold is supported and ld.lld is not.
>
> Thanks.
>
> Pull-request: https://github.com/gentoo/gentoo/pull/19116
>
>
I am not a Gentoo maintainer but this forces bfd linker for the ebuilds
when gold is not even used e.g. lld is default linker. I am curious how
many places where gold is disabled do not work with lld.
In my experience, LLD is far more compatible with bfd than gold e.g. it can
link Linux kernels. So, imo we should not disable lld as a side effect when
the compatibility problem is with gold only.
i.e. It is ok to add a function to force bfd but disabling gold needs to
have a check if gold is the current linker.

My preference us to add 2 functions:
tc-ld-force-bfd
tc-ld-disable-lld

And tc-ld-disable-gold should check if gold is the current linker. If not,
only then force bfd.

What do the maintainers think?

Thanks,
Manoj




> From c0894e304cbd209ab2cf6b3754f75d9bfd93634a Mon Sep 17 00:00:00 2001
> From: Theo Anderson <telans@posteo.de>
> Date: Thu, 21 Jan 2021 09:59:09 +1300
> Subject: [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()
>
> No functions currently force bfd usage when lld is active.
> This function takes over tc-ld-disable-gold so that any current
>  calls to tc-ld-disable-gold will also disable lld. With ld.ldd
>  this fixes configure for packages like sys-libs/db where
>  --default-symver is added to ldflags.
> tc-ld-disable-gold has been marked as deprecated and should be
>  replaced by tc-ld-force-bfd within ebuilds.
>
> Package-Manager: Portage-3.0.14, Repoman-3.0.2
> Signed-off-by: Theo Anderson <telans@posteo.de>
> ---
>  eclass/toolchain-funcs.eclass | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/eclass/toolchain-funcs.eclass
> b/eclass/toolchain-funcs.eclass index 4a4bb27fc08..2cf7ddfb790 100644
> --- a/eclass/toolchain-funcs.eclass
> +++ b/eclass/toolchain-funcs.eclass
> @@ -1,4 +1,4 @@
> -# Copyright 2002-2019 Gentoo Authors
> +# Copyright 2002-2021 Gentoo Authors
>  # Distributed under the terms of the GNU General Public License v2
>
>  # @ECLASS: toolchain-funcs.eclass
> @@ -502,15 +502,26 @@ tc-ld-is-lld() {
>  # @FUNCTION: tc-ld-disable-gold
>  # @USAGE: [toolchain prefix]
>  # @DESCRIPTION:
> +# Deprecated in favor of tc-ld-force-bfd.
> +#
>  # If the gold linker is currently selected, configure the compilation
>  # settings so that we use the older bfd linker instead.
>  tc-ld-disable-gold() {
> -       if ! tc-ld-is-gold "$@" ; then
> -               # They aren't using gold, so nothing to do!
> +       tc-ld-force-bfd "$@"
> +}
> +
> +# @FUNCTION: tc-ld-force-bfd
> +# @USAGE: [toolchain prefix]
> +# @DESCRIPTION:
> +# If the gold or lld linker is currently selected, configure the
> compilation +# settings so that we use the bfd linker instead.
> +tc-ld-force-bfd() {
> +       if ! tc-ld-is-gold "$@" && ! tc-ld-is-lld "$@" ; then
> +               # They aren't using gold or lld, so nothing to do!
>                 return
>         fi
>
> -       ewarn "Forcing usage of the BFD linker instead of GOLD"
> +       ewarn "Forcing usage of the BFD linker"
>
>         # Set up LD to point directly to bfd if it's available.
>         # We need to extract the first word in case there are flags
> appended @@ -520,7 +531,7 @@ tc-ld-disable-gold() {
>         local path_ld=$(which "${bfd_ld}" 2>/dev/null)
>         [[ -e ${path_ld} ]] && export LD=${bfd_ld}
>
> -       # Set up LDFLAGS to select gold based on the gcc / clang
> version.
> +       # Set up LDFLAGS to select bfd based on the gcc / clang
> version. local fallback="true"
>         if tc-is-gcc; then
>                 local major=$(gcc-major-version "$@")
> @@ -548,7 +559,7 @@ tc-ld-disable-gold() {
>                         ln -sf "${path_ld}" "${d}"/ld
>                         export LDFLAGS="${LDFLAGS} -B${d}"
>                 else
> -                       die "unable to locate a BFD linker to bypass
> gold"
> +                       die "unable to locate a BFD linker"
>                 fi
>         fi
>  }
> --
> 2.30.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 5862 bytes --]

Re: [gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[Mike Gilbert]

On Wed, Jan 20, 2021 at 9:41 PM Manoj Gupta <manojgupta@google.com> wrote:
>
>
>
> On Wed, Jan 20, 2021 at 1:01 PM Theo Anderson <telans@posteo.de> wrote:
>>
>> Hello, please see the below patch to support disabling ld.lld like
>> ld.gold. This has not been split into a separate function
>> such as tc-ld-disable-lld(), as I do not believe there is a use case
>> where ld.gold is supported and ld.lld is not.
>>
>> Thanks.
>>
>> Pull-request: https://github.com/gentoo/gentoo/pull/19116
>>
>
> I am not a Gentoo maintainer but this forces bfd linker for the ebuilds when gold is not even used e.g. lld is default linker. I am curious how many places where gold is disabled do not work with lld.
> In my experience, LLD is far more compatible with bfd than gold e.g. it can link Linux kernels. So, imo we should not disable lld as a side effect when the compatibility problem is with gold only.
> i.e. It is ok to add a function to force bfd but disabling gold needs to have a check if gold is the current linker.
>
> My preference us to add 2 functions:
> tc-ld-force-bfd
> tc-ld-disable-lld
>
> And tc-ld-disable-gold should check if gold is the current linker. If not, only then force bfd.
>
> What do the maintainers think?

Please see bug 729510 for an example where gold and lld do not work,
but bfd does. That bug precipitated this change in the first place.

I don't think there are any cases where we want to disable lld without
disabling gold. Maybe it would suffice to un-deprecate
tc-ld-disable-gold and only have it call tc-ld-force-bfd if the
default linker is gold. I don't think a separate tc-ld-disable-lld
function is necessary at this time, and it could be easily added
later.

Re: [gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[Arfrever Frehtes Taifersar Arahesis]

Maybe one generic function (called e.g. tc-set-linker) whose arguments
are a preference-ordered list of supported linkers?
If currently used linker is not in the specified list, then this
function would force using first found linker from that list.

Forcing bfd only:
  tc-set-linker bfd

Forcing gold only:
  tc-set-linker gold

Forcing lld only:
  tc-set-linker lld

Forcing bfd or gold (with bfd preferred if both are available):
  tc-set-linker bfd gold

Forcing bfd or lld (with bfd preferred if both are available):
  tc-set-linker bfd lld

Forcing lld or bfd (with lld preferred if both are available):
  tc-set-linker lld bfd

Re: [gentoo-dev] [PATCH] toolchain-funcs.eclass: new function tc-ld-force-bfd()

[Theo Anderson]

On Thu, 21 Jan 2021 00:10:36 -0500
Mike Gilbert <floppym@gentoo.org> wrote:

> On Wed, Jan 20, 2021 at 9:41 PM Manoj Gupta <manojgupta@google.com>
> wrote:
> >
> >
> >
> > On Wed, Jan 20, 2021 at 1:01 PM Theo Anderson <telans@posteo.de>
> > wrote:  
> >>
> >> Hello, please see the below patch to support disabling ld.lld like
> >> ld.gold. This has not been split into a separate function
> >> such as tc-ld-disable-lld(), as I do not believe there is a use
> >> case where ld.gold is supported and ld.lld is not.
> >>
> >> Thanks.
> >>
> >> Pull-request: https://github.com/gentoo/gentoo/pull/19116
> >>  
> >
> > I am not a Gentoo maintainer but this forces bfd linker for the
> > ebuilds when gold is not even used e.g. lld is default linker. I am
> > curious how many places where gold is disabled do not work with
> > lld. In my experience, LLD is far more compatible with bfd than
> > gold e.g. it can link Linux kernels. So, imo we should not disable
> > lld as a side effect when the compatibility problem is with gold
> > only. i.e. It is ok to add a function to force bfd but disabling
> > gold needs to have a check if gold is the current linker.
> >
> > My preference us to add 2 functions:
> > tc-ld-force-bfd
> > tc-ld-disable-lld
> >
> > And tc-ld-disable-gold should check if gold is the current linker.
> > If not, only then force bfd.
> >
> > What do the maintainers think?  
> 
> Please see bug 729510 for an example where gold and lld do not work,
> but bfd does. That bug precipitated this change in the first place.
> 
> I don't think there are any cases where we want to disable lld without
> disabling gold. Maybe it would suffice to un-deprecate
> tc-ld-disable-gold and only have it call tc-ld-force-bfd if the
> default linker is gold. I don't think a separate tc-ld-disable-lld
> function is necessary at this time, and it could be easily added
> later.
> 

This is probably the solution to go for at the moment. I will update the
patch and post a v2. Regarding a new function such as tc-set-linker as
mentioned by Afrever, a complete overhaul like that would be better
suited as a separate patch.