diff mbox

asm-generic ABI: Allow statx syscall despite fstatat64,fstat64

Message ID 20200224182305.28027-1-vgupta@synopsys.com
State New
Headers show

Commit Message

Vineet Gupta Feb. 24, 2020, 6:23 p.m. UTC
An old version of asm-generic syscall ABI (e.g. ARC 2013) provided
fstat[at]64 syscalls for implementing the stat family.

These sycalls are passthru (no interworking/itemized-copy needed
between kernel and user structs since they are binary comaptible).
However with switch to 64-bit time_t the compatibily goes away so we
need explicit interworking. Luckily such code already exists albeit it
uses the statx syscall.

So this patch reworks the asm-generic based stat functions to use the
statx code-path if the arch so desires. It essentially reworks the commit
6bbfc5c09fc5b5e3 ("Add statx conditionals for wordsize-32 *xstat.c")

This will be used by ARC port for 64-bit time_t. But it has no real
bearing on 64-bit and can just be independently opted-in by an arch port
provided the ABI supports it.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 sysdeps/unix/sysv/linux/fxstat64.c                     |  8 ++++----
 sysdeps/unix/sysv/linux/fxstatat64.c                   |  6 +++---
 sysdeps/unix/sysv/linux/generic/kernel_stat.h          |  7 +++++++
 sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat.c   |  6 +++---
 sysdeps/unix/sysv/linux/generic/wordsize-32/fxstatat.c |  6 +++---
 sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat.c   | 10 +++++-----
 sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat64.c |  8 ++++----
 sysdeps/unix/sysv/linux/generic/wordsize-32/xstat.c    |  6 +++---
 sysdeps/unix/sysv/linux/generic/wordsize-32/xstat64.c  |  6 +++---
 sysdeps/unix/sysv/linux/statx_cp.c                     |  2 +-
 10 files changed, 36 insertions(+), 29 deletions(-)

Comments

Florian Weimer Feb. 24, 2020, 6:39 p.m. UTC | #1
* Vineet Gupta:

> +/* Ports define to 1 when glibc struct stat64 is not compatible with
> +   kernel stat64. This forces use of statx syscall (and explicit interworking)
> +   even when fstat64 syscall is available. For asm-generic ABI this happened
> +   with switch to 64-bit time_t.  */
> +
> +# undef STAT64_IS_NOT_KERNEL_STAT64

Sorry, I think that going forward, we prefer that such macros are always
defined, with values 0 or 1 as appropriate.

Thanks,
Florian
Vineet Gupta Feb. 24, 2020, 7:03 p.m. UTC | #2
On 2/24/20 10:39 AM, Florian Weimer wrote:
>> +# undef STAT64_IS_NOT_KERNEL_STAT64

>

> Sorry, I think that going forward, we prefer that such macros are always

> defined, with values 0 or 1 as appropriate.


And that means we also need to additionally define this to 0 in all ports which
don't end up including the generic header ?

Thx,
-Vineet
Florian Weimer Feb. 24, 2020, 7:13 p.m. UTC | #3
* Vineet Gupta:

> On 2/24/20 10:39 AM, Florian Weimer wrote:
>>> +# undef STAT64_IS_NOT_KERNEL_STAT64
>>
>> Sorry, I think that going forward, we prefer that such macros are always
>> defined, with values 0 or 1 as appropriate.
>
> And that means we also need to additionally define this to 0 in all
> ports which don't end up including the generic header ?

Yes, or move the definition into its own header, with the default for
*future* architectures (not the majority of the current architectures)
in the header in the sysdeps/unix/sysv/linux directory.

Thanks,
Florian
Vineet Gupta Feb. 24, 2020, 7:27 p.m. UTC | #4
On 2/24/20 11:13 AM, Florian Weimer wrote:
> * Vineet Gupta:

> 

>> On 2/24/20 10:39 AM, Florian Weimer wrote:

>>>> +# undef STAT64_IS_NOT_KERNEL_STAT64

>>> Sorry, I think that going forward, we prefer that such macros are always

>>> defined, with values 0 or 1 as appropriate.

>>

>> And that means we also need to additionally define this to 0 in all

>> ports which don't end up including the generic header ?

>

> Yes, or move the definition into its own header, with the default for

> *future* architectures (not the majority of the current architectures)

> in the header in the sysdeps/unix/sysv/linux directory.


Adding a new file would seem too kludgy. Adding it to existing arch headers is
still OK but then one needs to know what exact syscall ABI each arch has as even
asm-generic ones differ depending on when the linux port was merged. And the only
way to know that is to do a build for each arch or is there some documentation one
could refer to.

