linux: Define STAT64_IS_KERNEL_STAT64 by default

  In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
back.

For architectures that want to used the default kernel_stat.h and do not
have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
the default as 1 as modern port's stat64 struct should match the kernel
stat64 layout.

I tested this on the OpenRISC port and it seems to work fine.  Currently,
all archs that use the default kernel_stat.h define __NR_newfstatat so
they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
to be an outlier it uses the default kernel_stat.h, but does not define
__NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
---
 sysdeps/unix/sysv/linux/kernel_stat.h | 1 +
 1 file changed, 1 insertion(+)
  

On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote:
> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
> default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
> back.
> 
> For architectures that want to used the default kernel_stat.h and do not
> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
> the default as 1 as modern port's stat64 struct should match the kernel
> stat64 layout.
> 
> I tested this on the OpenRISC port and it seems to work fine.  Currently,
> all archs that use the default kernel_stat.h define __NR_newfstatat so
> they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
> to be an outlier it uses the default kernel_stat.h, but does not define
> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.

arc and usually newer 32-bit ports will only use __NR_statx:

138 #if (__WORDSIZE == 32 \
139      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
140      || defined STAT_HAS_TIME32
141 # define FSTATAT_USE_STATX 1
142 #else
143 # define FSTATAT_USE_STATX 0
144 #endif

The patch looks ok, but it seems strange that ork1 requires it since it
setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX
would be defined and only fstatat64_time64_statx would be used.

> ---
>  sysdeps/unix/sysv/linux/kernel_stat.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h
> index 6c1b175cb4..b8c376f41e 100644
> --- a/sysdeps/unix/sysv/linux/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/kernel_stat.h
> @@ -20,3 +20,4 @@
>  #define XSTAT_IS_XSTAT64     1
>  #define STATFS_IS_STATFS64   __STATFS_MATCHES_STATFS64
>  #define STAT_IS_KERNEL_STAT  1
> +#define STAT64_IS_KERNEL_STAT64 1
>
  

On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote:
> 
> 
> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote:
> > In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
> > default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
> > back.
> > 
> > For architectures that want to used the default kernel_stat.h and do not
> > have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
> > the default as 1 as modern port's stat64 struct should match the kernel
> > stat64 layout.
> > 
> > I tested this on the OpenRISC port and it seems to work fine.  Currently,
> > all archs that use the default kernel_stat.h define __NR_newfstatat so
> > they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
> > to be an outlier it uses the default kernel_stat.h, but does not define
> > __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
> 
> arc and usually newer 32-bit ports will only use __NR_statx:
> 
> 138 #if (__WORDSIZE == 32 \
> 139      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> 140      || defined STAT_HAS_TIME32
> 141 # define FSTATAT_USE_STATX 1
> 142 #else
> 143 # define FSTATAT_USE_STATX 0
> 144 #endif
> 
> The patch looks ok, but it seems strange that ork1 requires it since it
> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX
> would be defined and only fstatat64_time64_statx would be used.

Right,

In that case maybe another ifdef is needed in fstatat64.c?  I don't see
fstatat64_time64_stat is actually getting compiled into the libc.so binary.  But
if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error.

I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in
the compile unit and I get the below which looks right:

  * __ASSUME_STATX: 1
  * FSTATAT_USE_STATX: 1

Error:

or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno      -ftls-model=initial-exec      -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io  -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc  -I../sysdeps/unix/sysv/linux/or1k  -I../sysdeps/or1k/nptl  -I../sysdeps/unix/sysv/linux/generic/wordsize-32  -I../sysdeps/unix/sysv/linux/generic  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/or1k/nofpu  -I../sysdeps/ieee754/soft-fp  -I../sysdeps/or1k  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.  -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h       -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o
../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’:
../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef]
   89 | #  if STAT64_IS_KERNEL_STAT64
      |       ^~~~~~~~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known
   94 |   struct kernel_stat64 kst64;
      |                        ^~~~~
../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration]
   97 |     __cp_stat64_kstat64 (buf, &kst64);
      |     ^~~~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable]
   94 |   struct kernel_stat64 kst64;
      |                        ^~~~~
../sysdeps/unix/sysv/linux/fstatat64.c: At top level:
../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’
  149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX)
      |         ^~~~~~~
../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’
  150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX)
      |         ^~~~~~~

Again I am not sure how arc avoids this error I shall try to compile it too.

-Stafford

