[PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

[Eric Fiselier via cfe-commits]

EricWF created this revision.
EricWF added reviewers: mclow.lists, majnemer.
EricWF added a subscriber: cfe-commits.
Herald added a subscriber: emaste.

system_error::message() uses `strerror` for the generic and system categories. 
This function is not thread safe.

The fix is to use `strerror_r`. It has been available since 2001 for GNU libc 
and since BSD 4.4 on FreeBSD/OS X.
On platforms with GNU libc the extended version is used which always returns a 
valid string, even if an error occurs.

In single-threaded builds `strerror` is still used.

See https://llvm.org/bugs/show_bug.cgi?id=25598

http://reviews.llvm.org/D20903

Files:
  src/system_error.cpp

Index: src/system_error.cpp
===================================================================
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -13,8 +13,13 @@
 #include "system_error"
 
 #include "include/config_elast.h"
+#include "cerrno"
 #include "cstring"
+#include "cstdio"
+#include "cstdlib"
+#include "cassert"
 #include "string"
+#include "string.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -46,10 +51,48 @@
     return *this == code.category() && code.value() == condition;
 }
 
+namespace {
+
+
+string do_strerror_r(int ev);
+
+#if defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC)
+// GNU Extended version
+string do_strerror_r(int ev) {
+    static thread_local char buffer[1024];
+    char* ret = ::strerror_r(ev, buffer, 1024);
+    return string(ret);
+}
+#else
+// POSIX version
+string do_strerror_r(int ev) {
+    static thread_local char buffer[1024];
+    if (::strerror_r(ev, buffer, 1024) == -1) {
+        if (errno == EINVAL) {
+            std::snprintf(buffer, 1024, "Unknown error %d", ev);
+            return string(buffer);
+        } else {
+            assert(errno == ERANGE);
+            // FIXME maybe? 1024 is likely to exceed the maximum error
+            // size so ERANGE shouldn't be returned. GLIBC also uses 1024
+            // as the maximum buffer size internally.
+            std::abort();
+        }
+    }
+    return string(buffer);
+}
+#endif
+
+} // end namespace
+
 string
 __do_message::message(int ev) const
 {
-    return string(strerror(ev));
+#if defined(_LIBCPP_HAS_NO_THREADS)
+    return string(::strerror(ev));
+#else
+    return do_strerror_r(ev);
+#endif
 }
 
 class _LIBCPP_HIDDEN __generic_error_category



Index: src/system_error.cpp
===================================================================
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -13,8 +13,13 @@
 #include "system_error"
 
 #include "include/config_elast.h"
+#include "cerrno"
 #include "cstring"
+#include "cstdio"
+#include "cstdlib"
+#include "cassert"
 #include "string"
+#include "string.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -46,10 +51,48 @@
     return *this == code.category() && code.value() == condition;
 }
 
+namespace {
+
+
+string do_strerror_r(int ev);
+
+#if defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC)
+// GNU Extended version
+string do_strerror_r(int ev) {
+    static thread_local char buffer[1024];
+    char* ret = ::strerror_r(ev, buffer, 1024);
+    return string(ret);
+}
+#else
+// POSIX version
+string do_strerror_r(int ev) {
+    static thread_local char buffer[1024];
+    if (::strerror_r(ev, buffer, 1024) == -1) {
+        if (errno == EINVAL) {
+            std::snprintf(buffer, 1024, "Unknown error %d", ev);
+            return string(buffer);
+        } else {
+            assert(errno == ERANGE);
+            // FIXME maybe? 1024 is likely to exceed the maximum error
+            // size so ERANGE shouldn't be returned. GLIBC also uses 1024
+            // as the maximum buffer size internally.
+            std::abort();
+        }
+    }
+    return string(buffer);
+}
+#endif
+
+} // end namespace
+
 string
 __do_message::message(int ev) const
 {
-    return string(strerror(ev));
+#if defined(_LIBCPP_HAS_NO_THREADS)
+    return string(::strerror(ev));
+#else
+    return do_strerror_r(ev);
+#endif
 }
 
 class _LIBCPP_HIDDEN __generic_error_category

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits