locale.c: strerror_l() not fool proof

Commit 7aaa36b196e5a478a3d1bd32506797db7cebf0b2 changed to use
strerror_l() if available on the platform.  But there is a potential bug
with this on threaded perls.  The code uses strerror_l() when it needs
the answer on a locale that isn't necessarily the current one.  But it
uses plain strerror() when the locale is known to be the current one.
Plain strerror() isn't necessarily thread-safe.  However, on systems
that have strerror_r(), reentr.h has caused our apparent call to plain
strerror() to instead call the thread-safe strerror_r() under the hood.
So there is no bug on unthreaded perls nor on ones that have
strerror_r().

This commit fixes the bug on threaded builds which have strerror_l() but
not strerror_r().  It does this by using strerror_l() for everything,
and constructing a locale object that is the current locale to use when
the locale doesn't need to be changed.  This is somewhat more work than
the alternative above does, so that one is used if available.

No changes are made to how it works on systems that don't have
strerror_l().

Some systems have deprecated strerror_r().  reentr.h does not use it on
such systems.  The reason for the deprecation, we would hope, may be
that the plain strerror() is implemented thread-safely.  We don't know
that, so we just assume that the plain version is thread-unsafe.

We do have tests that try to find races here, but they haven't shown
any.  It could be that systems that are advanced enough to have
strerror_l() also have strerror_r().

diff --git a/locale.c b/locale.c
index d56fd40..436456a 100644 (file)
--- a/locale.c
+++ b/locale.c
@@ -3403,7 +3403,17 @@ Perl_my_strerror(pTHX_ const int errnum)
 
 #  if defined(HAS_POSIX_2008_LOCALE) && defined(HAS_STRERROR_L)
 
-    /* This function is trivial if we have strerror_l() */
+    /* This function is trivial if we don't have to worry about thread safety
+     * and have strerror_l(), as it handles the switch of locales so we don't
+     * have to deal with that.  We don't have to worry about thread safety if
+     * this is an unthreaded build, or if strerror_r() is also available.  Both
+     * it and strerror_l() are thread-safe.  Plain strerror() isn't thread
+     * safe.  But on threaded builds when strerror_r() is available, the
+     * apparent call to strerror() below is actually a macro that
+     * behind-the-scenes calls strerror_r().
+     */
+
+#    if ! defined(USE_ITHREADS) || defined(HAS_STRERROR_R)
 
     if (within_locale_scope) {
         errstr = savepv(strerror(errnum));
@@ -3412,7 +3422,34 @@ Perl_my_strerror(pTHX_ const int errnum)
         errstr = savepv(strerror_l(errnum, PL_C_locale_obj));
     }
 
-#  else /* Doesn't have strerror_l(). */
+#    else
+
+    /* Here we have strerror_l(), but not strerror_r() and we are on a
+     * threaded-build.  We use strerror_l() for everything, constructing a
+     * locale to pass to it if necessary */
+
+    bool do_free = FALSE;
+    locale_t locale_to_use;
+
+    if (within_locale_scope) {
+        locale_to_use = uselocale((locale_t) 0);
+        if (locale_to_use == LC_GLOBAL_LOCALE) {
+            locale_to_use = duplocale(LC_GLOBAL_LOCALE);
+            do_free = TRUE;
+        }
+    }
+    else {  /* Use C locale if not within 'use locale' scope */
+        locale_to_use = PL_C_locale_obj;
+    }
+
+    errstr = savepv(strerror_l(errnum, locale_to_use));
+
+    if (do_free) {
+        freelocale(locale_to_use);
+    }
+
+#    endif
+#  else /* Doesn't have strerror_l() */
 
 #    ifdef USE_POSIX_2008_LOCALE