Re: [PATCH 0/2] linux: Keep time64 stat layout independent of feature macros

On 05/08/26 12:23, Adhemerval Zanella Netto wrote:
> 
> 
> On 05/08/26 05:03, Matthias Goergens wrote:
>> Bug 32119 reports that feature-test macros change the public time64
>> struct stat layout on 32-bit targets. The series separates two causes.
>>
>> Patch 1 includes the endian definitions used by the shared helper. Without
>> them, strict pre-POSIX.1-2008 modes select big-endian timestamp ordering on
>> little-endian targets. This is an unambiguous layout bug.
>>
>> Patch 2 removes two trailing reserved words which the helper exposes only in
>> those strict modes. History shows that they were imported accidentally when
>> the public and internal time64 declarations were consolidated. The stat
>> implementation does not write them, and the default public and internal
>> layouts omit them.
>>
>> Patch 2 nevertheless changes sizeof(struct stat) for 32-bit objects already
>> compiled with both 64-bit time and strict pre-POSIX.1-2008 feature macros.
>> Please advise whether preserving that niche accidental layout is required by
>> glibc ABI policy. Patch 1 is independent if the size correction is unsuitable.
> 
> I still strictly an ABI break, but I also think the blast radius is quite minimal.
> The fields were unused by glibc, so it would be mostly between TU built with
> different flags.
> 
> And I do not think it would worth adding compat symbols, but I also think it
> would be good to backport this fix to all affected branches.  
> 
>>
>> An i386 build and the io tests pass. The regression compares timestamp
>> offsets and sizes across translation units built in default and POSIX.1-2001
>> modes. Compile-time layout models also pass for 32-bit Arm, PowerPC, and MIPS;
>> these models are supplementary and are not full cross builds.
> 
> I think it would be better to move this test to script that uses glibcextracy.py,
> so these tests can be checked in cross-compiling mode.  Something like:
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 14a56d5cc3f..a5811977d4a 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -413,6 +413,17 @@ $(objpfx)tst-sched-consts.out: ../sysdeps/unix/sysv/linux/tst-sched-consts.py
>  	  < /dev/null > $@ 2>&1; $(evaluate-test)
>  $(objpfx)tst-sched-consts.out: $(sysdeps-linux-python-deps)
>  
> +tests-special += \
> +  $(objpfx)tst-stat-layout-time64.out \
> +  # tests-special
> +$(objpfx)tst-stat-layout-time64.out: \
> +  ../sysdeps/unix/sysv/linux/tst-stat-layout-time64.py
> +	$(sysdeps-linux-python) \
> +	  ../sysdeps/unix/sysv/linux/tst-stat-layout-time64.py \
> +	    $(sysdeps-linux-python-cc) \
> +	  < /dev/null > $@ 2>&1; $(evaluate-test)
> +$(objpfx)tst-stat-layout-time64.out: $(sysdeps-linux-python-deps)
> +
>  tst-rseq-disable-TUNABLES += glibc.pthread.rseq=0
>  tst-rseq-disable-static-TUNABLES += glibc.pthread.rseq=0
>  
> diff --git a/sysdeps/unix/sysv/linux/tst-stat-layout-time64.py b/sysdeps/unix/sysv/linux/tst-stat-layout-time64.py
> new file mode 100644
> index 00000000000..2c6dd66a491
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-stat-layout-time64.py
> @@ -0,0 +1,94 @@
> +#!/usr/bin/python3
> +# Check that feature-test macros do not change the time64 stat layout.
> +# Copyright (C) 2026 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +import argparse
> +import sys
> +
> +import glibcextract
> +
> +MODES = {
> +    'POSIX.1-1996': '#define _POSIX_C_SOURCE 199506L',
> +    'POSIX.1-2001': '#define _POSIX_C_SOURCE 200112L',
> +    'POSIX.1-2008': '#define _POSIX_C_SOURCE 200809L',
> +    'XPG6': '#define _XOPEN_SOURCE 600',
> +}
> +
> +
> +def compute_stat_layout(cc, mode_define):
> +    # The nanosecond members are named differently in the two helper
> +    # branches: st_Xtim.tv_nsec with __USE_XOPEN2K8, st_Xtimensec
> +    #otherwise.
> +    sym_data = [
> +        '#undef _GNU_SOURCE',
> +        mode_define,
> +        '#define _TIME_BITS 64',
> +        '#define _FILE_OFFSET_BITS 64',
> +        '#include <stddef.h>',
> +        '#include <sys/stat.h>',
> +        'START',
> +        ('sizeof_stat', 'sizeof (struct stat)'),
> +        ('st_dev', 'offsetof (struct stat, st_dev)'),
> +        ('st_ino', 'offsetof (struct stat, st_ino)'),
> +        ('st_mode', 'offsetof (struct stat, st_mode)'),
> +        ('st_nlink', 'offsetof (struct stat, st_nlink)'),
> +        ('st_uid', 'offsetof (struct stat, st_uid)'),
> +        ('st_gid', 'offsetof (struct stat, st_gid)'),
> +        ('st_rdev', 'offsetof (struct stat, st_rdev)'),
> +        ('st_size', 'offsetof (struct stat, st_size)'),
> +        ('st_blksize', 'offsetof (struct stat, st_blksize)'),
> +        ('st_blocks', 'offsetof (struct stat, st_blocks)'),
> +        ('st_atime', 'offsetof (struct stat, st_atime)'),
> +        ('st_mtime', 'offsetof (struct stat, st_mtime)'),
> +        ('st_ctime', 'offsetof (struct stat, st_ctime)'),
> +        '#ifdef __USE_XOPEN2K8',
> +        ('st_atimensec', 'offsetof (struct stat, st_atim.tv_nsec)'),
> +        ('st_mtimensec', 'offsetof (struct stat, st_mtim.tv_nsec)'),
> +        ('st_ctimensec', 'offsetof (struct stat, st_ctim.tv_nsec)'),
> +        '#else',
> +        ('st_atimensec', 'offsetof (struct stat, st_atimensec)'),
> +        ('st_mtimensec', 'offsetof (struct stat, st_mtimensec)'),
> +        ('st_ctimensec', 'offsetof (struct stat, st_ctimensec)'),
> +        '#endif',
> +    ]
> +    return glibcextract.compute_c_consts(sym_data, cc)
> +
> +
> +def main():
> +    parser = argparse.ArgumentParser(
> +        description='Check that feature-test macros do not change '
> +        'the time64 stat layout.')
> +    parser.add_argument('--cc', metavar='CC',
> +                        help='C compiler (including options) to use')
> +    args = parser.parse_args()
> +    default_layout = compute_stat_layout(args.cc, '#define _GNU_SOURCE 1')
> +    status = 0
> +    for mode, mode_define in sorted(MODES.items()):
> +        mode_layout = compute_stat_layout(args.cc, mode_define)
> +        for name, value in default_layout.items():
> +            if mode_layout[name] != value:
> +                print('FAIL: %s: %s is %s, %s in default mode'
> +                      % (mode, name, mode_layout[name], value))
> +                status = 1
> +    if status == 0:
> +        print('PASS: struct stat layout is feature-test-macro invariant')
> +    sys.exit(status)
> +
> +
> +if __name__ == '__main__':
> +    main()
> 
> 
> And this approach uncovered a similar issue on arc and ork1, which are both
> 32-bit with 64-bit time_t by default.  Different than riscv32, another 32-bit
> with 64-bit time_t, alignof(int64) == 4 and thus 'struct stat' size and
> internal layout (sigh...) differ.
> 
> The fix would require to change de generic implementation
> sysdeps/unix/sysv/linux/bits/struct_stat.h:
> 
> iff --git a/sysdeps/unix/sysv/linux/bits/struct_stat.h b/sysdeps/unix/sysv/linux/bits/struct_stat.h
> index e912c3f6ba5..cdbe1f8f762 100644
> --- a/sysdeps/unix/sysv/linux/bits/struct_stat.h
> +++ b/sysdeps/unix/sysv/linux/bits/struct_stat.h
> @@ -41,6 +41,22 @@
>    int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name
>  #endif
> 
> +/* The pre-POSIX.1-2008 timestamp fields must match the layout of the
> +   'struct timespec' members used in the POSIX.1-2008 case, including the
> +   padding required when the seconds field is wider than the word size.  */
> +#if __WORDSIZE == 64 \
> +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
> +  || (__TIMESIZE == 32 && !defined __USE_TIME64_REDIRECTS)
> +# define __fieldts(name) \
> +  __time_t name; unsigned long int name ## nsec
> +#elif __BYTE_ORDER == __BIG_ENDIAN
> +# define __fieldts(name) \
> +  __time_t name; int: 32; unsigned long int name ## nsec
> +#else
> +# define __fieldts(name) \
> +  __time_t name; unsigned long int name ## nsec; int: 32
> +#endif
> +
>  struct stat
>    {
>      __dev_t st_dev;            /* Device.  */
> @@ -69,12 +85,9 @@ struct stat
>  # define st_mtime st_mtim.tv_sec
>  # define st_ctime st_ctim.tv_sec
>  #else
> -    __time_t st_atime;                 /* Time of last access.  */
> -    unsigned long int st_atimensec;    /* Nscecs of last access.  */
> -    __time_t st_mtime;                 /* Time of last modification.  */
> -    unsigned long int st_mtimensec;    /* Nsecs of last modification.  */
> -    __time_t st_ctime;                 /* Time of last status change.  */
> -    unsigned long int st_ctimensec;    /* Nsecs of last status change.  */
> +    __fieldts (st_atime);              /* Time of last access.  */
> +    __fieldts (st_mtime);              /* Time of last modification.  */
> +    __fieldts (st_ctime);              /* Time of last status change.  */
>  #endif
>      int __glibc_reserved[2];
>    };
> @@ -107,17 +120,16 @@ struct stat64
>      struct timespec st_mtim;           /* Time of last modification.  */
>      struct timespec st_ctim;           /* Time of last status change.  */
>  #else
> -    __time_t st_atime;                 /* Time of last access.  */
> -    unsigned long int st_atimensec;    /* Nscecs of last access.  */
> -    __time_t st_mtime;                 /* Time of last modification.  */
> -    unsigned long int st_mtimensec;    /* Nsecs of last modification.  */
> -    __time_t st_ctime;                 /* Time of last status change.  */
> -    unsigned long int st_ctimensec;    /* Nsecs of last status change.  */
> +    __fieldts (st_atime);              /* Time of last access.  */
> +    __fieldts (st_mtime);              /* Time of last modification.  */
> +    __fieldts (st_ctime);              /* Time of last status change.  */
>  #endif
>      int __glibc_reserved[2];
>    };
>  #endif
> 
> +#undef __fieldts
> +
>  /* Tell code we have these members.  */
>  #define        _STATBUF_ST_BLKSIZE
>  #define _STATBUF_ST_RDEV
> 
> I am not sure if this would require a different bug, but we will need to fix
> it along with BZ#34466.
I squashed your proposed change with the extra one the generic and the python
tests [1], this should be complete change for all ABIs. I will test on some
ABIs, but it should only really affect 32 bit ABIs.

Ideally this should be backported up to 2.34.

[1] https://sourceware.org/git/?p=glibc.git;a=commit;h=ecf95727a78731465b33290bc7374114ab38b63e