libgcc, emutls: Allow building weak definitions of the emutls functions.

  Hi,

The non-Darwin part of this patch is trivial but raises a couple of questions

A/
We define builtins to support emulated TLS.
These are defined with void * pointers
The implementation (in libgcc) uses the correct type (struct __emutls_object *)
in both a forward declaration of the functions and in thier eventual implementation.

This leads to a (long-standing, nothing new) complaint at build-time about
the mismatch in the builtin/implementation decls.

AFAICT, there’s no way to fix that unless we introduce struct __emutls_object *
as a built-in type?

B/ 
It seems that a consequence of the mismatch in decls means that if I apply
attributes to the decl (in the implementation file), they are ignored and I have
to apply them to the definition in order for this to work.

This (B) is what the patch below does.

tested on powerpc,i686,x86_64-darwin, x86_64-linux
OK for master?
thanks,
Iain

If the current situation is that A or B indicates “there’s a bug”, please could that
be considered as distinct from the current patch (which doesn’t alter this in any
way) so that we can make progress on fixing Darwin libgcc issues.

===== commit log

In order to better support use of the emulated TLS between objects with
DSO dependencies and static-linked libgcc, allow a target to make weak
definitions.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libgcc/ChangeLog:

	* config.host: Add weak-defined emutls crt.
	* config/t-darwin: Build weak-defined emutls objects.
	* emutls.c (__emutls_get_address): Add optional attributes.
	(__emutls_register_common): Likewise.
	(EMUTLS_ATTR): New.
---
 libgcc/config.host     |  2 +-
 libgcc/config/t-darwin | 13 +++++++++++++
 libgcc/emutls.c        | 17 +++++++++++++++--
 3 files changed, 29 insertions(+), 3 deletions(-)

--
  

Hi,

as noted below the non-Darwin parts of this are trivial (and a no-OP).
I’d like to apply this to start work towards solving Darwin’s libgcc issues,

OTOH, the two raised questions remain…
thanks
Iain