> > ---
> >  sysdeps/unix/sysv/linux/kernel_stat.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h
> > index 6c1b175cb4..b8c376f41e 100644
> > --- a/sysdeps/unix/sysv/linux/kernel_stat.h
> > +++ b/sysdeps/unix/sysv/linux/kernel_stat.h
> > @@ -20,3 +20,4 @@
> >  #define XSTAT_IS_XSTAT64     1
> >  #define STATFS_IS_STATFS64   __STATFS_MATCHES_STATFS64
> >  #define STAT_IS_KERNEL_STAT  1
> > +#define STAT64_IS_KERNEL_STAT64 1
> > 
>
  

On 23/11/2021 18:39, Stafford Horne wrote:
> On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote:
>>
>>
>> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote:
>>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
>>> default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
>>> back.
>>>
>>> For architectures that want to used the default kernel_stat.h and do not
>>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
>>> the default as 1 as modern port's stat64 struct should match the kernel
>>> stat64 layout.
>>>
>>> I tested this on the OpenRISC port and it seems to work fine.  Currently,
>>> all archs that use the default kernel_stat.h define __NR_newfstatat so
>>> they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
>>> to be an outlier it uses the default kernel_stat.h, but does not define
>>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
>>
>> arc and usually newer 32-bit ports will only use __NR_statx:
>>
>> 138 #if (__WORDSIZE == 32 \
>> 139      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
>> 140      || defined STAT_HAS_TIME32
>> 141 # define FSTATAT_USE_STATX 1
>> 142 #else
>> 143 # define FSTATAT_USE_STATX 0
>> 144 #endif
>>
>> The patch looks ok, but it seems strange that ork1 requires it since it
>> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX
>> would be defined and only fstatat64_time64_statx would be used.
> 
> Right,
> 
> In that case maybe another ifdef is needed in fstatat64.c?  I don't see
> fstatat64_time64_stat is actually getting compiled into the libc.so binary.  But
> if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error.
> 
> I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in
> the compile unit and I get the below which looks right:
> 
>   * __ASSUME_STATX: 1
>   * FSTATAT_USE_STATX: 1
> 
> Error:
> 
> or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno      -ftls-model=initial-exec      -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io  -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc  -I../sysdeps/unix/sysv/linux/or1k  -I../sysdeps/or1k/nptl  -I../sysdeps/unix/sysv/linux/generic/wordsize-32  -I../sysdeps/unix/sysv/linux/generic  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/or1k/nofpu  -I../sysdeps/ieee754/soft-fp  -I../sysdeps/or1k  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.  -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h       -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o
> ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’:
> ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef]
>    89 | #  if STAT64_IS_KERNEL_STAT64
>       |       ^~~~~~~~~~~~~~~~~~~~~~~
> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known
>    94 |   struct kernel_stat64 kst64;
>       |                        ^~~~~
> ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration]
>    97 |     __cp_stat64_kstat64 (buf, &kst64);
>       |     ^~~~~~~~~~~~~~~~~~~
> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable]
>    94 |   struct kernel_stat64 kst64;
>       |                        ^~~~~
> ../sysdeps/unix/sysv/linux/fstatat64.c: At top level:
> ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’
>   149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX)
>       |         ^~~~~~~
> ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’
>   150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX)
>       |         ^~~~~~~
> 
> Again I am not sure how arc avoids this error I shall try to compile it too.

The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures
with __ASSUME_STATX (since only statx provides 64-bit timestamps).  Other architecture
might get an improved codegen using older syscalls (specially 64-bit ones), so statx
should be used only when really required.

The patch below fixes it:

diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
index f968e4ef05..50ae5ad748 100644
--- a/sysdeps/unix/sysv/linux/fstatat64.c
+++ b/sysdeps/unix/sysv/linux/fstatat64.c
@@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
   return r;
 }
 
+#if (__WORDSIZE == 32 \
+     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
+     || defined STAT_HAS_TIME32
+# define FSTATAT_USE_STATX 1
+#else
+# define FSTATAT_USE_STATX 0
+#endif
+
+/* Only statx supports 64-bit timestamps for 32-bit architectures with
+   __ASSUME_STATX, so there is no point in building the fallback.  */
+#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX)
 static inline int
 fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
                       int flag)
@@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
 
   return r;
 }
-
-#if (__WORDSIZE == 32 \
-     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
-     || defined STAT_HAS_TIME32
-# define FSTATAT_USE_STATX 1
-#else
-# define FSTATAT_USE_STATX 0
 #endif
 
 int
  

