[PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[Fangrui Song]

In GNU ld, -d assigns space to common symbols for -r (i.e. change common
symbols to STB_GLOBAL definitions).  This option was added in commit
da2d1bc5adf49352232ad0514e79fbd5dcae08e8 (1998) likely because ld at
that time had a bug that common symbols did not override shared object
definitions.  -d has been long unneeded and more so since -fno-common
was added to +cflags.
---
 Makerules | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/Makerules b/Makerules
index dfe89e9e39..d1e139d03c 100644
--- a/Makerules
+++ b/Makerules
@@ -633,14 +633,10 @@ LDFLAGS-c.so = -nostdlib -nostartfiles
 LDLIBS-c.so += $(libc.so-gnulib)
 # Give libc.so an entry point and make it directly runnable itself.
 LDFLAGS-c.so += -e __libc_main
-# Pre-link the objects of libc_pic.a so that we can locally resolve
-# COMMON symbols before we link against ld.so.  This is because ld.so
-# contains some of libc_pic.a already, which will prevent the COMMONs
-# from being allocated in libc.so, which introduces evil dependencies
-# between libc.so and ld.so, which can make it impossible to upgrade.
+# Pre-link the objects of libc_pic.a for .gnu.glibc-stub.* processing.
 $(common-objpfx)libc_pic.os: $(common-objpfx)libc_pic.a
 	$(LINK.o) -nostdlib -nostartfiles -r -o $@ \
-	$(LDFLAGS-c_pic.os) -Wl,-d $(whole-archive) $^ -o $@
+	$(LDFLAGS-c_pic.os) $(whole-archive) $^ -o $@
 
 ifeq (,$(strip $(shlib-lds-flags)))
 # Generate a list of -R options to excise .gnu.glibc-stub.* sections.
-- 
2.37.0.rc0.161.g10f37bed90-goog

Re: [PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[Florian Weimer]

* Fangrui Song via Libc-alpha:

> In GNU ld, -d assigns space to common symbols for -r (i.e. change common
> symbols to STB_GLOBAL definitions).  This option was added in commit
> da2d1bc5adf49352232ad0514e79fbd5dcae08e8 (1998) likely because ld at
> that time had a bug that common symbols did not override shared object
> definitions.  -d has been long unneeded and more so since -fno-common
> was added to +cflags.
> ---
>  Makerules | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/Makerules b/Makerules
> index dfe89e9e39..d1e139d03c 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -633,14 +633,10 @@ LDFLAGS-c.so = -nostdlib -nostartfiles
>  LDLIBS-c.so += $(libc.so-gnulib)
>  # Give libc.so an entry point and make it directly runnable itself.
>  LDFLAGS-c.so += -e __libc_main
> -# Pre-link the objects of libc_pic.a so that we can locally resolve
> -# COMMON symbols before we link against ld.so.  This is because ld.so
> -# contains some of libc_pic.a already, which will prevent the COMMONs
> -# from being allocated in libc.so, which introduces evil dependencies
> -# between libc.so and ld.so, which can make it impossible to upgrade.
> +# Pre-link the objects of libc_pic.a for .gnu.glibc-stub.* processing.
>  $(common-objpfx)libc_pic.os: $(common-objpfx)libc_pic.a
>  	$(LINK.o) -nostdlib -nostartfiles -r -o $@ \
> -	$(LDFLAGS-c_pic.os) -Wl,-d $(whole-archive) $^ -o $@
> +	$(LDFLAGS-c_pic.os) $(whole-archive) $^ -o $@
>  
>  ifeq (,$(strip $(shlib-lds-flags)))
>  # Generate a list of -R options to excise .gnu.glibc-stub.* sections.

This looks okay to me, thanks.

Do you know what the .gnu.glibc-stub.* processing processing refers
to?

Re: [PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[Fangrui Song]

On 2022-06-26, Florian Weimer wrote:
>* Fangrui Song via Libc-alpha:
>
>> In GNU ld, -d assigns space to common symbols for -r (i.e. change common
>> symbols to STB_GLOBAL definitions).  This option was added in commit
>> da2d1bc5adf49352232ad0514e79fbd5dcae08e8 (1998) likely because ld at
>> that time had a bug that common symbols did not override shared object
>> definitions.  -d has been long unneeded and more so since -fno-common
>> was added to +cflags.
>> ---
>>  Makerules | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/Makerules b/Makerules
>> index dfe89e9e39..d1e139d03c 100644
>> --- a/Makerules
>> +++ b/Makerules
>> @@ -633,14 +633,10 @@ LDFLAGS-c.so = -nostdlib -nostartfiles
>>  LDLIBS-c.so += $(libc.so-gnulib)
>>  # Give libc.so an entry point and make it directly runnable itself.
>>  LDFLAGS-c.so += -e __libc_main
>> -# Pre-link the objects of libc_pic.a so that we can locally resolve
>> -# COMMON symbols before we link against ld.so.  This is because ld.so
>> -# contains some of libc_pic.a already, which will prevent the COMMONs
>> -# from being allocated in libc.so, which introduces evil dependencies
>> -# between libc.so and ld.so, which can make it impossible to upgrade.
>> +# Pre-link the objects of libc_pic.a for .gnu.glibc-stub.* processing.
>>  $(common-objpfx)libc_pic.os: $(common-objpfx)libc_pic.a
>>  	$(LINK.o) -nostdlib -nostartfiles -r -o $@ \
>> -	$(LDFLAGS-c_pic.os) -Wl,-d $(whole-archive) $^ -o $@
>> +	$(LDFLAGS-c_pic.os) $(whole-archive) $^ -o $@
>>
>>  ifeq (,$(strip $(shlib-lds-flags)))
>>  # Generate a list of -R options to excise .gnu.glibc-stub.* sections.
>
>This looks okay to me, thanks.
>
>Do you know what the .gnu.glibc-stub.* processing processing refers
>to?

[a] Just few lines below, there is

     # Generate a list of -R options to excise .gnu.glibc-stub.* sections.

The idea is to create libc_pic.os from libc_pic.a, strip .gnu.glibc-stub.*, then link libc_pic.os.clean into libc.so.

Technically the relocatable link can be omitted.  We can change `build-shlib`
to link -Wl,--whole-archive libc_pic.a -Wl,--no-whole-archive instead.  objcopy -R works on an archive for
a long time.  I am unsure whether the 1990+ objcopy supports archives.
But at a first glance I do not find a good place to insert -Wl,--whole-archive into build-shlib...


[b] Around $(objpfx)stubs: $(objs-for-stubs), we can see that
.gnu.glibc-stub.* is to generate */stubs files and finally testroot.root/$prefix/include/gnu/stubs-64.h

% cat testroot.root/tmp/glibc/lld/include/gnu/stubs-64.h
/* This file is automatically generated.
    It defines a symbol `__stub_FUNCTION' for each function
    in the C library which is a stub, meaning it will fail
    every time called, usually setting errno to ENOSYS.  */

#ifdef _LIBC
  #error Applications may not define the macro _LIBC
#endif

#define __stub___compat_bdflush
#define __stub_chflags
#define __stub_fchflags
#define __stub_gtty
#define __stub_revoke
#define __stub_setlogin
#define __stub_sigreturn
#define __stub_stty

Re: [PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[Florian Weimer]

* Fangrui Song:

>>Do you know what the .gnu.glibc-stub.* processing processing refers
>>to?
>
> [a] Just few lines below, there is
>
>      # Generate a list of -R options to excise .gnu.glibc-stub.* sections.
>
> The idea is to create libc_pic.os from libc_pic.a, strip .gnu.glibc-stub.*, then link libc_pic.os.clean into libc.so.
>
> Technically the relocatable link can be omitted.  We can change `build-shlib`
> to link -Wl,--whole-archive libc_pic.a -Wl,--no-whole-archive instead.  objcopy -R works on an archive for
> a long time.  I am unsure whether the 1990+ objcopy supports archives.
> But at a first glance I do not find a good place to insert -Wl,--whole-archive into build-shlib...

For linking libc.so, we can list the objects directly, then we don't
need -Wl,--whole-archive.

For linking ld.so, we do not use -Wl,--whole-archive because we
actually want to link a subset.  I was under the impression that we
rebuild objects for inclusion into ld.so, so I'm rather puzzled that
we need this.

> [b] Around $(objpfx)stubs: $(objs-for-stubs), we can see that
> .gnu.glibc-stub.* is to generate */stubs files and finally testroot.root/$prefix/include/gnu/stubs-64.h
>
> % cat testroot.root/tmp/glibc/lld/include/gnu/stubs-64.h
> /* This file is automatically generated.
>     It defines a symbol `__stub_FUNCTION' for each function
>     in the C library which is a stub, meaning it will fail
>     every time called, usually setting errno to ENOSYS.  */
>
> #ifdef _LIBC
>   #error Applications may not define the macro _LIBC
> #endif
>
> #define __stub___compat_bdflush
> #define __stub_chflags
> #define __stub_fchflags
> #define __stub_gtty
> #define __stub_revoke
> #define __stub_setlogin
> #define __stub_sigreturn
> #define __stub_stty

Those warning sections must propagate into libc.so, so I think we
could parse that and the generate header from it.

Re: [PATCH] Makerules: Remove no-op -Wl,-d when linking libc_pic.os

[Fangrui Song]

On 2022-06-27, Florian Weimer wrote:
>* Fangrui Song:
>
>>>Do you know what the .gnu.glibc-stub.* processing processing refers
>>>to?
>>
>> [a] Just few lines below, there is
>>
>>      # Generate a list of -R options to excise .gnu.glibc-stub.* sections.
>>
>> The idea is to create libc_pic.os from libc_pic.a, strip .gnu.glibc-stub.*, then link libc_pic.os.clean into libc.so.
>>
>> Technically the relocatable link can be omitted.  We can change `build-shlib`
>> to link -Wl,--whole-archive libc_pic.a -Wl,--no-whole-archive instead.  objcopy -R works on an archive for
>> a long time.  I am unsure whether the 1990+ objcopy supports archives.
>> But at a first glance I do not find a good place to insert -Wl,--whole-archive into build-shlib...
>
>For linking libc.so, we can list the objects directly, then we don't
>need -Wl,--whole-archive.
>
>For linking ld.so, we do not use -Wl,--whole-archive because we
>actually want to link a subset.  I was under the impression that we
>rebuild objects for inclusion into ld.so, so I'm rather puzzled that
>we need this.

This plan looks good to me.
Removing the relocatable link for libc_pic.os will make incremental build slightly faster.

>> [b] Around $(objpfx)stubs: $(objs-for-stubs), we can see that
>> .gnu.glibc-stub.* is to generate */stubs files and finally testroot.root/$prefix/include/gnu/stubs-64.h
>>
>> % cat testroot.root/tmp/glibc/lld/include/gnu/stubs-64.h
>> /* This file is automatically generated.
>>     It defines a symbol `__stub_FUNCTION' for each function
>>     in the C library which is a stub, meaning it will fail
>>     every time called, usually setting errno to ENOSYS.  */
>>
>> #ifdef _LIBC
>>   #error Applications may not define the macro _LIBC
>> #endif
>>
>> #define __stub___compat_bdflush
>> #define __stub_chflags
>> #define __stub_fchflags
>> #define __stub_gtty
>> #define __stub_revoke
>> #define __stub_setlogin
>> #define __stub_sigreturn
>> #define __stub_stty
>
>Those warning sections must propagate into libc.so, so I think we
>could parse that and the generate header from it.

The final libc.so does not want .gnu.glibc-stub.* sections.  The current
build steps look like:

1. ar cruv libc_pic.a `cat csu/stamp.os iconv/stamp.os ...`
2. build libc_pic.os libc_pic.a
3. build libc_pic.os.clean from libc_pic.os
4. build libc.so from libc_pic.os.clean csu/abi-note.o elf/interp.os elf/ld.so ...

I think ideally 2 and 3 can be omitted.  It seems like you want to omit
libc_pic.a as well?  That looks good to me. stubs files can be either
generated from .os files (in step 1) or libc.so (then a separate strip
command on libc.so is needed).