[COMMITTED] alpha: Fix static gettimeofday symbol

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.
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(-)

-- 
2.17.1

Comments

Florian Weimer Feb. 13, 2020, 12:04 p.m. | #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. | #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. | #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. | #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. | #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 --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