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

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

[Fangrui Song]

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]
   AC_MSG_RESULT([no])
   [# Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
   rm -f conftest.o
diff --git a/conf/Makefile.common b/conf/Makefile.common
index f0bb6e160..1d7b39599 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -41,7 +41,7 @@ CCASFLAGS_KERNEL = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
 STRIPFLAGS_KERNEL = -R .rel.dyn -R .reginfo -R .note -R .comment -R .drectve -R .note.gnu.gold-version -R .MIPS.abiflags -R .ARM.exidx
 
 CFLAGS_MODULE = $(CFLAGS_PLATFORM) -ffreestanding
-LDFLAGS_MODULE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC) -Wl,-r,-d
+LDFLAGS_MODULE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC) -Wl,-r
 CPPFLAGS_MODULE = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM)
 CCASFLAGS_MODULE = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
 
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 8022e1c0a..ac00cc8a2 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -49,26 +49,26 @@ kernel = {
 
   nostrip = emu;
 
-  emu_ldflags              = '-Wl,-r,-d';
-  i386_efi_ldflags         = '-Wl,-r,-d';
+  emu_ldflags              = '-Wl,-r';
+  i386_efi_ldflags         = '-Wl,-r';
   i386_efi_stripflags      = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
-  x86_64_efi_ldflags       = '-Wl,-r,-d';
+  x86_64_efi_ldflags       = '-Wl,-r';
   x86_64_efi_stripflags    = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
   ia64_efi_cflags = '-fno-builtin -fpic -minline-int-divide-max-throughput';
-  ia64_efi_ldflags = '-Wl,-r,-d';
+  ia64_efi_ldflags = '-Wl,-r';
   ia64_efi_stripflags = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
-  arm_efi_ldflags          = '-Wl,-r,-d';
+  arm_efi_ldflags          = '-Wl,-r';
   arm_efi_stripflags       = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
-  arm64_efi_ldflags          = '-Wl,-r,-d';
+  arm64_efi_ldflags          = '-Wl,-r';
   arm64_efi_stripflags       = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version -R .eh_frame';
 
-  riscv32_efi_ldflags      = '-Wl,-r,-d';
+  riscv32_efi_ldflags      = '-Wl,-r';
   riscv32_efi_stripflags   = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version -R .eh_frame';
 
-  riscv64_efi_ldflags      = '-Wl,-r,-d';
+  riscv64_efi_ldflags      = '-Wl,-r';
   riscv64_efi_stripflags   = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version -R .eh_frame';
 
   i386_pc_ldflags          = '$(TARGET_IMG_LDFLAGS)';
@@ -98,9 +98,9 @@ kernel = {
   i386_qemu_cppflags     = '-DGRUB_BOOT_MACHINE_LINK_ADDR=$(GRUB_BOOT_MACHINE_LINK_ADDR)';
   emu_cflags = '$(CFLAGS_GNULIB)';
   emu_cppflags = '$(CPPFLAGS_GNULIB)';
-  arm_uboot_ldflags       = '-Wl,-r,-d';
+  arm_uboot_ldflags       = '-Wl,-r';
   arm_uboot_stripflags    = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
-  arm_coreboot_ldflags       = '-Wl,-r,-d';
+  arm_coreboot_ldflags       = '-Wl,-r';
   arm_coreboot_stripflags    = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
   i386_pc_startup = kern/i386/pc/startup.S;
diff --git a/grub-core/genmod.sh.in b/grub-core/genmod.sh.in
index 1250589b3..e57c4d920 100644
--- a/grub-core/genmod.sh.in
+++ b/grub-core/genmod.sh.in
@@ -83,9 +83,9 @@ else
     for dep in $deps; do echo "char moddep_$dep[] __attribute__ ((section(\"_moddeps, _moddeps\"))) = \"$dep\";" >>$t2; done
 
     if test -n "$deps"; then
-	@TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o $tmpfile2 $t1 $t2 $tmpfile -Wl,-r,-d
+	@TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o $tmpfile2 $t1 $t2 $tmpfile -Wl,-r
     else
-	@TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o $tmpfile2 $t1 $tmpfile -Wl,-r,-d
+	@TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o $tmpfile2 $t1 $tmpfile -Wl,-r
     fi
     rm -f $t1 $t2 $tmpfile
     mv $tmpfile2 $tmpfile
-- 
2.35.0.263.gb82422642f-goog

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

[Daniel Kiper]

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

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

[Fāng-ruì Sòng]

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? :)

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