Re: [PATCH] configure: Replace -Wl,-r,-d with -Wl,-r

[Daniel Kiper]

On Fri, Feb 11, 2022 at 09:35:04AM -0800, Fāng-ruì Sòng via Grub-devel wrote:
> On Fri, Feb 11, 2022 at 6:04 AM Daniel Kiper <dkiper@net-space.pl> wrote:
> >
> > On Tue, Feb 08, 2022 at 03:25:27PM -0800, Fangrui Song via Grub-devel wrote:
> > > In GNU ld and ld.lld, -d is used with -r to allocate space to COMMON 
> > > symbols.
> > > This behavior is presumably to work around legacy projects which inspect
> > > relocatable output by themselves and do not handle COMMON symbols. grub 
> > > does
> > > not do this.
> > >
> > > See https://github.com/llvm/llvm-project/issues/53660
> > > -d is quite useless and ld.lld may remove -d or make it a no-op for the
> > > 15.0.0 release.
> > > ---
> > >  acinclude.m4                |  2 +-
> > >  conf/Makefile.common        |  2 +-
> > >  grub-core/Makefile.core.def | 20 ++++++++++----------
> > >  grub-core/genmod.sh.in      |  4 ++--
> > >  4 files changed, 14 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/acinclude.m4 b/acinclude.m4
> > > index 6e14bb553..fa7840f09 100644
> > > --- a/acinclude.m4
> > > +++ b/acinclude.m4
> > > @@ -430,7 +430,7 @@ link_nopie_needed=no]
> > >  AC_MSG_CHECKING([whether linker needs disabling of PIE to work])
> > >  AC_LANG_CONFTEST([AC_LANG_SOURCE([[]])])
> > >
> > > -[if eval "$ac_compile -Wl,-r,-d -nostdlib -Werror -o conftest.o" 2> 
> > > /dev/null; then]
> > > +[if eval "$ac_compile -Wl,-r -nostdlib -Werror -o conftest.o" 2> 
> > > /dev/null; then]
> >
> > It looks that these changes do not break any builds. So, I am OK with
> > them. Though should not we add -fno-common to the CFLAGS too, e.g.:
> >
> > diff --git a/configure.ac b/configure.ac
> > index 5c01af0fa..d1eaafee1 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -85,9 +85,9 @@ fi
> >
> >  # Enable support for "restrict" keyword and other
> >  # features from gnu99 C language standard.
> > -BUILD_CFLAGS="-std=gnu99 $BUILD_CFLAGS"
> > -HOST_CFLAGS="-std=gnu99 $HOST_CFLAGS"
> > -TARGET_CFLAGS="-std=gnu99 $TARGET_CFLAGS"
> > +BUILD_CFLAGS="-std=gnu99 -fno-common $BUILD_CFLAGS"
> > +HOST_CFLAGS="-std=gnu99 -fno-common $HOST_CFLAGS"
> > +TARGET_CFLAGS="-std=gnu99 -fno-common $TARGET_CFLAGS"
> >
> >  # Default HOST_CPPFLAGS
> >  HOST_CPPFLAGS="$HOST_CPPFLAGS -Wall -W"
> >
> > Daniel
>
> Adding -fno-common to ensure no COMMON symbol will also be a good idea.
> Are you making the change? :)

If you could update your patch with the changes above that would be
perfect... :-) Or make it a separate patch. I do not have strong
preference here.

Daniel