[PATCH 1/5] libstdc++: Fix numeric save offset on Zone lines [PR 124851]
Tomasz Kaminski
tkaminsk@redhat.com
Wed Apr 15 08:12:16 GMT 2026
On Sat, Apr 11, 2026 at 3:34 PM Alvaro Begue <alvaro.begue@gmail.com> wrote:
> When a Zone line specifies a numeric value as its RULES field (the
> constant DST save value for that zone line, e.g. Africa/Gaborone's
> "2 1 CAST" line), the parser stored the standard offset alone in
> ZoneInfo::m_offset. ZoneInfo::to() then returned that as
> sys_info::offset, dropping the numeric save and reporting a total
> offset that was wrong by the save amount.
>
> This was inconsistent with the two ZoneInfo constructors that take a
> sys_info, which previously stored the *total* offset (stdoff + save) in
> m_offset. As a result m_offset's semantics depended on which code path
> created the ZoneInfo, and only the parser path's lines with non-zero
> numeric save were observably broken.
>
> Fix by giving m_offset a single semantics: always the standard offset
> only. The two sys_info-taking constructors now subtract the save before
> storing, and to() adds it back when reconstructing the sys_info.
>
> The remaining .offset() callers inside _M_get_sys_info already expect
> the standard offset (they are computing rule firing times, where the
> save component is added separately from the active rule's save value),
> so no other call sites need adjustment.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/124851
> * src/c++20/tzdb.cc (ZoneInfo::ZoneInfo(sys_info&&)): Store
> stdoff only in m_offset (subtract info.save).
> (ZoneInfo::ZoneInfo(const pair<sys_info, string_view>&)):
> Likewise.
> (ZoneInfo::offset()): Document new semantics.
> (ZoneInfo::to(sys_info&)): Add m_save back to offset() when
> populating sys_info::offset.
> * testsuite/std/time/time_zone/numeric_save.cc: New test.
> ---
>
The patch looks good. I have some comments regarding the included comments,
I would reduce them a bit. This will be reoccurring feedback for all of the
patches.
> libstdc++-v3/src/c++20/tzdb.cc | 13 ++--
> .../std/time/time_zone/numeric_save.cc | 66 +++++++++++++++++++
> 2 files changed, 75 insertions(+), 4 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/std/time/time_zone/numeric_save.cc
>
> diff --git a/libstdc++-v3/src/c++20/tzdb.cc
> b/libstdc++-v3/src/c++20/tzdb.cc
> index eb6ac7d07..33217f0a9 100644
> --- a/libstdc++-v3/src/c++20/tzdb.cc
> +++ b/libstdc++-v3/src/c++20/tzdb.cc
> @@ -478,11 +478,12 @@ namespace std::chrono
>
> ZoneInfo(sys_info&& info)
> : m_buf(std::move(info.abbrev)), m_expanded(true),
> m_save(info.save),
> - m_offset(info.offset), m_until(info.end)
> + m_offset(info.offset - seconds(info.save)), m_until(info.end)
> { }
>
> ZoneInfo(const pair<sys_info, string_view>& info)
> - : m_expanded(true), m_save(info.first.save),
> m_offset(info.first.offset),
> + : m_expanded(true), m_save(info.first.save),
> + m_offset(info.first.offset - seconds(info.first.save)),
> m_until(info.first.end)
> {
> if (info.second.size())
> @@ -494,7 +495,9 @@ namespace std::chrono
> m_buf += info.first.abbrev;
> }
>
> - // STDOFF: Seconds from UTC during standard time.
> + // STDOFF: Seconds from UTC during standard time. Always the
>
I would simplify that to: // STDOFF: Seconds from UTC during standard time
(without any save).
> + // standard offset only; the saved value (if any) is in m_save and
> + // is added back when reconstructing a sys_info via to().
> seconds
> offset() const noexcept { return m_offset; }
>
> @@ -539,7 +542,9 @@ namespace std::chrono
> return false;
>
> info.end = until();
> - info.offset = offset();
> + // m_offset is the standard offset only; add the saved value to
> + // reconstruct the total offset. See ZoneInfo's m_offset comment.
>
This comment repeats one from 40 lines above, I think that is a bit
excessive.
> + info.offset = offset() + seconds(m_save);
> info.save = minutes(m_save);
> info.abbrev = format();
> format_abbrev_str(info); // expand %z
> diff --git a/libstdc++-v3/testsuite/std/time/time_zone/numeric_save.cc
> b/libstdc++-v3/testsuite/std/time/time_zone/numeric_save.cc
> new file mode 100644
> index 000000000..d7b78f215
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/time_zone/numeric_save.cc
> @@ -0,0 +1,66 @@
> +// { dg-do run { target c++20 } }
> +// { dg-require-effective-target tzdb }
> +// { dg-require-effective-target cxx11_abi }
> +// { dg-xfail-run-if "no weak override on AIX" { powerpc-ibm-aix* } }
> +
> +// Regression test: when a Zone line specifies a numeric value as its
> +// RULES field, that value is the constant DST save value for that zone
> +// line. ZoneInfo::to() previously set sys_info::offset to the zone
>
We do not need to record the past bug indefinitely in the test file, having
it in
commit description is sufficeint. I would simplify to description of
behavior.
+ // When a Zone line specifies a numeric value as its
+// RULES field, that value is the constant DST save value for that zone
+// line.
> +// line's STDOFF only, ignoring the parsed save. Per [time.zone.info.sys]
> +// sys_info::offset is the *total* UTC offset (stdoff + save), so any
> +// zone line with a non-zero numeric save reported the wrong offset.
> +//
> +// Mirrors Africa/Gaborone's tzdata, which uses
> +// 2 - CAT 1943 S 19 2
> +// 2 1 CAST 1944 Mar 19 2 <-- numeric "1" RULES, save = +1h
> +// 2 - CAT
> +// The middle line is what triggers the bug.
>
And also remove this line.
> +
> +#include <chrono>
> +#include <fstream>
> +#include <testsuite_hooks.h>
> +
> +static bool override_used = false;
> +
> +namespace __gnu_cxx
> +{
> + const char* zoneinfo_dir_override() {
> + override_used = true;
> + return "./";
> + }
> +}
> +
> +int
> +main()
> +{
> + using namespace std::chrono;
> +
> + std::ofstream("tzdata.zi") << R"(# version test_numeric_save
> +Z Test/Gaborone 2 - CAT 1943 Sep 19 2
> + 2 1 CAST 1944 Mar 19 2
> + 2 - CAT
> +)";
> +
> + const auto& db = reload_tzdb();
> + VERIFY( override_used );
> + VERIFY( db.version == "test_numeric_save" );
> +
> + auto* tz = locate_zone("Test/Gaborone");
> +
> + // Sample well inside the CAST (numeric-save) zone line.
> + auto info = tz->get_info(sys_days(1943y/December/15));
> + VERIFY( info.offset == 3h ); // stdoff +2h + save +1h
> + VERIFY( info.save == 60min );
> + VERIFY( info.abbrev == "CAST" );
> +
> + // Bordering zone lines should report the standard offset with save 0.
> + auto before = tz->get_info(sys_days(1943y/September/1));
> + VERIFY( before.offset == 2h );
> + VERIFY( before.save == 0min );
> + VERIFY( before.abbrev == "CAT" );
> +
> + auto after = tz->get_info(sys_days(1944y/April/15));
> + VERIFY( after.offset == 2h );
> + VERIFY( after.save == 0min );
> + VERIFY( after.abbrev == "CAT" );
> +}
> --
> 2.34.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20260415/51a3f446/attachment.htm>
More information about the Libstdc++
mailing list