From: YunQiang Su <syq@debian.org>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org, Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [PATCH] linux: Alias non-LFS to LFS stat function for mips and sparc (BZ #29730)
Date: Tue, 1 Nov 2022 01:07:35 +0800 [thread overview]
Message-ID: <CAKcpw6Xz_A03=dS4J3h=SBOhmVV7==vJ=MNDE7-JdGN2HP-oTA@mail.gmail.com> (raw)
In-Reply-To: <20221031160032.1813610-1-adhemerval.zanella@linaro.org>
Adhemerval Zanella <adhemerval.zanella@linaro.org> 于2022年11月1日周二 00:00写道：
>
> The XSTAT_IS_XSTAT64 is not a fully correct macro to define whether
> stat non-LFS and LFS symbols should alias. It is used on both old
> compat code (__xstat*) and on kernel syscall wrapper to define
> whether the kernel struct needs to be adjusted to glibc exported
> layout.
>
> However for misp64 and sparc64, which have a different struct stat
> layout between glibc and kABI; still have the same layout for non-LFS
> and LFS struct stat. For both cases there is no need to provide a
> different non-LFS / non 64 bit time_t stat symbol, the default
> LFS is suffice.
>
> This patch adds a new STAT_IS_STAT64 to define whether LFS stat
> should alias to old non-LFS one (similar to how __OFF_T_MATCHES_OFF64_T
> is used for pread and similar symbols). Only mips64 and sparc64
> have a different value than XSTAT_IS_XSTAT64, so this change should
> be a nop for other ABIs. This also fixes BZ#29730, where there is
> no need for the time_t check on non-LFS to LFS transition.
>
> Also, this is a small optimization for mips64 and sparc64 where
> it avoid the need to copy/paste struct stat over non-LFS to LFS
> and also a small code size redution.
>
> Checked with glob and io tests on mips64.
> ---
> sysdeps/unix/sysv/linux/alpha/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/arm/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/csky/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/fstat.c | 2 +-
> sysdeps/unix/sysv/linux/fstat64.c | 2 +-
> sysdeps/unix/sysv/linux/fstatat.c | 2 +-
> sysdeps/unix/sysv/linux/fstatat64.c | 2 +-
> sysdeps/unix/sysv/linux/glob.c | 2 +-
> sysdeps/unix/sysv/linux/glob64.c | 4 ++--
> sysdeps/unix/sysv/linux/globfree.c | 2 +-
> sysdeps/unix/sysv/linux/globfree64.c | 2 +-
> sysdeps/unix/sysv/linux/hppa/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/i386/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/lstat.c | 2 +-
> sysdeps/unix/sysv/linux/lstat64.c | 2 +-
> sysdeps/unix/sysv/linux/m68k/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/microblaze/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/mips/kernel_stat.h | 2 ++
> sysdeps/unix/sysv/linux/nios2/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/powerpc/powerpc32/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/s390/s390-32/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/sh/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/sparc/sparc32/kernel_stat.h | 1 +
> sysdeps/unix/sysv/linux/sparc/sparc64/kernel_stat.h | 2 ++
> sysdeps/unix/sysv/linux/stat.c | 2 +-
> sysdeps/unix/sysv/linux/stat64.c | 2 +-
> 27 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> index a292920969..9c31a163f0 100644
> --- a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> @@ -88,4 +88,5 @@ struct glibc21_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 1
> +#define STAT_IS_STAT64 1
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/arm/kernel_stat.h b/sysdeps/unix/sysv/linux/arm/kernel_stat.h
> index b1bc1459f0..f199bea565 100644
> --- a/sysdeps/unix/sysv/linux/arm/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/arm/kernel_stat.h
> @@ -37,4 +37,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/csky/kernel_stat.h b/sysdeps/unix/sysv/linux/csky/kernel_stat.h
> index 7970c508ef..5eebe23139 100644
> --- a/sysdeps/unix/sysv/linux/csky/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/csky/kernel_stat.h
> @@ -18,4 +18,5 @@
>
> #define STAT_IS_KERNEL_STAT 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/fstat.c b/sysdeps/unix/sysv/linux/fstat.c
> index ff3d206fb9..8767f692a0 100644
> --- a/sysdeps/unix/sysv/linux/fstat.c
> +++ b/sysdeps/unix/sysv/linux/fstat.c
> @@ -21,7 +21,7 @@
> #include <fcntl.h>
> #include <errno.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
> int
> __fstat (int fd, struct stat *buf)
> {
> diff --git a/sysdeps/unix/sysv/linux/fstat64.c b/sysdeps/unix/sysv/linux/fstat64.c
> index 581746ce63..2cca4b194b 100644
> --- a/sysdeps/unix/sysv/linux/fstat64.c
> +++ b/sysdeps/unix/sysv/linux/fstat64.c
> @@ -58,7 +58,7 @@ __fstat64 (int fd, struct stat64 *buf)
> hidden_def (__fstat64)
> weak_alias (__fstat64, fstat64)
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> strong_alias (__fstat64, __fstat)
> weak_alias (__fstat64, fstat)
> #endif
> diff --git a/sysdeps/unix/sysv/linux/fstatat.c b/sysdeps/unix/sysv/linux/fstatat.c
> index 055fb4762e..58f3139532 100644
> --- a/sysdeps/unix/sysv/linux/fstatat.c
> +++ b/sysdeps/unix/sysv/linux/fstatat.c
> @@ -20,7 +20,7 @@
> #include <kernel_stat.h>
> #include <sysdep.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
> # include <kstat_cp.h>
>
> int
> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index 8b1a1a290d..f79e3b40b5 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -186,7 +186,7 @@ __fstatat64 (int fd, const char *file, struct stat64 *buf, int flags)
> hidden_def (__fstatat64)
> weak_alias (__fstatat64, fstatat64)
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> strong_alias (__fstatat64, __fstatat)
> weak_alias (__fstatat64, fstatat)
> strong_alias (__fstatat64, __GI___fstatat);
> diff --git a/sysdeps/unix/sysv/linux/glob.c b/sysdeps/unix/sysv/linux/glob.c
> index 7fd526a48c..9dffbf0c54 100644
> --- a/sysdeps/unix/sysv/linux/glob.c
> +++ b/sysdeps/unix/sysv/linux/glob.c
> @@ -31,7 +31,7 @@
> #undef glob64
> #undef __glob64
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> strong_alias (__glob, __glob64)
> versioned_symbol (libc, __glob64, glob64, GLIBC_2_27);
> #endif
> diff --git a/sysdeps/unix/sysv/linux/glob64.c b/sysdeps/unix/sysv/linux/glob64.c
> index 8845921849..36dd75cd45 100644
> --- a/sysdeps/unix/sysv/linux/glob64.c
> +++ b/sysdeps/unix/sysv/linux/glob64.c
> @@ -19,7 +19,7 @@
> #include <sys/stat.h>
> #include <kernel_stat.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
> # include <glob.h>
> # include <dirent.h>
> # include <sys/stat.h>
> @@ -50,4 +50,4 @@ libc_hidden_def (__glob64)
> versioned_symbol (libc, __glob64, glob64, GLIBC_2_27);
> libc_hidden_ver (__glob64, glob64)
> # endif
> -#endif /* XSTAT_IS_XSTAT64 */
> +#endif /* STAT_IS_STAT64 */
> diff --git a/sysdeps/unix/sysv/linux/globfree.c b/sysdeps/unix/sysv/linux/globfree.c
> index 0deafd500b..d85101c7bf 100644
> --- a/sysdeps/unix/sysv/linux/globfree.c
> +++ b/sysdeps/unix/sysv/linux/globfree.c
> @@ -24,7 +24,7 @@
> #include <posix/globfree.c>
> #undef globfree64
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> weak_alias (globfree, globfree64)
> libc_hidden_ver (globfree, globfree64)
> #endif
> diff --git a/sysdeps/unix/sysv/linux/globfree64.c b/sysdeps/unix/sysv/linux/globfree64.c
> index 3914307aa3..a172febbff 100644
> --- a/sysdeps/unix/sysv/linux/globfree64.c
> +++ b/sysdeps/unix/sysv/linux/globfree64.c
> @@ -20,7 +20,7 @@
> #include <sys/stat.h>
> #include <kernel_stat.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
>
> # include <glob.h>
>
> diff --git a/sysdeps/unix/sysv/linux/hppa/kernel_stat.h b/sysdeps/unix/sysv/linux/hppa/kernel_stat.h
> index e8ad135e70..97f7d04d0b 100644
> --- a/sysdeps/unix/sysv/linux/hppa/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/hppa/kernel_stat.h
> @@ -33,4 +33,5 @@ struct kernel_stat {
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/i386/kernel_stat.h b/sysdeps/unix/sysv/linux/i386/kernel_stat.h
> index b1bc1459f0..f199bea565 100644
> --- a/sysdeps/unix/sysv/linux/i386/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/i386/kernel_stat.h
> @@ -37,4 +37,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h
> index 6483c76dd9..4236391164 100644
> --- a/sysdeps/unix/sysv/linux/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/kernel_stat.h
> @@ -18,5 +18,6 @@
>
> /* The default Linux ABI assumes only LFS support. */
> #define XSTAT_IS_XSTAT64 1
> +#define STAT_IS_STAT64 1
> #define STATFS_IS_STATFS64 __STATFS_MATCHES_STATFS64
> #define STAT_IS_KERNEL_STAT 1
> diff --git a/sysdeps/unix/sysv/linux/lstat.c b/sysdeps/unix/sysv/linux/lstat.c
> index b8920d4e53..f19ff582fe 100644
> --- a/sysdeps/unix/sysv/linux/lstat.c
> +++ b/sysdeps/unix/sysv/linux/lstat.c
> @@ -20,7 +20,7 @@
> #include <fcntl.h>
> #include <kernel_stat.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
> int
> __lstat (const char *file, struct stat *buf)
> {
> diff --git a/sysdeps/unix/sysv/linux/lstat64.c b/sysdeps/unix/sysv/linux/lstat64.c
> index 5a3dddf4ce..fca9334e50 100644
> --- a/sysdeps/unix/sysv/linux/lstat64.c
> +++ b/sysdeps/unix/sysv/linux/lstat64.c
> @@ -45,7 +45,7 @@ weak_alias (__lstat64, lstat64)
> #undef __lstat
> #undef lstat
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> strong_alias (__lstat64, __lstat)
> weak_alias (__lstat64, lstat)
> #endif
> diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_stat.h b/sysdeps/unix/sysv/linux/m68k/kernel_stat.h
> index b1bc1459f0..f199bea565 100644
> --- a/sysdeps/unix/sysv/linux/m68k/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/m68k/kernel_stat.h
> @@ -37,4 +37,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/microblaze/kernel_stat.h b/sysdeps/unix/sysv/linux/microblaze/kernel_stat.h
> index d8b3df26b4..735016bbdd 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/microblaze/kernel_stat.h
> @@ -51,4 +51,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/mips/kernel_stat.h b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> index 19524f7ea4..494727de36 100644
> --- a/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> @@ -63,8 +63,10 @@ struct kernel_stat
> #define STAT64_IS_KERNEL_STAT64 0
> #define XSTAT_IS_XSTAT64 0
> #if _MIPS_SIM == _ABI64
> +# define STAT_IS_STAT64 1
No. It is not correct.
The reason of that we cannot use STAT_IS_STAT64 for MIPS N64 is that:
struct stat
struct stat64
are different in their internal padding.
So to keep binary compatible, we cannot use STAT_IS_STAT64.
Not due to the quirk struct kernel_stat.
In fact we can workaround this problem with statx.
> # define STATFS_IS_STATFS64 1
> #else
> +# define STAT_IS_STAT64 0
> # define STATFS_IS_STATFS64 0
> #endif
> /* MIPS64 has unsigned 32 bit timestamps fields, so use statx as well. */
> diff --git a/sysdeps/unix/sysv/linux/nios2/kernel_stat.h b/sysdeps/unix/sysv/linux/nios2/kernel_stat.h
> index 4bd95750a0..a0fa6af2e3 100644
> --- a/sysdeps/unix/sysv/linux/nios2/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/nios2/kernel_stat.h
> @@ -18,4 +18,5 @@
>
> #define STAT_IS_KERNEL_STAT 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 1
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/kernel_stat.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/kernel_stat.h
> index 166aa44372..06e9a24c36 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/kernel_stat.h
> @@ -50,4 +50,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/kernel_stat.h b/sysdeps/unix/sysv/linux/s390/s390-32/kernel_stat.h
> index b1bc1459f0..f199bea565 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/kernel_stat.h
> @@ -37,4 +37,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/sh/kernel_stat.h b/sysdeps/unix/sysv/linux/sh/kernel_stat.h
> index b1bc1459f0..f199bea565 100644
> --- a/sysdeps/unix/sysv/linux/sh/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/sh/kernel_stat.h
> @@ -37,4 +37,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/kernel_stat.h b/sysdeps/unix/sysv/linux/sparc/sparc32/kernel_stat.h
> index 4a2df42d37..0cf73b664e 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/kernel_stat.h
> @@ -34,4 +34,5 @@ struct kernel_stat
> #define STAT_IS_KERNEL_STAT 0
> #define STAT64_IS_KERNEL_STAT64 1
> #define XSTAT_IS_XSTAT64 0
> +#define STAT_IS_STAT64 0
> #define STATFS_IS_STATFS64 0
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/kernel_stat.h b/sysdeps/unix/sysv/linux/sparc/sparc64/kernel_stat.h
> index 29d18908da..6a55faa66e 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/kernel_stat.h
> @@ -51,8 +51,10 @@ struct kernel_stat64
> #define STAT64_IS_KERNEL_STAT64 0
> #define XSTAT_IS_XSTAT64 1
> #ifdef __arch64__
> +# define STAT_IS_STAT64 1
> # define STATFS_IS_STATFS64 1
> #else
> +# define STAT_IS_STAT64 0
> # define STATFS_IS_STATFS64 0
> #endif
> #endif /* _KERNEL_STAT_H */
> diff --git a/sysdeps/unix/sysv/linux/stat.c b/sysdeps/unix/sysv/linux/stat.c
> index 32cc0eab62..7709f3c989 100644
> --- a/sysdeps/unix/sysv/linux/stat.c
> +++ b/sysdeps/unix/sysv/linux/stat.c
> @@ -20,7 +20,7 @@
> #include <fcntl.h>
> #include <kernel_stat.h>
>
> -#if !XSTAT_IS_XSTAT64
> +#if !STAT_IS_STAT64
> int
> __stat (const char *file, struct stat *buf)
> {
> diff --git a/sysdeps/unix/sysv/linux/stat64.c b/sysdeps/unix/sysv/linux/stat64.c
> index a68bd349ce..7941471396 100644
> --- a/sysdeps/unix/sysv/linux/stat64.c
> +++ b/sysdeps/unix/sysv/linux/stat64.c
> @@ -46,7 +46,7 @@ __stat64 (const char *file, struct stat64 *buf)
> hidden_def (__stat64)
> weak_alias (__stat64, stat64)
>
> -#if XSTAT_IS_XSTAT64
> +#if STAT_IS_STAT64
> strong_alias (__stat64, __stat)
> weak_alias (__stat64, stat)
> #endif
> --
> 2.34.1
>
next prev parent reply other threads:[~2022-10-31 17:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-31 16:00 Adhemerval Zanella
2022-10-31 16:58 ` Joseph Myers
2022-10-31 17:21 ` Adhemerval Zanella Netto
2022-10-31 17:07 ` YunQiang Su [this message]
2022-10-31 17:22 ` Adhemerval Zanella Netto
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKcpw6Xz_A03=dS4J3h=SBOhmVV7==vJ=MNDE7-JdGN2HP-oTA@mail.gmail.com' \
--to=syq@debian.org \
--cc=adhemerval.zanella@linaro.org \
--cc=aurelien@aurel32.net \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).