diff mbox

[1/7] Mark lazy tlsdesc helper functions unused to avoid warnings

Message ID 59EF4D34.4020506@arm.com
State New, archived
Headers

Commit Message

Szabolcs Nagy Oct. 24, 2017, 2:24 p.m. UTC

Comments

Szabolcs Nagy Oct. 27, 2017, 2:20 p.m. UTC | #1 
On 24/10/17 15:24, Szabolcs Nagy wrote:
> From 74f8c71fee285657860926c8e45227041265d15d Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Mon, 23 Oct 2017 12:15:40 +0100
> Subject: [PATCH 1/7] Mark lazy tlsdesc helper functions unused to avoid
>  warnings
> 
> These static functions are not needed if a target does not do lazy
> tlsdesc initialization.
> 
> 2017-10-23  Szabolcs Nagy  <szabolcs.nagy@arm.com>
> 
> 	* elf/tlsdeschtab.h (_dl_tls_resolve_early_return_p): Mark unused.
> 	(_dl_tlsdesc_wake_up_held_fixups): Likewise.

i'd like to get consensus on this (trivial) change.

once it's committed, other targets (i386, x86_64) can
also remove lazy tlsdesc code.

after that these functions can be completely removed.

> ---
>  elf/tlsdeschtab.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/elf/tlsdeschtab.h b/elf/tlsdeschtab.h
> index ad3001dac5..879631897c 100644
> --- a/elf/tlsdeschtab.h
> +++ b/elf/tlsdeschtab.h
> @@ -137,6 +137,7 @@ _dl_make_tlsdesc_dynamic (struct link_map *map, size_t ti_offset)
>     avoid introducing such dependencies.  */
>  
>  static int
> +__attribute__ ((unused))
>  _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>  {
>    if (caller != atomic_load_relaxed (&td->entry))
> @@ -155,6 +156,7 @@ _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>  }
>  
>  static void
> +__attribute__ ((unused))
>  _dl_tlsdesc_wake_up_held_fixups (void)
>  {
>    __rtld_lock_unlock_recursive (GL(dl_load_lock));
> -- 2.11.0
>
Szabolcs Nagy Nov. 3, 2017, 12:05 p.m. UTC | #2 
On 27/10/17 15:20, Szabolcs Nagy wrote:
> On 24/10/17 15:24, Szabolcs Nagy wrote:
>> From 74f8c71fee285657860926c8e45227041265d15d Mon Sep 17 00:00:00 2001
>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>> Date: Mon, 23 Oct 2017 12:15:40 +0100
>> Subject: [PATCH 1/7] Mark lazy tlsdesc helper functions unused to avoid
>>  warnings
>>
>> These static functions are not needed if a target does not do lazy
>> tlsdesc initialization.
>>
>> 2017-10-23  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* elf/tlsdeschtab.h (_dl_tls_resolve_early_return_p): Mark unused.
>> 	(_dl_tlsdesc_wake_up_held_fixups): Likewise.
> 
> i'd like to get consensus on this (trivial) change.
> 

any comments on this?

> once it's committed, other targets (i386, x86_64) can
> also remove lazy tlsdesc code.
> 
> after that these functions can be completely removed.
> 
>> ---
>>  elf/tlsdeschtab.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/elf/tlsdeschtab.h b/elf/tlsdeschtab.h
>> index ad3001dac5..879631897c 100644
>> --- a/elf/tlsdeschtab.h
>> +++ b/elf/tlsdeschtab.h
>> @@ -137,6 +137,7 @@ _dl_make_tlsdesc_dynamic (struct link_map *map, size_t ti_offset)
>>     avoid introducing such dependencies.  */
>>  
>>  static int
>> +__attribute__ ((unused))
>>  _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>>  {
>>    if (caller != atomic_load_relaxed (&td->entry))
>> @@ -155,6 +156,7 @@ _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>>  }
>>  
>>  static void
>> +__attribute__ ((unused))
>>  _dl_tlsdesc_wake_up_held_fixups (void)
>>  {
>>    __rtld_lock_unlock_recursive (GL(dl_load_lock));
>> -- 2.11.0
>>
>
Florian Weimer Nov. 3, 2017, 12:14 p.m. UTC | #3 
On 10/24/2017 04:24 PM, Szabolcs Nagy wrote:
> diff --git a/elf/tlsdeschtab.h b/elf/tlsdeschtab.h
> index ad3001dac5..879631897c 100644
> --- a/elf/tlsdeschtab.h
> +++ b/elf/tlsdeschtab.h
> @@ -137,6 +137,7 @@ _dl_make_tlsdesc_dynamic (struct link_map *map, size_t ti_offset)
>      avoid introducing such dependencies.  */
>   
>   static int
> +__attribute__ ((unused))
>   _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>   {
>     if (caller != atomic_load_relaxed (&td->entry))
> @@ -155,6 +156,7 @@ _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>   }
>   
>   static void
> +__attribute__ ((unused))
>   _dl_tlsdesc_wake_up_held_fixups (void)
>   {
>     __rtld_lock_unlock_recursive (GL(dl_load_lock));

I think the preferred syntax is to put the attribute before the return 
type; after the type, it would apply to the type, and not the function, 
and GCC has hacks to support that, but that will only work for some types.

Otherwise, this looks okay to me.

Thanks,
Florian
Szabolcs Nagy Nov. 3, 2017, 12:41 p.m. UTC | #4 
On 03/11/17 12:14, Florian Weimer wrote:
> On 10/24/2017 04:24 PM, Szabolcs Nagy wrote:
>> diff --git a/elf/tlsdeschtab.h b/elf/tlsdeschtab.h
>> index ad3001dac5..879631897c 100644
>> --- a/elf/tlsdeschtab.h
>> +++ b/elf/tlsdeschtab.h
>> @@ -137,6 +137,7 @@ _dl_make_tlsdesc_dynamic (struct link_map *map, size_t ti_offset)
>>      avoid introducing such dependencies.  */
>>     static int
>> +__attribute__ ((unused))
>>   _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>>   {
>>     if (caller != atomic_load_relaxed (&td->entry))
>> @@ -155,6 +156,7 @@ _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
>>   }
>>     static void
>> +__attribute__ ((unused))
>>   _dl_tlsdesc_wake_up_held_fixups (void)
>>   {
>>     __rtld_lock_unlock_recursive (GL(dl_load_lock));
> 
> I think the preferred syntax is to put the attribute before the return type; after the type, it would apply to
> the type, and not the function, and GCC has hacks to support that, but that will only work for some types.
> 

hm.. current uses of unused attribute for the
same purpose come after the return type:

crypt/crypt_util.c
nptl/pthread_cond_common.c
stdio-common/_itowa.h
nis/nss-nisplus.h
sysdeps/unix/sysv/linux/dl-librecon.h
sysdeps/unix/sysv/linux/exit-thread.h
sysdeps/unix/sysv/linux/sigset-cvt-mask.h
sysdeps/*/dl-procinfo.h
sysdeps/*/dl-machine.h
sysdeps/unix/sysv/linux/*/dl-procinfo.h
iconv/gconv_charset.h
locale/elem-hash.h
locale/programs/locfile.h
nscd/nscd-client.h
posix/regex_internal.h
elf/get-dynamic-info.h

in fact the only cases where i see different
order is your code in:

resolv/resolv_context.h
malloc/dynarray-skeleton.c

(and even these two files use inconsistent
ordering)

> Otherwise, this looks okay to me.
> 
> Thanks,
> Florian
Florian Weimer Nov. 3, 2017, 2:28 p.m. UTC | #5 
On 11/03/2017 01:41 PM, Szabolcs Nagy wrote:
>> I think the preferred syntax is to put the attribute before the return type; after the type, it would apply to
>> the type, and not the function, and GCC has hacks to support that, but that will only work for some types.
>>
> hm.. current uses of unused attribute for the
> same purpose come after the return type:

I meant the preferred syntax on the GCC side.  But it was just a 
suggestion.  For the unused attribute, the distinction truly does not 
matter.

Thanks,
Florian
Szabolcs Nagy Nov. 3, 2017, 2:55 p.m. UTC | #6 
On 03/11/17 14:28, Florian Weimer wrote:
> On 11/03/2017 01:41 PM, Szabolcs Nagy wrote:
>>> I think the preferred syntax is to put the attribute before the return type; after the type, it would apply to
>>> the type, and not the function, and GCC has hacks to support that, but that will only work for some types.
>>>
>> hm.. current uses of unused attribute for the
>> same purpose come after the return type:
> 
> I meant the preferred syntax on the GCC side.  But it was just a suggestion.  For the unused attribute, the
> distinction truly does not matter.
> 

i see.

i committed the patch as is,
we can fix the order later together with other attributes.
(although the plan is to drop this code eventually)
diff mbox

Patch

From 74f8c71fee285657860926c8e45227041265d15d Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Mon, 23 Oct 2017 12:15:40 +0100
Subject: [PATCH 1/7] Mark lazy tlsdesc helper functions unused to avoid
 warnings

These static functions are not needed if a target does not do lazy
tlsdesc initialization.

2017-10-23  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	* elf/tlsdeschtab.h (_dl_tls_resolve_early_return_p): Mark unused.
	(_dl_tlsdesc_wake_up_held_fixups): Likewise.
---
 elf/tlsdeschtab.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elf/tlsdeschtab.h b/elf/tlsdeschtab.h
index ad3001dac5..879631897c 100644
--- a/elf/tlsdeschtab.h
+++ b/elf/tlsdeschtab.h
@@ -137,6 +137,7 @@  _dl_make_tlsdesc_dynamic (struct link_map *map, size_t ti_offset)
    avoid introducing such dependencies.  */
 
 static int
+__attribute__ ((unused))
 _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
 {
   if (caller != atomic_load_relaxed (&td->entry))
@@ -155,6 +156,7 @@  _dl_tlsdesc_resolve_early_return_p (struct tlsdesc volatile *td, void *caller)
 }
 
 static void
+__attribute__ ((unused))
 _dl_tlsdesc_wake_up_held_fixups (void)
 {
   __rtld_lock_unlock_recursive (GL(dl_load_lock));
-- 
2.11.0