[PATCH v2] libstdc++: Fix chrono::current_zone() for three-level names [PR122567]
Tomasz Kaminski
tkaminsk@redhat.com
Fri Mar 20 06:37:44 GMT 2026
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;
> ;
> }
> +
> + // 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20260320/678c2688/attachment-0001.htm>
More information about the Libstdc++
mailing list