On Wed, Nov 24, 2021 at 12:50:40PM -0300, Adhemerval Zanella wrote:
> 
> 
> On 23/11/2021 18:39, Stafford Horne wrote:
> > On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote:
> >>
> >>
> >> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote:
> >>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
> >>> default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
> >>> back.
> >>>
> >>> For architectures that want to used the default kernel_stat.h and do not
> >>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
> >>> the default as 1 as modern port's stat64 struct should match the kernel
> >>> stat64 layout.
> >>>
> >>> I tested this on the OpenRISC port and it seems to work fine.  Currently,
> >>> all archs that use the default kernel_stat.h define __NR_newfstatat so
> >>> they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
> >>> to be an outlier it uses the default kernel_stat.h, but does not define
> >>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
> >>
> >> arc and usually newer 32-bit ports will only use __NR_statx:
> >>
> >> 138 #if (__WORDSIZE == 32 \
> >> 139      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> >> 140      || defined STAT_HAS_TIME32
> >> 141 # define FSTATAT_USE_STATX 1
> >> 142 #else
> >> 143 # define FSTATAT_USE_STATX 0
> >> 144 #endif
> >>
> >> The patch looks ok, but it seems strange that ork1 requires it since it
> >> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX
> >> would be defined and only fstatat64_time64_statx would be used.
> > 
> > Right,
> > 
> > In that case maybe another ifdef is needed in fstatat64.c?  I don't see
> > fstatat64_time64_stat is actually getting compiled into the libc.so binary.  But
> > if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error.
> > 
> > I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in
> > the compile unit and I get the below which looks right:
> > 
> >   * __ASSUME_STATX: 1
> >   * FSTATAT_USE_STATX: 1
> > 
> > Error:
> > 
> > or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno      -ftls-model=initial-exec      -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io  -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc  -I../sysdeps/unix/sysv/linux/or1k  -I../sysdeps/or1k/nptl  -I../sysdeps/unix/sysv/linux/generic/wordsize-32  -I../sysdeps/unix/sysv/linux/generic  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/or1k/nofpu  -I../sysdeps/ieee754/soft-fp  -I../sysdeps/or1k  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.  -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h       -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o
> > ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’:
> > ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef]
> >    89 | #  if STAT64_IS_KERNEL_STAT64
> >       |       ^~~~~~~~~~~~~~~~~~~~~~~
> > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known
> >    94 |   struct kernel_stat64 kst64;
> >       |                        ^~~~~
> > ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration]
> >    97 |     __cp_stat64_kstat64 (buf, &kst64);
> >       |     ^~~~~~~~~~~~~~~~~~~
> > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable]
> >    94 |   struct kernel_stat64 kst64;
> >       |                        ^~~~~
> > ../sysdeps/unix/sysv/linux/fstatat64.c: At top level:
> > ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’
> >   149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX)
> >       |         ^~~~~~~
> > ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’
> >   150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX)
> >       |         ^~~~~~~
> > 
> > Again I am not sure how arc avoids this error I shall try to compile it too.
> 
> The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures
> with __ASSUME_STATX (since only statx provides 64-bit timestamps).  Other architecture
> might get an improved codegen using older syscalls (specially 64-bit ones), so statx
> should be used only when really required.
> 
> The patch below fixes it:

Right, this was what I was thinking too.  However, I am still not sure why or1k
needs it and arc does not.  I tested the arc build and confirmed with the same
version of GCC the arc build passes but or1k fails with the above error.  So as
you said, maybe there is better codegen somewhere.

> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index f968e4ef05..50ae5ad748 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
>    return r;
>  }
>  
> +#if (__WORDSIZE == 32 \
> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> +     || defined STAT_HAS_TIME32
> +# define FSTATAT_USE_STATX 1
> +#else
> +# define FSTATAT_USE_STATX 0
> +#endif
> +
> +/* Only statx supports 64-bit timestamps for 32-bit architectures with
> +   __ASSUME_STATX, so there is no point in building the fallback.  */
> +#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX)
>  static inline int
>  fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>                        int flag)
> @@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>  
>    return r;
>  }
> -
> -#if (__WORDSIZE == 32 \
> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> -     || defined STAT_HAS_TIME32
> -# define FSTATAT_USE_STATX 1
> -#else
> -# define FSTATAT_USE_STATX 0
>  #endif
>
>  int

I tested this patch and it works.

-Stafford
  

