On Thu, Mar 20, 2025 at 10:39 AM Jonathan Wakely <jwak...@redhat.com> wrote:
> The previous commit fixed most cases of %c formatting, but it
> incorrectly prints times using the system's local time zone. This only
> matters if the locale's %c format includes %Z, but some do.
>
> To print a correct value for %Z we can set tm.tm_zone to either "UTC" or
> the abbreviation passed to the formatter in the local-time-format-t
> structure.
>
> For local times with no info and for systems that don't support tm_zone
> (which is new in POSIX.1-2024) we just set tm_isdst = -1 so that no zone
> name is printed.
>
> In theory, a locale's %c format could use %z which should print a +hhmm
> offset from UTC. I'm unsure how to control that though. The new
> tm_gmtoff field in combination with tm_isdst != -1 seems like it should
> work, but using that without also setting tm_zone causes the system zone
> to be used for %Z again. That means local_time_format(lt, nullptr, &off)
> might work for a locale that uses %z but prints the wrong thing for %Z.
> This commit doesn't set tm_gmtoff even if _M_offset_sec is provided for
> a local-time-format-t value.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/117214
> * configure.ac: Use AC_STRUCT_TIMEZONE.
> * config.h.in: Regenerate.
> * configure: Regenerate.
> * include/bits/chrono_io.h (__formatter_chrono::_M_c): Set
> tm_isdst and tm_zone.
> * testsuite/std/time/format/pr117214.cc: Check %c formatting of
> zoned_time and local time.
> ---
>
> Tested x86_64-linux.
>
The c++ code parts LGTM, however the changes in config.h.in and
configure went over my head: they seem to check for new members,
but I cannot confirm if they are correct and according to best practices.
>
> libstdc++-v3/config.h.in | 18 ++
> libstdc++-v3/configure | 179 +++++++++++++++++-
> libstdc++-v3/configure.ac | 2 +
> libstdc++-v3/include/bits/chrono_io.h | 32 +++-
> .../testsuite/std/time/format/pr117214.cc | 65 +++++++
> 5 files changed, 287 insertions(+), 9 deletions(-)
>
> diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
> index 91e920044ee..be151f43dd6 100644
> --- a/libstdc++-v3/config.h.in
> +++ b/libstdc++-v3/config.h.in
> @@ -74,6 +74,10 @@
> don't. */
> #undef HAVE_DECL_STRNLEN
>
> +/* Define to 1 if you have the declaration of `tzname', and to 0 if you
> don't.
> + */
> +#undef HAVE_DECL_TZNAME
> +
> /* Define to 1 if you have the <dirent.h> header file. */
> #undef HAVE_DIRENT_H
>
> @@ -408,6 +412,9 @@
> /* Define to 1 if `d_type' is a member of `struct dirent'. */
> #undef HAVE_STRUCT_DIRENT_D_TYPE
>
> +/* Define to 1 if `tm_zone' is a member of `struct tm'. */
> +#undef HAVE_STRUCT_TM_TM_ZONE
> +
> /* Define if strxfrm_l is available in <string.h>. */
> #undef HAVE_STRXFRM_L
>
> @@ -499,9 +506,17 @@
> /* Define to 1 if the target supports thread-local storage. */
> #undef HAVE_TLS
>
> +/* Define to 1 if your `struct tm' has `tm_zone'. Deprecated, use
> + `HAVE_STRUCT_TM_TM_ZONE' instead. */
> +#undef HAVE_TM_ZONE
> +
> /* Define if truncate is available in <unistd.h>. */
> #undef HAVE_TRUNCATE
>
> +/* Define to 1 if you don't have `tm_zone' but do have the external array
> + `tzname'. */
> +#undef HAVE_TZNAME
> +
> /* Define to 1 if you have the <uchar.h> header file. */
> #undef HAVE_UCHAR_H
>
> @@ -590,6 +605,9 @@
> /* Define to 1 if you have the ANSI C header files. */
> #undef STDC_HEADERS
>
> +/* Define to 1 if your <sys/time.h> declares `struct tm'. */
> +#undef TM_IN_SYS_TIME
> +
> /* Version number of package */
> #undef VERSION
>
> diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
> index 78758285f21..67d2b8c7b72 100755
> --- a/libstdc++-v3/configure
> +++ b/libstdc++-v3/configure
> @@ -2731,6 +2731,63 @@ $as_echo "$ac_res" >&6; }
> eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
>
> } # ac_fn_c_check_decl
> +
> +# ac_fn_c_check_member LINENO AGGR MEMBER VAR INCLUDES
> +# ----------------------------------------------------
> +# Tries to find if the field MEMBER exists in type AGGR, after including
> +# INCLUDES, setting cache variable VAR accordingly.
> +ac_fn_c_check_member ()
> +{
> + as_lineno=${as_lineno-"$1"}
> as_lineno_stack=as_lineno_stack=$as_lineno_stack
> + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2.$3" >&5
> +$as_echo_n "checking for $2.$3... " >&6; }
> +if eval \${$4+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +$5
> +int
> +main ()
> +{
> +static $2 ac_aggr;
> +if (ac_aggr.$3)
> +return 0;
> + ;
> + return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> + eval "$4=yes"
> +else
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +$5
> +int
> +main ()
> +{
> +static $2 ac_aggr;
> +if (sizeof ac_aggr.$3)
> +return 0;
> + ;
> + return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> + eval "$4=yes"
> +else
> + eval "$4=no"
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +eval ac_res=\$$4
> + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res"
> >&5
> +$as_echo "$ac_res" >&6; }
> + eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
> +
> +} # ac_fn_c_check_member
> cat >config.log <<_ACEOF
> This file contains any messages produced by compilers while
> running configure, to aid debugging if configure makes a mistake.
> @@ -12280,7 +12337,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 12283 "configure"
> +#line 12340 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -12386,7 +12443,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 12389 "configure"
> +#line 12446 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -16182,7 +16239,7 @@ $as_echo "$glibcxx_cv_atomic_long_long" >&6; }
> # Fake what AC_TRY_COMPILE does.
>
> cat > conftest.$ac_ext << EOF
> -#line 16185 "configure"
> +#line 16242 "configure"
> int main()
> {
> typedef bool atomic_type;
> @@ -16217,7 +16274,7 @@ $as_echo "$glibcxx_cv_atomic_bool" >&6; }
> rm -f conftest*
>
> cat > conftest.$ac_ext << EOF
> -#line 16220 "configure"
> +#line 16277 "configure"
> int main()
> {
> typedef short atomic_type;
> @@ -16252,7 +16309,7 @@ $as_echo "$glibcxx_cv_atomic_short" >&6; }
> rm -f conftest*
>
> cat > conftest.$ac_ext << EOF
> -#line 16255 "configure"
> +#line 16312 "configure"
> int main()
> {
> // NB: _Atomic_word not necessarily int.
> @@ -16288,7 +16345,7 @@ $as_echo "$glibcxx_cv_atomic_int" >&6; }
> rm -f conftest*
>
> cat > conftest.$ac_ext << EOF
> -#line 16291 "configure"
> +#line 16348 "configure"
> int main()
> {
> typedef long long atomic_type;
> @@ -16444,7 +16501,7 @@ $as_echo "mutex" >&6; }
> # unnecessary for this test.
>
> cat > conftest.$ac_ext << EOF
> -#line 16447 "configure"
> +#line 16504 "configure"
> int main()
> {
> _Decimal32 d1;
> @@ -16486,7 +16543,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
> # unnecessary for this test.
>
> cat > conftest.$ac_ext << EOF
> -#line 16489 "configure"
> +#line 16546 "configure"
> template<typename T1, typename T2>
> struct same
> { typedef T2 type; };
> @@ -54640,6 +54697,112 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>
>
>
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether struct tm is in
> sys/time.h or time.h" >&5
> +$as_echo_n "checking whether struct tm is in sys/time.h or time.h... "
> >&6; }
> +if ${ac_cv_struct_tm+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +#include <sys/types.h>
> +#include <time.h>
> +
> +int
> +main ()
> +{
> +struct tm tm;
> + int *p = &tm.tm_sec;
> + return !p;
> + ;
> + return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> + ac_cv_struct_tm=time.h
> +else
> + ac_cv_struct_tm=sys/time.h
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_struct_tm" >&5
> +$as_echo "$ac_cv_struct_tm" >&6; }
> +if test $ac_cv_struct_tm = sys/time.h; then
> +
> +$as_echo "#define TM_IN_SYS_TIME 1" >>confdefs.h
> +
> +fi
> +
> +ac_fn_c_check_member "$LINENO" "struct tm" "tm_zone"
> "ac_cv_member_struct_tm_tm_zone" "#include <sys/types.h>
> +#include <$ac_cv_struct_tm>
> +
> +"
> +if test "x$ac_cv_member_struct_tm_tm_zone" = xyes; then :
> +
> +cat >>confdefs.h <<_ACEOF
> +#define HAVE_STRUCT_TM_TM_ZONE 1
> +_ACEOF
> +
> +
> +fi
> +
> +if test "$ac_cv_member_struct_tm_tm_zone" = yes; then
> +
> +$as_echo "#define HAVE_TM_ZONE 1" >>confdefs.h
> +
> +else
> + ac_fn_c_check_decl "$LINENO" "tzname" "ac_cv_have_decl_tzname"
> "#include <time.h>
> +"
> +if test "x$ac_cv_have_decl_tzname" = xyes; then :
> + ac_have_decl=1
> +else
> + ac_have_decl=0
> +fi
> +
> +cat >>confdefs.h <<_ACEOF
> +#define HAVE_DECL_TZNAME $ac_have_decl
> +_ACEOF
> +
> + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for tzname" >&5
> +$as_echo_n "checking for tzname... " >&6; }
> +if ${ac_cv_var_tzname+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + if test x$gcc_no_link = xyes; then
> + as_fn_error $? "Link tests are not allowed after GCC_NO_EXECUTABLES."
> "$LINENO" 5
> +fi
> +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +#include <time.h>
> +#if !HAVE_DECL_TZNAME
> +extern char *tzname[];
> +#endif
> +
> +int
> +main ()
> +{
> +return tzname[0][0];
> + ;
> + return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_link "$LINENO"; then :
> + ac_cv_var_tzname=yes
> +else
> + ac_cv_var_tzname=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> + conftest$ac_exeext conftest.$ac_ext
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_var_tzname" >&5
> +$as_echo "$ac_cv_var_tzname" >&6; }
> + if test $ac_cv_var_tzname = yes; then
> +
> +$as_echo "#define HAVE_TZNAME 1" >>confdefs.h
> +
> + fi
> +fi
> +
> +
> # Define documentation rules conditionally.
>
> # See if makeinfo has been installed and is modern enough
> diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> index a3b257fe652..fe0cdde1f7a 100644
> --- a/libstdc++-v3/configure.ac
> +++ b/libstdc++-v3/configure.ac
> @@ -584,6 +584,8 @@ GLIBCXX_CHECK_FILEBUF_NATIVE_HANDLES
> # For std::text_encoding
> GLIBCXX_CHECK_TEXT_ENCODING
>
> +AC_STRUCT_TIMEZONE
> +
> # Define documentation rules conditionally.
>
> # See if makeinfo has been installed and is modern enough
> diff --git a/libstdc++-v3/include/bits/chrono_io.h
> b/libstdc++-v3/include/bits/chrono_io.h
> index 86338d48d18..08969166d2f 100644
> --- a/libstdc++-v3/include/bits/chrono_io.h
> +++ b/libstdc++-v3/include/bits/chrono_io.h
> @@ -894,13 +894,42 @@ namespace __format
> // %Ec Locale's alternate date and time representation.
>
> using namespace chrono;
> + using ::std::chrono::__detail::__utc_leap_second;
> + using ::std::chrono::__detail::__local_time_fmt;
> +
> + struct tm __tm{};
> +
> + // Some locales use %Z in their %c format but we don't want
> strftime
> + // to use the system's local time zone (from /etc/localtime or
> $TZ)
> + // as the output for %Z. Setting tm_isdst to -1 says there is no
> + // time zone info available for the time in __tm.
> + __tm.tm_isdst = -1;
> +
> +#ifdef _GLIBCXX_HAVE_STRUCT_TM_TM_ZONE
> + // POSIX.1-2024 adds tm.tm_zone which will be used for %Z.
> + if constexpr (__is_time_point_v<_Tp>)
> + {
> + // One of sys_time, utc_time, or local_time.
> + if constexpr (!is_same_v<typename _Tp::clock, local_t>)
> + __tm.tm_zone = "UTC";
>
I have checked that other clocks, like TAI and GPS that user different zone
name,
are formatted using __local_time_fmt.
> + }
> + else if constexpr (__is_specialization_of<_Tp, __local_time_fmt>)
> + {
> + // local-time-format-t is used to provide time zone info for
> + // one of zoned_time, tai_time, gps_time, or local_time.
> + if (__t._M_abbrev)
> + __tm.tm_zone = __t._M_abbrev->c_str();
> + }
> + else
> + __tm.tm_zone = "UTC";
> +#endif
> +
> auto __d = _S_days(__t); // Either sys_days or local_days.
> using _TDays = decltype(__d);
> const year_month_day __ymd(__d);
> const auto __y = __ymd.year();
> const auto __hms = _S_hms(__t);
>
> - struct tm __tm{};
> __tm.tm_year = (int)__y - 1900;
> __tm.tm_yday = (__d - _TDays(__y/January/1)).count();
> __tm.tm_mon = (unsigned)__ymd.month() - 1;
> @@ -909,6 +938,7 @@ namespace __format
> __tm.tm_hour = __hms.hours().count();
> __tm.tm_min = __hms.minutes().count();
> __tm.tm_sec = __hms.seconds().count();
> +
> return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm,
> 'c',
> __mod ? 'E' : '\0');
> }
> diff --git a/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> index e7831832206..87c703dd255 100644
> --- a/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> +++ b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> @@ -28,7 +28,72 @@ test_c()
> }
> }
>
> +#include <stdlib.h>
> +#include <time.h>
> +
> +extern "C++" int tzset(int);
> +
> +template<typename T = void>
> +void maybe_set_tz()
> +{
> +#ifdef _GLIBCXX_HAVE_SETENV
> + // True if a function 'T tzset()' has been declared.
> + if constexpr (requires (T (*p)()) { p = &tzset; })
> + {
> + setenv("TZ", "GMT0", 1);
> + tzset(); // Set C library's global time zone to GMT0
> + }
> +#endif
> +}
> +
> +void
> +test_c_zoned()
> +{
> +#if _GLIBCXX_USE_CXX11_ABI
> + maybe_set_tz();
> +
> + using namespace std::chrono;
> + auto time = sys_days(2025y/March/19) + 15h;
> + zoned_time z("America/New_York", time);
> + auto s = std::format(std::locale("aa_DJ.UTF-8"), "{:L%c}", z);
> + VERIFY( s.find("11:00") != s.npos );
> +
> + // We should not print an incorrect time zone.
> + VERIFY( s.find("GMT") == s.npos );
> + VERIFY( s.find("UTC") == s.npos );
> +
> +#ifdef _GLIBCXX_HAVE_STRUCT_TM_TM_ZONE
> + // When tm.tm_zone is supported, we do print the correct time zone.
> + VERIFY( s.find(z.get_info().abbrev) != s.npos );
> +#endif
> +#endif
> +}
> +
> +void
> +test_c_local()
> +{
> + maybe_set_tz();
> +
> + std::locale("aa_DJ.UTF-8");
> + using namespace std::chrono;
> + auto time = local_days(2025y/March/19) + 11h;
> + auto s = std::format(std::locale("aa_DJ.UTF-8"), "{:L%c}", time);
> + // __builtin_printf("%s\n", s.c_str()); return;
> + VERIFY( s.find("11:00") != s.npos );
> +
> + // We should not print any time zone.
> + VERIFY( s.find("GMT") == s.npos );
> + VERIFY( s.find("UTC") == s.npos );
> +
> +#if _GLIBCXX_USE_CXX11_ABI
> + zoned_time z(system_clock::now());
> + VERIFY( s.find(z.get_info().abbrev) == s.npos );
> +#endif
> +}
> +
> int main()
> {
> test_c();
> + test_c_zoned();
> + test_c_local();
> }
> --
> 2.48.1
>
>
