[PATCH v2] libstdc++: Fix chrono::current_zone() for three-level names [PR122567]
Jonathan Wakely
jwakely@redhat.com
Fri Mar 20 09:49:09 GMT 2026
On Fri, 20 Mar 2026 at 06:38, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Fri, Mar 20, 2026 at 2:19 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> chrono::current_zone() fails if /etc/localtime is a symlink to a zone
>> with three components, like "America/Indiana/Indianapolis", because we
>> only try to find "Indianapolis" and "Indiana/Indianapolis" but neither
>> of those is a valid zone name.
>>
>> We need to try up to three components to handle all valid cases, such as
>> "UTC", "America/Indianapolis", and "America/Indiana/Indianapolis". It's
>> also possible that users could provide a custom tzdata.zi file which
>> includes zones with names using more than three levels, so loop over all
>> filename components of the path.
>>
>> This also replaces std::filesystem::read_symlink with a plain readlink
>> call and find+substr operations on a std::string_view, which is more
>> than twice as fast as using std::filesystem::path.
>>
>> One consequence of not using filesystem::path is that redundant
>> consecutive slashes in the pathname aren't automatically ignored,
>> e.g. /usr/share/zoneinfo/Europe//London worked fine with the old
>> implementation because we manually concatenated the path components,
>> i.e. "Europe" + '/' + "London". So that this continues to work there is
>> a new loop to remove redundant slashes from the string being processed.
>> That adds a slower, allocating path, but is unlikely to be needed in
>> practice (the systemd spec for /etc/localtime explicitly says it should
>> end with a time zone name, so "Europe//London" would be invalid anyway,
>> even if it points to a valid file).
>>
>> While manually testing this I noticed that we will interpret a bogus
>> symlink such as /usr/share/zoneinfo/America/Europe/London as a valid
>> timezone, even though it's a dangling symlink. We find a name match for
>> "Europe/London" before we get to the "America" component. This seems
>> unlikely to matter in practice.
>>
>> There's no testcase for current_zone() correctly handling three-level
>> names or symlinks with unusual targets. It cannot be tested without
>> changing the target of /etc/localtime which requires root access.
>>
>> I'm still considering whether we want to cache the result of
>> current_zone(), either globally or in the tzdb object. Just returning a
>> cached variable takes 20-30ns instead of more than 700ns to access the
>> filesystem and read the symlink.
>>
>> libstdc++-v3/ChangeLog:
>>
>> PR libstdc++/122567
>> * src/c++20/tzdb.cc (tzdb::current_zone): Loop over all trailing
>> components of /etc/localtime path. Use readlink instead of
>> std::filesystem::read_symlink.
>> ---
>>
>> v2: Tomasz wanted this to check for more than 3 components. I did that,
>> and then completely re-implemented it, improving the performance.
>
> LGTM with one small comment.
>>
>>
>> Tested x86_64-linux.
>>
>> libstdc++-v3/src/c++20/tzdb.cc | 77 +++++++++++++++++++++++++++-------
>> 1 file changed, 61 insertions(+), 16 deletions(-)
>>
>> diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc
>> index e42ba70dee24..8dcb26c1da12 100644
>> --- a/libstdc++-v3/src/c++20/tzdb.cc
>> +++ b/libstdc++-v3/src/c++20/tzdb.cc
>> @@ -35,10 +35,14 @@
>> #include <atomic> // atomic<T*>, atomic<int>
>> #include <memory> // atomic<shared_ptr<T>>
>> #include <mutex> // mutex
>> +#include <iomanip> // quoted
>> #if defined __GTHREADS && ! defined _GLIBCXX_HAS_GTHREADS
>> # include <ext/concurrence.h> // __gnu_cxx::__mutex
>> #endif
>> -#include <filesystem> // filesystem::read_symlink
>> +
>> +#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H)
>> +# include <unistd.h> // readlink
>> +#endif
>>
>> #ifndef _AIX
>> # include <cstdlib> // getenv
>> @@ -1890,28 +1894,64 @@ namespace std::chrono
>> tzdb::current_zone() const
>> {
>> // TODO cache this function's result?
>> + // Could check the modification time of /etc/localtime, and not re-read
>> + // it if it hasn't changed. reload_tzdb() could clear the cache too,
>> + // to have a way to force a re-read.
>>
>> #if !defined(_AIX) && !defined(_GLIBCXX_HAVE_WINDOWS_H)
>> - // Repeat the preprocessor condition used by filesystem::read_symlink,
>> - // to avoid a dependency on src/c++17/fs_ops.o if it won't work anyway.
>> -#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_SYS_STAT_H)
>> +#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H)
>> + // Use readlink directly to avoid std::filesystem overhead.
>> error_code ec;
>> // This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
>> - auto path = filesystem::read_symlink("/etc/localtime", ec);
>> - if (!ec)
>> + // https://www.freedesktop.org/software/systemd/man/latest/localtime.html
>> + // This should work on GNU/Linux, macOS, NetBSD, and OpenBSD.
>> + // Some FreeBSD systems also use a symlink for /etc/localtime.
>> + char buf[512];
>
> Using char buffer seem like overkill here, we could just declare a std::string
> before the if, and reffer to it in the loop.
>>
>> + if (auto n = ::readlink("/etc/localtime", buf, sizeof(buf)); n > 0)
>> {
>> - auto first = path.begin(), last = path.end();
>> - if (std::distance(first, last) > 2)
>> + string_view str(buf, n);
>> +
>> + // Remove any redundant slashes so we can match zone names.
>> + // e.g. /usr/share/zoneinfo/Europe//London is a valid symlink,
>> + // but won't match against "Europe/London".
>> + if (auto pos = str.rfind("//"); pos != str.npos) [[unlikely]]
>> {
>> - --last;
>> - string name = last->string();
>> - if (auto tz = do_locate_zone(this->zones, this->links, name))
>> - return tz;
>> - --last;
>> - name = last->string() + '/' + name;
>> - if (auto tz = do_locate_zone(this->zones, this->links, name))
>> - return tz;
>> + string sanitized(str);
>> + string::size_type spos = pos;
>> + do
>> + {
>> + sanitized.erase(spos, 1);
>> + spos = sanitized.rfind("//", spos);
>> + }
>> + while (spos != sanitized.npos);
>> + sanitized.copy(buf, sanitized.size());
>>
>> + str = string_view(buf, sanitized.size())
>
> This way the above would be replaced with
> str = buf;
My goal was to avoid allocating anything on the heap for the common
case. I don't think we should care about the extra copy for the //
case, because that should never happen anyway.
But I just realized that I forgot to check n < sizeof(buf) to check
that the result wasn't truncated.
>>
>> ;
>> }
>> +
>> + // Check the trailing components of the path against known zone names.
>> + // Valid IANA times zones can have one, two, or three parts, e.g.
>> + // "UTC", "Europe/London", and "America/Indiana/Indianapolis".
>> + // Custom tzdata.zi files could in theory use four or more parts.
>> +
>> + auto pos = str.rfind('/');
>> + while (pos != str.npos && pos != 0)
>> + {
>> + if (auto tz = do_locate_zone(this->zones, this->links,
>> + str.substr(pos + 1)))
>> + return tz;
>> + pos = str.rfind('/', pos - 1);
>> + }
>> + // If we didn't match yet, try once more so that we will match
>> + // a symlink to a relative path such as "Europe/London"
>> + // or symlink to an absolute path such as "/Europe/London".
>> + // Both cases seem unlikely because it would require either
>> + // /etc/Europe or /Europe to be a directory (or a symlink to one)
>> + // containing the TZif files, but it's theoretically possible.
>> + // If pos==npos then pos+1 wraps to 0 and we use the whole string.
>> + // If pos==0 then substr(1) discards the leading slash.
>
> I like that you included a comment explaining what happens for npos.
>
>>
>> + if (auto tz = do_locate_zone(this->zones, this->links,
>> + str.substr(pos + 1)))
>> + return tz;
>> }
>> #endif
>> // Otherwise, look for a file naming the time zone.
>> @@ -1948,6 +1988,11 @@ namespace std::chrono
>> return tz;
>> }
>> }
>> +
>> + // FIXME: For DragonFly BSD /etc/localtime is a copy of one of the
>> + // zone files in /usr/share/zoneinfo so we need to compare its contents
>> + // to each one until we find a match.
>> +
>> #elif defined(_GLIBCXX_HAVE_WINDOWS_H)
>> if (auto tz
>> = do_locate_zone(this->zones, this->links, detect_windows_zone()))
>> --
>> 2.53.0
>>
More information about the Libstdc++
mailing list