Thx,
-Vineet
Florian Weimer Feb. 24, 2020, 7:38 p.m. UTC | #5
* Vineet Gupta:

> On 2/24/20 11:13 AM, Florian Weimer wrote:
>> * Vineet Gupta:
>> 
>>> On 2/24/20 10:39 AM, Florian Weimer wrote:
>>>>> +# undef STAT64_IS_NOT_KERNEL_STAT64
>>>> Sorry, I think that going forward, we prefer that such macros are always
>>>> defined, with values 0 or 1 as appropriate.
>>>
>>> And that means we also need to additionally define this to 0 in all
>>> ports which don't end up including the generic header ?
>>
>> Yes, or move the definition into its own header, with the default for
>> *future* architectures (not the majority of the current architectures)
>> in the header in the sysdeps/unix/sysv/linux directory.
>
> Adding a new file would seem too kludgy.

I think it's the preferred approach.  Here's a recent discussion that
looks at a few options in this space:

  <https://sourceware.org/ml/libc-alpha/2019-10/msg00030.html>

> Adding it to existing arch headers is still OK but then one needs to
> know what exact syscall ABI each arch has as even asm-generic ones
> differ depending on when the linux port was merged. And the only way
> to know that is to do a build for each arch or is there some
> documentation one could refer to.

In your case, the build should fail somewhere if there is a mismatch and
the #define is incorrect for a particular architecture.  In this case,
you can keep running build-many-glibcs.py and making tweaks to the
failing architectures until all failures are gone.

Thanks,
Florian
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/fxstat64.c b/sysdeps/unix/sysv/linux/fxstat64.c
index 9133a0a29b8e..d4da85f7949c 100644
--- a/sysdeps/unix/sysv/linux/fxstat64.c
+++ b/sysdeps/unix/sysv/linux/fxstat64.c
@@ -33,15 +33,15 @@  int
 ___fxstat64 (int vers, int fd, struct stat64 *buf)
 {
   int result;
-#ifdef __NR_fstat64
-  result = INLINE_SYSCALL (fstat64, 2, fd, buf);
-#else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
   struct statx tmp;
   result = INLINE_SYSCALL (statx, 5, fd, "", AT_EMPTY_PATH, STATX_BASIC_STATS,
                            &tmp);
   if (result == 0)
     __cp_stat64_statx (buf, &tmp);
-#endif
+# else
+  result = INLINE_SYSCALL (fstat64, 2, fd, buf);
+# endif
   return result;
 }
 
diff --git a/sysdeps/unix/sysv/linux/fxstatat64.c b/sysdeps/unix/sysv/linux/fxstatat64.c
index e24b456604ff..eb59c8be845f 100644
--- a/sysdeps/unix/sysv/linux/fxstatat64.c
+++ b/sysdeps/unix/sysv/linux/fxstatat64.c
@@ -38,15 +38,15 @@  __fxstatat64 (int vers, int fd, const char *file, struct stat64 *st, int flag)
 
   int result;
 
-#ifdef __NR_fstatat64
-  result = INTERNAL_SYSCALL_CALL (fstatat64, fd, file, st, flag);
-#else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstatat64)
   struct statx tmp;
 
   result = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
 				  STATX_BASIC_STATS, &tmp);
   if (result == 0)
     __cp_stat64_statx (st, &tmp);
+#else
+  result = INTERNAL_SYSCALL_CALL (fstatat64, fd, file, st, flag);
 #endif
   if (!__glibc_likely (INTERNAL_SYSCALL_ERROR_P (result)))
     return 0;
diff --git a/sysdeps/unix/sysv/linux/generic/kernel_stat.h b/sysdeps/unix/sysv/linux/generic/kernel_stat.h
index 2eed3596c0ed..916deb7ed8c1 100644
--- a/sysdeps/unix/sysv/linux/generic/kernel_stat.h
+++ b/sysdeps/unix/sysv/linux/generic/kernel_stat.h
@@ -28,3 +28,10 @@ 
 #endif
 
 #define STATFS_IS_STATFS64 0
+
+/* Ports define to 1 when glibc struct stat64 is not compatible with
+   kernel stat64. This forces use of statx syscall (and explicit interworking)
+   even when fstat64 syscall is available. For asm-generic ABI this happened
+   with switch to 64-bit time_t.  */
+
+# undef STAT64_IS_NOT_KERNEL_STAT64
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat.c
index 850450e1e8a1..8ab11c62d1b3 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat.c
@@ -36,14 +36,14 @@  __fxstat (int vers, int fd, struct stat *buf)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-# ifdef __NR_fstat64
-      int rc = INLINE_SYSCALL (fstat64, 2, fd, buf);
-# else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, fd, "", AT_EMPTY_PATH,
                                STATX_BASIC_STATS, &tmp);
       if (rc == 0)
         __cp_stat64_statx ((struct stat64 *)buf, &tmp);
