[COMMITTED] alpha: Fix static gettimeofday symbol
diff mbox series

Message ID 20200213113824.32309-1-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • [COMMITTED] alpha: Fix static gettimeofday symbol
Related show

Commit Message

Adhemerval Zanella Feb. 13, 2020, 11:38 a.m. UTC
By undef strong_alias on alpha implementation, the
default_symbol_version macro becomes an empty macro on static build.
It fixes the issue introduced at c953219420.

Checked on alpha-linux-gnu with a 'make check run-built-tests=no'.
---
 sysdeps/unix/sysv/linux/alpha/gettimeofday.c | 7 ++-----
 time/gettimeofday.c                          | 4 +++-
 2 files changed, 5 insertions(+), 6 deletions(-)

Comments

Florian Weimer Feb. 13, 2020, 12:04 p.m. UTC | #1
* Adhemerval Zanella:

> By undef strong_alias on alpha implementation, the
> default_symbol_version macro becomes an empty macro on static build.
> It fixes the issue introduced at c953219420.

Aren't we using a different mechanism for this in other cases?
Like defining macros for the alias targets?

In any case, SET_VERSION for this macro isn't very descriptive.

Thanks,
Florian
Adhemerval Zanella Feb. 13, 2020, 12:29 p.m. UTC | #2
On 13/02/2020 09:04, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> By undef strong_alias on alpha implementation, the
>> default_symbol_version macro becomes an empty macro on static build.
>> It fixes the issue introduced at c953219420.
> 
> Aren't we using a different mechanism for this in other cases?
> Like defining macros for the alias targets?
> 
> In any case, SET_VERSION for this macro isn't very descriptive.

I don't have a strong opinion here, on some ifunc variant definitions
some implementation undef/redefine the alias macros.  The problems with 
this approach are:

  1. once you have composed macros (such as default_symbol_version).
    
  2. if you want to use the same macro you need to undef after the
     common case inclusion. 

On both cases you will need to redefined it using libc-symbol.h 
internals which defeats the usage of such macros imho (for instance
the usage of _weak_alias or any other internal macros).

So I start to see that just avoid messing with macro undef/def usually
is a cleaner solution.
Florian Weimer Feb. 13, 2020, 12:37 p.m. UTC | #3
* Adhemerval Zanella:

> On 13/02/2020 09:04, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> By undef strong_alias on alpha implementation, the
>>> default_symbol_version macro becomes an empty macro on static build.
>>> It fixes the issue introduced at c953219420.
>> 
>> Aren't we using a different mechanism for this in other cases?
>> Like defining macros for the alias targets?
>> 
>> In any case, SET_VERSION for this macro isn't very descriptive.
>
> I don't have a strong opinion here, on some ifunc variant definitions
> some implementation undef/redefine the alias macros.  The problems with 
> this approach are:
>
>   1. once you have composed macros (such as default_symbol_version).
>     
>   2. if you want to use the same macro you need to undef after the
>      common case inclusion. 
>
> On both cases you will need to redefined it using libc-symbol.h 
> internals which defeats the usage of such macros imho (for instance
> the usage of _weak_alias or any other internal macros).

I thought we had cases where we redefined the *target* identifiers of
these aliases, but maybe I'm wrong about that.

But I'm sure we have cases somewhere where we have a -common.c file
which contains the code, without the aliases.

Thanks,
Florian
Adhemerval Zanella Feb. 13, 2020, 12:43 p.m. UTC | #4
On 13/02/2020 09:37, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 13/02/2020 09:04, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> By undef strong_alias on alpha implementation, the
>>>> default_symbol_version macro becomes an empty macro on static build.
>>>> It fixes the issue introduced at c953219420.
>>>
>>> Aren't we using a different mechanism for this in other cases?
>>> Like defining macros for the alias targets?
>>>
>>> In any case, SET_VERSION for this macro isn't very descriptive.
>>
>> I don't have a strong opinion here, on some ifunc variant definitions
>> some implementation undef/redefine the alias macros.  The problems with 
>> this approach are:
>>
>>   1. once you have composed macros (such as default_symbol_version).
>>     
>>   2. if you want to use the same macro you need to undef after the
>>      common case inclusion. 
>>
>> On both cases you will need to redefined it using libc-symbol.h 
>> internals which defeats the usage of such macros imho (for instance
>> the usage of _weak_alias or any other internal macros).
> 
> I thought we had cases where we redefined the *target* identifiers of
> these aliases, but maybe I'm wrong about that.
> 
> But I'm sure we have cases somewhere where we have a -common.c file
> which contains the code, without the aliases.

It might be a better option, I will prepare a patch.
Adhemerval Zanella Feb. 13, 2020, 12:44 p.m. UTC | #5
On 13/02/2020 09:37, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 13/02/2020 09:04, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> By undef strong_alias on alpha implementation, the
>>>> default_symbol_version macro becomes an empty macro on static build.
>>>> It fixes the issue introduced at c953219420.
>>>
>>> Aren't we using a different mechanism for this in other cases?
>>> Like defining macros for the alias targets?
>>>
>>> In any case, SET_VERSION for this macro isn't very descriptive.
>>
>> I don't have a strong opinion here, on some ifunc variant definitions
>> some implementation undef/redefine the alias macros.  The problems with 
>> this approach are:
>>
>>   1. once you have composed macros (such as default_symbol_version).
>>     
>>   2. if you want to use the same macro you need to undef after the
>>      common case inclusion. 
>>
>> On both cases you will need to redefined it using libc-symbol.h 
>> internals which defeats the usage of such macros imho (for instance
>> the usage of _weak_alias or any other internal macros).
> 
> I thought we had cases where we redefined the *target* identifiers of
> these aliases, but maybe I'm wrong about that.

It works if the generic alias match the expected override code, once
you need to remove/redefine some alias (such as the gettimeofday code)
it becomes quite complex to handle it.

Patch
diff mbox series

diff --git a/sysdeps/unix/sysv/linux/alpha/gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/gettimeofday.c
index 7ad3c6a412..25e86410fd 100644
--- a/sysdeps/unix/sysv/linux/alpha/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/gettimeofday.c
@@ -18,12 +18,9 @@ 
 
 /* We can use the generic implementation, but we have to override its
    default symbol version.  */
-#undef weak_alias
-#define weak_alias(a,b)
-#undef strong_alias
-#define strong_alias(a, b)
+#define SET_VERSION
 #include <time/gettimeofday.c>
 
-_weak_alias (___gettimeofday, __wgettimeofday);
+weak_alias (___gettimeofday, __wgettimeofday);
 default_symbol_version (___gettimeofday, __gettimeofday, GLIBC_2.1);
 default_symbol_version (__wgettimeofday,   gettimeofday, GLIBC_2.1);
diff --git a/time/gettimeofday.c b/time/gettimeofday.c
index 07c6e10d12..e4671edd13 100644
--- a/time/gettimeofday.c
+++ b/time/gettimeofday.c
@@ -35,6 +35,8 @@  ___gettimeofday (struct timeval *restrict tv, void *restrict tz)
   TIMESPEC_TO_TIMEVAL (tv, &ts);
   return 0;
 }
-
+/* Define to override default symbol version.  */
+#ifndef SET_VERSION
 strong_alias (___gettimeofday, __gettimeofday)
 weak_alias (___gettimeofday, gettimeofday)
+#endif