On 24/11/2021 18:29, Stafford Horne wrote:
> On Wed, Nov 24, 2021 at 12:50:40PM -0300, Adhemerval Zanella wrote:
>>
>>
>> On 23/11/2021 18:39, Stafford Horne wrote:
>>> On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote:
>>>>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
>>>>> default for STAT64_IS_KERNEL_STAT64 was removed.  This patch adds it
>>>>> back.
>>>>>
>>>>> For architectures that want to used the default kernel_stat.h and do not
>>>>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined.  Set
>>>>> the default as 1 as modern port's stat64 struct should match the kernel
>>>>> stat64 layout.
>>>>>
>>>>> I tested this on the OpenRISC port and it seems to work fine.  Currently,
>>>>> all archs that use the default kernel_stat.h define __NR_newfstatat so
>>>>> they will not use the STAT64_IS_KERNEL_STAT64 macro.  However, arc seems
>>>>> to be an outlier it uses the default kernel_stat.h, but does not define
>>>>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
>>>>
>>>> arc and usually newer 32-bit ports will only use __NR_statx:
>>>>
>>>> 138 #if (__WORDSIZE == 32 \
>>>> 139      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
>>>> 140      || defined STAT_HAS_TIME32
>>>> 141 # define FSTATAT_USE_STATX 1
>>>> 142 #else
>>>> 143 # define FSTATAT_USE_STATX 0
>>>> 144 #endif
>>>>
>>>> The patch looks ok, but it seems strange that ork1 requires it since it
>>>> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX
>>>> would be defined and only fstatat64_time64_statx would be used.
>>>
>>> Right,
>>>
>>> In that case maybe another ifdef is needed in fstatat64.c?  I don't see
>>> fstatat64_time64_stat is actually getting compiled into the libc.so binary.  But
>>> if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error.
>>>
>>> I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in
>>> the compile unit and I get the below which looks right:
>>>
>>>   * __ASSUME_STATX: 1
>>>   * FSTATAT_USE_STATX: 1
>>>
>>> Error:
>>>
>>> or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno      -ftls-model=initial-exec      -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io  -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc  -I../sysdeps/unix/sysv/linux/or1k  -I../sysdeps/or1k/nptl  -I../sysdeps/unix/sysv/linux/generic/wordsize-32  -I../sysdeps/unix/sysv/linux/generic  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/or1k/nofpu  -I../sysdeps/ieee754/soft-fp  -I../sysdeps/or1k  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.  -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h       -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o
>>> ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’:
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef]
>>>    89 | #  if STAT64_IS_KERNEL_STAT64
>>>       |       ^~~~~~~~~~~~~~~~~~~~~~~
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known
>>>    94 |   struct kernel_stat64 kst64;
>>>       |                        ^~~~~
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration]
>>>    97 |     __cp_stat64_kstat64 (buf, &kst64);
>>>       |     ^~~~~~~~~~~~~~~~~~~
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable]
>>>    94 |   struct kernel_stat64 kst64;
>>>       |                        ^~~~~
>>> ../sysdeps/unix/sysv/linux/fstatat64.c: At top level:
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’
>>>   149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX)
>>>       |         ^~~~~~~
>>> ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’
>>>   150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX)
>>>       |         ^~~~~~~
>>>
>>> Again I am not sure how arc avoids this error I shall try to compile it too.
>>
>> The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures
>> with __ASSUME_STATX (since only statx provides 64-bit timestamps).  Other architecture
>> might get an improved codegen using older syscalls (specially 64-bit ones), so statx
>> should be used only when really required.
>>
>> The patch below fixes it:
> 
> Right, this was what I was thinking too.  However, I am still not sure why or1k
> needs it and arc does not.  I tested the arc build and confirmed with the same
> version of GCC the arc build passes but or1k fails with the above error.  So as
> you said, maybe there is better codegen somewhere.

ARC has:

sysdeps/unix/sysv/linux/arc/fixup-asm-unistd.h:24:#undef __NR_fstatat64

Which leads fstatat64_time64_stat to be an empty function:

static inline int
fstatat64_time64_stat (int fd, const char *file, struct stat64 *buf,
         int flag)
{ 
  int r;
# 135 "../sysdeps/unix/sysv/linux/fstatat64.c"
  return r;
} 


> 
>> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
>> index f968e4ef05..50ae5ad748 100644
>> --- a/sysdeps/unix/sysv/linux/fstatat64.c
>> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
>> @@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
>>    return r;
>>  }
>>  
>> +#if (__WORDSIZE == 32 \
>> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
>> +     || defined STAT_HAS_TIME32
>> +# define FSTATAT_USE_STATX 1
>> +#else
>> +# define FSTATAT_USE_STATX 0
>> +#endif
>> +
>> +/* Only statx supports 64-bit timestamps for 32-bit architectures with
>> +   __ASSUME_STATX, so there is no point in building the fallback.  */
>> +#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX)
>>  static inline int
>>  fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>>                        int flag)
>> @@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>>  
>>    return r;
>>  }
>> -
>> -#if (__WORDSIZE == 32 \
>> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
>> -     || defined STAT_HAS_TIME32
>> -# define FSTATAT_USE_STATX 1
>> -#else
>> -# define FSTATAT_USE_STATX 0
>>  #endif
>>
>>  int
> 
> I tested this patch and it works.

Thanks, I will push it upstream.