+# else
+      int rc = INLINE_SYSCALL (fstat64, 2, fd, buf);
 # endif
       return rc ?: stat_overflow (buf);
     }
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstatat.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstatat.c
index 8df65ebea084..19446a6cd917 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstatat.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/fxstatat.c
@@ -36,15 +36,15 @@  __fxstatat (int vers, int fd, const char *file, struct stat *buf, int flag)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-# ifdef __NR_fstatat64
-      int rc = INLINE_SYSCALL (fstatat64, 4, fd, file, buf, flag);
-# else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, fd, file,
                                AT_NO_AUTOMOUNT | flag,
                                STATX_BASIC_STATS, &tmp);
       if (rc == 0)
         __cp_stat64_statx ((struct stat64 *)buf, &tmp);
+# else
+      int rc = INLINE_SYSCALL (fstatat64, 4, fd, file, buf, flag);
 # endif
       return rc ?: stat_overflow (buf);
     }
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat.c
index bd02631d68cd..63fba924d14e 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat.c
@@ -35,17 +35,17 @@  __lxstat (int vers, const char *name, struct stat *buf)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-#ifdef __NR_fstatat64
-      int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf,
-                               AT_SYMLINK_NOFOLLOW);
-#else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, AT_FDCWD, name,
                                AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW,
                                STATX_BASIC_STATS, &tmp);
       if (rc == 0)
         __cp_stat64_statx ((struct stat64 *)buf, &tmp);
-#endif
+# else
+      int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf,
+                               AT_SYMLINK_NOFOLLOW);
+# endif
       return rc ?: stat_overflow (buf);
     }
   errno = EINVAL;
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat64.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat64.c
index 3eab0a184751..5f8361361e04 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat64.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/lxstat64.c
@@ -40,10 +40,7 @@  __lxstat64 (int vers, const char *name, struct stat64 *buf)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-#ifdef __NR_fstatat64
-      return INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf,
-                             AT_SYMLINK_NOFOLLOW);
-#else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, AT_FDCWD, name,
                                AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW,
@@ -51,6 +48,9 @@  __lxstat64 (int vers, const char *name, struct stat64 *buf)
       if (rc == 0)
         __cp_stat64_statx (buf, &tmp);
       return rc;
+#else
+      return INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf,
+                             AT_SYMLINK_NOFOLLOW);
 #endif
     }
 
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat.c
index a4a921c1ea6c..43b3e0b7b638 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat.c
@@ -35,14 +35,14 @@  __xstat (int vers, const char *name, struct stat *buf)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-# ifdef __NR_fstatat64
-      int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf, 0);
-# else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, AT_FDCWD, name, AT_NO_AUTOMOUNT,
                                STATX_BASIC_STATS, &tmp);
       if (rc == 0)
         __cp_stat64_statx ((struct stat64 *)buf, &tmp);
+# else
+      int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf, 0);
 # endif
       return rc ?: stat_overflow (buf);
     }
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat64.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat64.c
index 0848b5a4b4b1..e999bd2ffad5 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat64.c
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/xstat64.c
@@ -40,15 +40,15 @@  __xstat64 (int vers, const char *name, struct stat64 *buf)
 {
   if (vers == _STAT_VER_KERNEL)
     {
-#ifdef __NR_fstatat64
-      return INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf, 0);
-#else
+# if defined(STAT64_IS_NOT_KERNEL_STAT64) || !defined(__NR_fstat64)
       struct statx tmp;
       int rc = INLINE_SYSCALL (statx, 5, AT_FDCWD, name, AT_NO_AUTOMOUNT,
                                STATX_BASIC_STATS, &tmp);
       if (rc == 0)
         __cp_stat64_statx (buf, &tmp);
       return rc;
+#else
+      return INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, buf, 0);
 #endif
     }
   errno = EINVAL;
diff --git a/sysdeps/unix/sysv/linux/statx_cp.c b/sysdeps/unix/sysv/linux/statx_cp.c
index cc6e17929ea0..faba286d963c 100644
--- a/sysdeps/unix/sysv/linux/statx_cp.c
+++ b/sysdeps/unix/sysv/linux/statx_cp.c
@@ -22,7 +22,7 @@ 
 
 #include <statx_cp.h>
 
-#if !defined(__NR_fstat64) || !defined(__NR_fstatat64)
+#if !defined(__NR_fstat64) || !defined(__NR_fstatat64) || defined(STAT64_IS_NOT_KERNEL_STAT64)
 void
 __cp_stat64_statx (struct stat64 *to, struct statx *from)
 {