[v2] posix/tst-rfc3484: Fix compile failure linking to local __stat64

Message ID 20210122230538.3423312-1-shorne@gmail.com
State Committed
Commit a4efbf44757477717a907078c340386146c7623f
Delegated to: Adhemerval Zanella Netto
Headers
Series [v2] posix/tst-rfc3484: Fix compile failure linking to local __stat64 |

Commit Message

Stafford Horne Jan. 22, 2021, 11:05 p.m. UTC
  After 04986243d1 ("Remove internal usage of extensible stat functions")
linking the __stat64 symbol in getaddrinfo for this test fails with the
below error:

    /home/shorne/work/gnu-toolchain/local/lib/gcc/or1k-smh-linux-gnu/11.0.0/../../../../or1k-smh-linux-gnu/bin/ld: /home/shorne/work/gnu-toolchain/build-glibc/posix/tst-rfc3484.o: in function `gaiconf_reload':
    /home/shorne/work/gnu-toolchain/glibc/posix/../sysdeps/posix/getaddrinfo.c:2136: undefined reference to `__stat64'
    collect2: error: ld returned 1 exit status

This is because __stat64 is a local symbol, the test includes the
getaddrinfo directly and fails to link against the local symbol.  Fix
this by setting up an alias to the global stat64 symbol name like is
done for other local symbol usage.
---
Changes since v1:
 - Fix typo in commit s/list/like/
 - Address other tests that fail with the same issue

 posix/tst-rfc3484-2.c | 1 +
 posix/tst-rfc3484-3.c | 1 +
 posix/tst-rfc3484.c   | 1 +
 3 files changed, 3 insertions(+)
  

Comments

Adhemerval Zanella Netto Jan. 25, 2021, 2:55 p.m. UTC | #1
On 22/01/2021 20:05, Stafford Horne via Libc-alpha wrote:
> After 04986243d1 ("Remove internal usage of extensible stat functions")
> linking the __stat64 symbol in getaddrinfo for this test fails with the
> below error:
> 
>     /home/shorne/work/gnu-toolchain/local/lib/gcc/or1k-smh-linux-gnu/11.0.0/../../../../or1k-smh-linux-gnu/bin/ld: /home/shorne/work/gnu-toolchain/build-glibc/posix/tst-rfc3484.o: in function `gaiconf_reload':
>     /home/shorne/work/gnu-toolchain/glibc/posix/../sysdeps/posix/getaddrinfo.c:2136: undefined reference to `__stat64'
>     collect2: error: ld returned 1 exit status
> 
> This is because __stat64 is a local symbol, the test includes the
> getaddrinfo directly and fails to link against the local symbol.  Fix
> this by setting up an alias to the global stat64 symbol name like is
> done for other local symbol usage.

I am trying to understand why I haven't see a similar issue for riscv32,
since it does not provide any xstat symbols.  It seems that for tests,
I am not seeing any __stat64 call, but rather __fstat64 ones:

  $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484.o | grep stat
  00000000         *UND*  00000000 __fstat64
  $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484-2.o | grep stat
  00000000         *UND*  00000000 __fstat64
  $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484-3.o | grep stat
  00000000         *UND*  00000000 __fstat64

It comes from gaiconf_init and I am not sure why __stat64 call from
gaiconf_reload is not being generated (maybe this is being optimized
out).

And the __fstat64 seems to not generate a linking error because is being
exported by libc.so in GLIBC_PRIVATE namespace (due 8ed005daf0ab03). I
added this because of the libpthread usage, but it seems we don't
actually required it.  For 2.34 I plan to send:

diff --git a/io/Versions b/io/Versions
index 49c4d2d40a..4ffafd4bde 100644
--- a/io/Versions
+++ b/io/Versions
@@ -145,6 +145,5 @@ libc {
     __file_change_detection_for_stat;
     __file_change_detection_for_path;
     __file_change_detection_for_fp;
-    __fstat64;
   }
 }
diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c
index b0b722121d..b3b4de779b 100644
--- a/sysdeps/pthread/sem_open.c
+++ b/sysdeps/pthread/sem_open.c
@@ -68,7 +68,7 @@ check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
 
   /* Get the information about the file.  */
   struct stat64 st;