> On 20 Sep 2021, at 09:25, Iain Sandoe <iain@sandoe.co.uk> wrote:
> 
> Hi,
> 
> The non-Darwin part of this patch is trivial but raises a couple of questions
> 
> A/
> We define builtins to support emulated TLS.
> These are defined with void * pointers
> The implementation (in libgcc) uses the correct type (struct __emutls_object *)
> in both a forward declaration of the functions and in thier eventual implementation.
> 
> This leads to a (long-standing, nothing new) complaint at build-time about
> the mismatch in the builtin/implementation decls.
> 
> AFAICT, there’s no way to fix that unless we introduce struct __emutls_object *
> as a built-in type?
> 
> B/ 
> It seems that a consequence of the mismatch in decls means that if I apply
> attributes to the decl (in the implementation file), they are ignored and I have
> to apply them to the definition in order for this to work.
> 
> This (B) is what the patch below does.
> 
> tested on powerpc,i686,x86_64-darwin, x86_64-linux
> OK for master?
> thanks,
> Iain
> 
> If the current situation is that A or B indicates “there’s a bug”, please could that
> be considered as distinct from the current patch (which doesn’t alter this in any
> way) so that we can make progress on fixing Darwin libgcc issues.
> 
> ===== commit log
> 
> In order to better support use of the emulated TLS between objects with
> DSO dependencies and static-linked libgcc, allow a target to make weak
> definitions.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> libgcc/ChangeLog:
> 
> 	* config.host: Add weak-defined emutls crt.
> 	* config/t-darwin: Build weak-defined emutls objects.
> 	* emutls.c (__emutls_get_address): Add optional attributes.
> 	(__emutls_register_common): Likewise.
> 	(EMUTLS_ATTR): New.
> ---
> libgcc/config.host     |  2 +-
> libgcc/config/t-darwin | 13 +++++++++++++
> libgcc/emutls.c        | 17 +++++++++++++++--
> 3 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/libgcc/config.host b/libgcc/config.host
> index 6c34b13d611..a447ac7ae30 100644
> --- a/libgcc/config.host
> +++ b/libgcc/config.host
> @@ -215,7 +215,7 @@ case ${host} in
> *-*-darwin*)
>   asm_hidden_op=.private_extern
>   tmake_file="$tmake_file t-darwin ${cpu_type}/t-darwin t-libgcc-pic t-slibgcc-darwin"
> -  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o"
> +  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
>   ;;
> *-*-dragonfly*)
>   tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
> diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
> index 14ae6b35a4e..d6f688d66d5 100644
> --- a/libgcc/config/t-darwin
> +++ b/libgcc/config/t-darwin
> @@ -15,6 +15,19 @@ crttme.o: $(srcdir)/config/darwin-crt-tm.c
> LIB2ADDEH = $(srcdir)/unwind-dw2.c $(srcdir)/config/unwind-dw2-fde-darwin.c \
>   $(srcdir)/unwind-sjlj.c $(srcdir)/unwind-c.c
> 
> +# Make emutls weak so that we can deal with -static-libgcc, override the
> +# hidden visibility when this is present in libgcc_eh.
> +emutls.o: HOST_LIBGCC2_CFLAGS += \
> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
> +emutls_s.o: HOST_LIBGCC2_CFLAGS += \
> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
> +
> +# Make the emutls crt as a convenience lib so that it can be linked
> +# optionally, use the shared version so that we can link with DSO.
> +libemutls_w.a: emutls_s.o
> +	$(AR_CREATE_FOR_TARGET) $@ $<
> +	$(RANLIB_FOR_TARGET) $@
> +
> # Patch to __Unwind_Find_Enclosing_Function for Darwin10.
> d10-uwfef.o: $(srcdir)/config/darwin10-unwind-find-enc-func.c
> 	$(crt_compile) -mmacosx-version-min=10.6 -c $<
> diff --git a/libgcc/emutls.c b/libgcc/emutls.c
> index ed2658170f5..d553a74728f 100644
> --- a/libgcc/emutls.c
> +++ b/libgcc/emutls.c
> @@ -50,7 +50,16 @@ struct __emutls_array
>   void **data[];
> };
> 
> +/* EMUTLS_ATTR is provided to allow targets to build the emulated tls
> +   routines as weak definitions, for example.
> +   If there is no definition, fall back to the default.  */
> +#ifndef EMUTLS_ATTR
> +#  define EMUTLS_ATTR
> +#endif
> +
> +EMUTLS_ATTR
> void *__emutls_get_address (struct __emutls_object *);
> +EMUTLS_ATTR
> void __emutls_register_common (struct __emutls_object *, word, word, void *);
> 
> #ifdef __GTHREADS
> @@ -123,7 +132,11 @@ emutls_alloc (struct __emutls_object *obj)
>   return ret;
> }
> 
> -void *
> +/* Despite applying the attribute to the declaration, in this case the mis-
> +   match between the builtin's declaration [void * (*)(void *)] and the
> +   implementation here, causes the decl. attributes to be discarded.  */
> +
> +EMUTLS_ATTR void *
> __emutls_get_address (struct __emutls_object *obj)
> {
>   if (! __gthread_active_p ())
> @@ -187,7 +200,7 @@ __emutls_get_address (struct __emutls_object *obj)
> #endif
> }
> 
> -void
> +EMUTLS_ATTR void
> __emutls_register_common (struct __emutls_object *obj,
> 			  word size, word align, void *templ)
> {
> -- 
>
  

Hi,

So let’s ignore the questions for now - OK for the non-Darwin parts of the
patch ?

> On 24 Sep 2021, at 17:57, Iain Sandoe <iain@sandoe.co.uk> wrote:
> 

> as noted below the non-Darwin parts of this are trivial (and a no-OP).
> I’d like to apply this to start work towards solving Darwin’s libgcc issues,

>> On 20 Sep 2021, at 09:25, Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 

>> The non-Darwin part of this patch is trivial but raises a couple of questions
>> 
>> A/
>> We define builtins to support emulated TLS.
>> These are defined with void * pointers
>> The implementation (in libgcc) uses the correct type (struct __emutls_object *)
>> in both a forward declaration of the functions and in thier eventual implementation.
>> 
>> This leads to a (long-standing, nothing new) complaint at build-time about
>> the mismatch in the builtin/implementation decls.
>> 
>> AFAICT, there’s no way to fix that unless we introduce struct __emutls_object *
>> as a built-in type?
>> 
>> B/ 
>> It seems that a consequence of the mismatch in decls means that if I apply
>> attributes to the decl (in the implementation file), they are ignored and I have
>> to apply them to the definition in order for this to work.
>> 
>> This (B) is what the patch below does.
>> 
>> tested on powerpc,i686,x86_64-darwin, x86_64-linux
>> OK for master?
>> thanks,
>> Iain
>> 
>> If the current situation is that A or B indicates “there’s a bug”, please could that
>> be considered as distinct from the current patch (which doesn’t alter this in any
>> way) so that we can make progress on fixing Darwin libgcc issues.
>> 
>> ===== commit log
>> 
>> In order to better support use of the emulated TLS between objects with
>> DSO dependencies and static-linked libgcc, allow a target to make weak
>> definitions.
>> 
>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>> 
>> libgcc/ChangeLog:
>> 
>> 	* config.host: Add weak-defined emutls crt.
>> 	* config/t-darwin: Build weak-defined emutls objects.
>> 	* emutls.c (__emutls_get_address): Add optional attributes.
>> 	(__emutls_register_common): Likewise.
>> 	(EMUTLS_ATTR): New.
>> ---
>> libgcc/config.host     |  2 +-
>> libgcc/config/t-darwin | 13 +++++++++++++
>> libgcc/emutls.c        | 17 +++++++++++++++--
>> 3 files changed, 29 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libgcc/config.host b/libgcc/config.host
>> index 6c34b13d611..a447ac7ae30 100644
>> --- a/libgcc/config.host
>> +++ b/libgcc/config.host
>> @@ -215,7 +215,7 @@ case ${host} in
>> *-*-darwin*)
>>  asm_hidden_op=.private_extern
>>  tmake_file="$tmake_file t-darwin ${cpu_type}/t-darwin t-libgcc-pic t-slibgcc-darwin"
>> -  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o"
>> +  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
>>  ;;
>> *-*-dragonfly*)
>>  tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
>> diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
>> index 14ae6b35a4e..d6f688d66d5 100644
>> --- a/libgcc/config/t-darwin
>> +++ b/libgcc/config/t-darwin
>> @@ -15,6 +15,19 @@ crttme.o: $(srcdir)/config/darwin-crt-tm.c
>> LIB2ADDEH = $(srcdir)/unwind-dw2.c $(srcdir)/config/unwind-dw2-fde-darwin.c \
>>  $(srcdir)/unwind-sjlj.c $(srcdir)/unwind-c.c
>> 
>> +# Make emutls weak so that we can deal with -static-libgcc, override the
>> +# hidden visibility when this is present in libgcc_eh.
>> +emutls.o: HOST_LIBGCC2_CFLAGS += \
>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>> +emutls_s.o: HOST_LIBGCC2_CFLAGS += \
>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>> +
>> +# Make the emutls crt as a convenience lib so that it can be linked
>> +# optionally, use the shared version so that we can link with DSO.
>> +libemutls_w.a: emutls_s.o
>> +	$(AR_CREATE_FOR_TARGET) $@ $<
>> +	$(RANLIB_FOR_TARGET) $@
>> +
>> # Patch to __Unwind_Find_Enclosing_Function for Darwin10.
>> d10-uwfef.o: $(srcdir)/config/darwin10-unwind-find-enc-func.c
>> 	$(crt_compile) -mmacosx-version-min=10.6 -c $<
>> diff --git a/libgcc/emutls.c b/libgcc/emutls.c
>> index ed2658170f5..d553a74728f 100644
>> --- a/libgcc/emutls.c
>> +++ b/libgcc/emutls.c
>> @@ -50,7 +50,16 @@ struct __emutls_array
>>  void **data[];
>> };
>> 
>> +/* EMUTLS_ATTR is provided to allow targets to build the emulated tls
>> +   routines as weak definitions, for example.
>> +   If there is no definition, fall back to the default.  */
>> +#ifndef EMUTLS_ATTR
>> +#  define EMUTLS_ATTR
>> +#endif
>> +
>> +EMUTLS_ATTR
>> void *__emutls_get_address (struct __emutls_object *);
>> +EMUTLS_ATTR
>> void __emutls_register_common (struct __emutls_object *, word, word, void *);
>> 
>> #ifdef __GTHREADS
>> @@ -123,7 +132,11 @@ emutls_alloc (struct __emutls_object *obj)
>>  return ret;
>> }
>> 
>> -void *
>> +/* Despite applying the attribute to the declaration, in this case the mis-
>> +   match between the builtin's declaration [void * (*)(void *)] and the
>> +   implementation here, causes the decl. attributes to be discarded.  */
>> +
>> +EMUTLS_ATTR void *
>> __emutls_get_address (struct __emutls_object *obj)
>> {
>>  if (! __gthread_active_p ())
>> @@ -187,7 +200,7 @@ __emutls_get_address (struct __emutls_object *obj)
>> #endif
>> }
>> 
>> -void
>> +EMUTLS_ATTR void
>> __emutls_register_common (struct __emutls_object *obj,
>> 			  word size, word align, void *templ)
>> {
>> -- 
>> 
>
  

Iain Sandoe <iain@sandoe.co.uk> writes:
> Hi,
>
> So let’s ignore the questions for now - OK for the non-Darwin parts of the
> patch ?

Looks OK to me.

Thanks,
Richard

>
>> On 24 Sep 2021, at 17:57, Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>
>> as noted below the non-Darwin parts of this are trivial (and a no-OP).
>> I’d like to apply this to start work towards solving Darwin’s libgcc issues,
>
>>> On 20 Sep 2021, at 09:25, Iain Sandoe <iain@sandoe.co.uk> wrote:
>>> 
>
>>> The non-Darwin part of this patch is trivial but raises a couple of questions
>>> 
>>> A/
>>> We define builtins to support emulated TLS.
>>> These are defined with void * pointers
>>> The implementation (in libgcc) uses the correct type (struct __emutls_object *)
>>> in both a forward declaration of the functions and in thier eventual implementation.
>>> 
>>> This leads to a (long-standing, nothing new) complaint at build-time about
>>> the mismatch in the builtin/implementation decls.
>>> 
>>> AFAICT, there’s no way to fix that unless we introduce struct __emutls_object *
>>> as a built-in type?
>>> 
>>> B/ 
>>> It seems that a consequence of the mismatch in decls means that if I apply
>>> attributes to the decl (in the implementation file), they are ignored and I have
>>> to apply them to the definition in order for this to work.
>>> 
>>> This (B) is what the patch below does.
>>> 
>>> tested on powerpc,i686,x86_64-darwin, x86_64-linux
>>> OK for master?
>>> thanks,
>>> Iain
>>> 
>>> If the current situation is that A or B indicates “there’s a bug”, please could that
>>> be considered as distinct from the current patch (which doesn’t alter this in any
>>> way) so that we can make progress on fixing Darwin libgcc issues.
>>> 
>>> ===== commit log
>>> 
>>> In order to better support use of the emulated TLS between objects with
>>> DSO dependencies and static-linked libgcc, allow a target to make weak
>>> definitions.
>>> 
>>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>>> 
>>> libgcc/ChangeLog:
>>> 
>>> 	* config.host: Add weak-defined emutls crt.
>>> 	* config/t-darwin: Build weak-defined emutls objects.
>>> 	* emutls.c (__emutls_get_address): Add optional attributes.
>>> 	(__emutls_register_common): Likewise.
>>> 	(EMUTLS_ATTR): New.
>>> ---
>>> libgcc/config.host     |  2 +-
>>> libgcc/config/t-darwin | 13 +++++++++++++
>>> libgcc/emutls.c        | 17 +++++++++++++++--
>>> 3 files changed, 29 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/libgcc/config.host b/libgcc/config.host
>>> index 6c34b13d611..a447ac7ae30 100644
>>> --- a/libgcc/config.host
>>> +++ b/libgcc/config.host
>>> @@ -215,7 +215,7 @@ case ${host} in
>>> *-*-darwin*)
>>>  asm_hidden_op=.private_extern
>>>  tmake_file="$tmake_file t-darwin ${cpu_type}/t-darwin t-libgcc-pic t-slibgcc-darwin"
>>> -  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o"
>>> +  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
>>>  ;;
>>> *-*-dragonfly*)
>>>  tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
>>> diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
>>> index 14ae6b35a4e..d6f688d66d5 100644
>>> --- a/libgcc/config/t-darwin
>>> +++ b/libgcc/config/t-darwin
>>> @@ -15,6 +15,19 @@ crttme.o: $(srcdir)/config/darwin-crt-tm.c
>>> LIB2ADDEH = $(srcdir)/unwind-dw2.c $(srcdir)/config/unwind-dw2-fde-darwin.c \
>>>  $(srcdir)/unwind-sjlj.c $(srcdir)/unwind-c.c
>>> 
>>> +# Make emutls weak so that we can deal with -static-libgcc, override the
>>> +# hidden visibility when this is present in libgcc_eh.
>>> +emutls.o: HOST_LIBGCC2_CFLAGS += \
>>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>>> +emutls_s.o: HOST_LIBGCC2_CFLAGS += \
>>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>>> +
>>> +# Make the emutls crt as a convenience lib so that it can be linked
>>> +# optionally, use the shared version so that we can link with DSO.
>>> +libemutls_w.a: emutls_s.o
>>> +	$(AR_CREATE_FOR_TARGET) $@ $<
>>> +	$(RANLIB_FOR_TARGET) $@
>>> +
>>> # Patch to __Unwind_Find_Enclosing_Function for Darwin10.
>>> d10-uwfef.o: $(srcdir)/config/darwin10-unwind-find-enc-func.c
>>> 	$(crt_compile) -mmacosx-version-min=10.6 -c $<
>>> diff --git a/libgcc/emutls.c b/libgcc/emutls.c
>>> index ed2658170f5..d553a74728f 100644
>>> --- a/libgcc/emutls.c
>>> +++ b/libgcc/emutls.c
>>> @@ -50,7 +50,16 @@ struct __emutls_array
>>>  void **data[];
>>> };
>>> 
>>> +/* EMUTLS_ATTR is provided to allow targets to build the emulated tls
>>> +   routines as weak definitions, for example.
>>> +   If there is no definition, fall back to the default.  */
>>> +#ifndef EMUTLS_ATTR
>>> +#  define EMUTLS_ATTR
>>> +#endif
>>> +
>>> +EMUTLS_ATTR
>>> void *__emutls_get_address (struct __emutls_object *);
>>> +EMUTLS_ATTR
>>> void __emutls_register_common (struct __emutls_object *, word, word, void *);
>>> 
>>> #ifdef __GTHREADS
>>> @@ -123,7 +132,11 @@ emutls_alloc (struct __emutls_object *obj)
>>>  return ret;
>>> }
>>> 
>>> -void *
>>> +/* Despite applying the attribute to the declaration, in this case the mis-
>>> +   match between the builtin's declaration [void * (*)(void *)] and the
>>> +   implementation here, causes the decl. attributes to be discarded.  */
>>> +
>>> +EMUTLS_ATTR void *
>>> __emutls_get_address (struct __emutls_object *obj)
>>> {
>>>  if (! __gthread_active_p ())
>>> @@ -187,7 +200,7 @@ __emutls_get_address (struct __emutls_object *obj)
>>> #endif
>>> }
>>> 
>>> -void
>>> +EMUTLS_ATTR void
>>> __emutls_register_common (struct __emutls_object *obj,
>>> 			  word size, word align, void *templ)
>>> {
>>> -- 
>>> 
>>