On Tue, 3 Jun 2025 at 14:46, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> For some 32-bit targets Glibc supports changing the size of time_t to be
> 64 bits by defining _TIME_BITS=64. That causes an ABI change which
> would affect std::chrono::system_clock::to_time_t. Because to_time_t is
> not a function template, its mangled name does not depend on the return
> type, so it has the same mangled name whether it returns a 32-bit time_t
> or a 64-bit time_t. On targets where the size of time_t can be selected
> at preprocessing time, that can cause ODR violations, e.g. the linker
> selects a definition of to_time_t that returns a 32-bit value but a
> caller expects 64-bit and so reads 32 bits of garbage from the stack.
>
> This commit adds always_inline to to_time_t when time_t has been changed
> from a 32-bit type to a 64-bit type by defining _TIME_BITS=64. This
> ensures that callers expecting a 64-bit time_t can't link to a
> definition returning a 32-bit time_t.
>
> We use the internal Glibc macro __USE_TIME64_REDIRECTS to detect the
> case where time_t defaults to 32-bit for the target but has been
> explicitly changed to 64-bit by the user.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/99832
> * include/bits/chrono.h (system_clock::to_time_t): Add always
> inline_attribute for 64-bit time_t on 32-bit target.
> * testsuite/20_util/system_clock/time64.cc: New test.
> ---
>
> Tested x86_64-linux (-m64 and -m32, with recent glibc).
>
> libstdc++-v3/include/bits/chrono.h | 3 +++
> .../testsuite/20_util/system_clock/time64.cc | 21 +++++++++++++++++++
> 2 files changed, 24 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/20_util/system_clock/time64.cc
>
> diff --git a/libstdc++-v3/include/bits/chrono.h
> b/libstdc++-v3/include/bits/chrono.h
> index fad216203d2f..5c6ee759b381 100644
> --- a/libstdc++-v3/include/bits/chrono.h
> +++ b/libstdc++-v3/include/bits/chrono.h
> @@ -1244,6 +1244,9 @@ _GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
> now() noexcept;
>
> // Map to C API
> +#ifdef __USE_TIME64_REDIRECTS
Florian suggested not relying on this internal glibc macro (which was
not present in the first versions of glibc to support _TIME_BITS=64).
We can just make the always_inline attribute unconditional, which does
no harm. It's a tiny function that just extracts an integer from the
time and does an integer division to convert nanoseconds to seconds,
so always_inline is appropriate. And this way no targets will get a
dependency on any to_time_t symbol, with any mangling or any return
type.
Existing objects which were already compiled before the attribute was
added will still work, because the function is inline so those objects
will already have a COMDAT definition of the symbol (or will have
inlined it anyway).
The only case that can't work is linking together existing objects
which were compiled with and without -D_TIME_BITS=64 before libstdc++
knew how to support that macro, but we can't fix those, the objects
compiled with -D_TIME_BITS=64 might need to be recompiled.
> + [[__gnu__::__always_inline__]]
> +#endif
> static std::time_t
> to_time_t(const time_point& __t) noexcept
> {
> diff --git a/libstdc++-v3/testsuite/20_util/system_clock/time64.cc
> b/libstdc++-v3/testsuite/20_util/system_clock/time64.cc
> new file mode 100644
> index 000000000000..3cbf80e0f06e
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/system_clock/time64.cc
> @@ -0,0 +1,21 @@
> +// { dg-options "-D_FILE_OFFSET_BITS=64 -D_TIME_BITS=64 -O0 -g0" }
> +// { dg-do compile { target *-*-linux-gnu } }
> +// { dg-require-effective-target c++20 }
> +// { dg-require-effective-target ilp32 }
> +// { dg-final { scan-assembler-not "system_clock9to_time_t" } }
> +
> +#include <chrono>
> +
> +template<typename T>
> +std::time_t
> +test()
> +{
> + using std::chrono::system_clock;
> +
> + if constexpr (sizeof(T) == 8)
> + return system_clock::to_time_t(system_clock::now());
> + else // _TIME_BITS=64 had no effect, maybe an old Glibc
> + return 0;
> +}
> +
> +auto t = test<std::time_t>();
> --
> 2.49.0
>