-  if (__fstat64 (fd, &st) == 0)
+  if (fstat64 (fd, &st) == 0)
     {
       /* Get the lock.  */
       lll_lock (__sem_mappings_lock, LLL_PRIVATE);

To remove the __fstat64 requirement on GLIBC_PRIVATE.

This patch looks ok on 2.34 opens.

> ---
> Changes since v1:
>  - Fix typo in commit s/list/like/
>  - Address other tests that fail with the same issue
> 
>  posix/tst-rfc3484-2.c | 1 +
>  posix/tst-rfc3484-3.c | 1 +
>  posix/tst-rfc3484.c   | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/posix/tst-rfc3484-2.c b/posix/tst-rfc3484-2.c
> index 5f5ada9420..ce8ccd5f38 100644
> --- a/posix/tst-rfc3484-2.c
> +++ b/posix/tst-rfc3484-2.c
> @@ -11,6 +11,7 @@
>  #define __gethostbyaddr_r gethostbyaddr_r
>  #define __gethostbyname2_r gethostbyname2_r
>  #define __qsort_r qsort_r
> +#define __stat64 stat64
>  
>  void
>  attribute_hidden
> diff --git a/posix/tst-rfc3484-3.c b/posix/tst-rfc3484-3.c
> index d9ec5cc851..ecb163963f 100644
> --- a/posix/tst-rfc3484-3.c
> +++ b/posix/tst-rfc3484-3.c
> @@ -11,6 +11,7 @@
>  #define __gethostbyaddr_r gethostbyaddr_r
>  #define __gethostbyname2_r gethostbyname2_r
>  #define __qsort_r qsort_r
> +#define __stat64 stat64
>  
>  void
>  attribute_hidden
> diff --git a/posix/tst-rfc3484.c b/posix/tst-rfc3484.c
> index 97d065b6bf..3b2052eb54 100644
> --- a/posix/tst-rfc3484.c
> +++ b/posix/tst-rfc3484.c
> @@ -11,6 +11,7 @@
>  #define __gethostbyaddr_r gethostbyaddr_r
>  #define __gethostbyname2_r gethostbyname2_r
>  #define __qsort_r qsort_r
> +#define __stat64 stat64
>  
>  void
>  attribute_hidden
>
  
Stafford Horne Jan. 27, 2021, 12:37 p.m. UTC | #2
On Mon, Jan 25, 2021 at 11:55:43AM -0300, Adhemerval Zanella wrote:
> 
> 
> On 22/01/2021 20:05, Stafford Horne via Libc-alpha wrote:
> > After 04986243d1 ("Remove internal usage of extensible stat functions")
> > linking the __stat64 symbol in getaddrinfo for this test fails with the
> > below error:
> > 
> >     /home/shorne/work/gnu-toolchain/local/lib/gcc/or1k-smh-linux-gnu/11.0.0/../../../../or1k-smh-linux-gnu/bin/ld: /home/shorne/work/gnu-toolchain/build-glibc/posix/tst-rfc3484.o: in function `gaiconf_reload':
> >     /home/shorne/work/gnu-toolchain/glibc/posix/../sysdeps/posix/getaddrinfo.c:2136: undefined reference to `__stat64'
> >     collect2: error: ld returned 1 exit status
> > 
> > This is because __stat64 is a local symbol, the test includes the
> > getaddrinfo directly and fails to link against the local symbol.  Fix
> > this by setting up an alias to the global stat64 symbol name like is
> > done for other local symbol usage.
> 
> I am trying to understand why I haven't see a similar issue for riscv32,
> since it does not provide any xstat symbols.  It seems that for tests,
> I am not seeing any __stat64 call, but rather __fstat64 ones:
> 
>   $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484.o | grep stat
>   00000000         *UND*  00000000 __fstat64
>   $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484-2.o | grep stat
>   00000000         *UND*  00000000 __fstat64
>   $ riscv32-glibc-linux-gnu-objdump -t posix/tst-rfc3484-3.o | grep stat
>   00000000         *UND*  00000000 __fstat64
> 
> It comes from gaiconf_init and I am not sure why __stat64 call from
> gaiconf_reload is not being generated (maybe this is being optimized
> out).
> 
> And the __fstat64 seems to not generate a linking error because is being
> exported by libc.so in GLIBC_PRIVATE namespace (due 8ed005daf0ab03). I
> added this because of the libpthread usage, but it seems we don't
> actually required it.  For 2.34 I plan to send:

Thanks for the explaination.  I also wonder why it wasn't showing up in other
platforms.

> diff --git a/io/Versions b/io/Versions
> index 49c4d2d40a..4ffafd4bde 100644
> --- a/io/Versions
> +++ b/io/Versions
> @@ -145,6 +145,5 @@ libc {
>      __file_change_detection_for_stat;
>      __file_change_detection_for_path;
>      __file_change_detection_for_fp;
> -    __fstat64;
>    }
>  }
> diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c
> index b0b722121d..b3b4de779b 100644
> --- a/sysdeps/pthread/sem_open.c
> +++ b/sysdeps/pthread/sem_open.c
> @@ -68,7 +68,7 @@ check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
>  
>    /* Get the information about the file.  */
>    struct stat64 st;
> -  if (__fstat64 (fd, &st) == 0)
> +  if (fstat64 (fd, &st) == 0)
>      {
>        /* Get the lock.  */
>        lll_lock (__sem_mappings_lock, LLL_PRIVATE);
> 
> To remove the __fstat64 requirement on GLIBC_PRIVATE.
> 
> This patch looks ok on 2.34 opens.
> 
> > ---
> > Changes since v1:
> >  - Fix typo in commit s/list/like/
> >  - Address other tests that fail with the same issue
> > 
> >  posix/tst-rfc3484-2.c | 1 +
> >  posix/tst-rfc3484-3.c | 1 +
> >  posix/tst-rfc3484.c   | 1 +
> >  3 files changed, 3 insertions(+)
> > 
> > diff --git a/posix/tst-rfc3484-2.c b/posix/tst-rfc3484-2.c
> > index 5f5ada9420..ce8ccd5f38 100644
> > --- a/posix/tst-rfc3484-2.c
> > +++ b/posix/tst-rfc3484-2.c
> > @@ -11,6 +11,7 @@
> >  #define __gethostbyaddr_r gethostbyaddr_r
> >  #define __gethostbyname2_r gethostbyname2_r
> >  #define __qsort_r qsort_r
> > +#define __stat64 stat64
> >  
> >  void
> >  attribute_hidden
> > diff --git a/posix/tst-rfc3484-3.c b/posix/tst-rfc3484-3.c
> > index d9ec5cc851..ecb163963f 100644
> > --- a/posix/tst-rfc3484-3.c
> > +++ b/posix/tst-rfc3484-3.c
> > @@ -11,6 +11,7 @@
> >  #define __gethostbyaddr_r gethostbyaddr_r
> >  #define __gethostbyname2_r gethostbyname2_r
> >  #define __qsort_r qsort_r
> > +#define __stat64 stat64
> >  
> >  void
> >  attribute_hidden
> > diff --git a/posix/tst-rfc3484.c b/posix/tst-rfc3484.c
> > index 97d065b6bf..3b2052eb54 100644
> > --- a/posix/tst-rfc3484.c
> > +++ b/posix/tst-rfc3484.c
> > @@ -11,6 +11,7 @@
> >  #define __gethostbyaddr_r gethostbyaddr_r
> >  #define __gethostbyname2_r gethostbyname2_r
> >  #define __qsort_r qsort_r
> > +#define __stat64 stat64
> >  
> >  void
> >  attribute_hidden
> >
  

Patch

diff --git a/posix/tst-rfc3484-2.c b/posix/tst-rfc3484-2.c
index 5f5ada9420..ce8ccd5f38 100644
--- a/posix/tst-rfc3484-2.c
+++ b/posix/tst-rfc3484-2.c
@@ -11,6 +11,7 @@ 
 #define __gethostbyaddr_r gethostbyaddr_r
 #define __gethostbyname2_r gethostbyname2_r
 #define __qsort_r qsort_r
+#define __stat64 stat64
 
 void
 attribute_hidden
diff --git a/posix/tst-rfc3484-3.c b/posix/tst-rfc3484-3.c
index d9ec5cc851..ecb163963f 100644
--- a/posix/tst-rfc3484-3.c
+++ b/posix/tst-rfc3484-3.c
@@ -11,6 +11,7 @@ 
 #define __gethostbyaddr_r gethostbyaddr_r
 #define __gethostbyname2_r gethostbyname2_r
 #define __qsort_r qsort_r
+#define __stat64 stat64
 
 void
 attribute_hidden
diff --git a/posix/tst-rfc3484.c b/posix/tst-rfc3484.c
index 97d065b6bf..3b2052eb54 100644
--- a/posix/tst-rfc3484.c
+++ b/posix/tst-rfc3484.c
@@ -11,6 +11,7 @@ 
 #define __gethostbyaddr_r gethostbyaddr_r
 #define __gethostbyname2_r gethostbyname2_r
 #define __qsort_r qsort_r
+#define __stat64 stat64
 
 void
 attribute_hidden