From 9778a6d811c346eb9cdd8b6c6c28a915e58d1b67 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Tue, 14 Jun 2016 03:45:31 +0000 Subject: [PATCH] Make system_error::message() thread safe. Fixes PR25598. Summary: 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 Reviewers: majnemer, mclow.lists Subscribers: erik65536, cfe-commits, emaste Differential Revision: http://reviews.llvm.org/D20903 llvm-svn: 272633 --- libcxx/src/system_error.cpp | 49 ++++++++++++++++++- .../generic_category.pass.cpp | 11 +++++ .../system_category.pass.cpp | 12 +++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/libcxx/src/system_error.cpp b/libcxx/src/system_error.cpp index 3023e200aa34..748b5f7bbd07 100644 --- a/libcxx/src/system_error.cpp +++ b/libcxx/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,52 @@ error_category::equivalent(const error_code& code, int condition) const _NOEXCEP return *this == code.category() && code.value() == condition; } +namespace { + +// GLIBC also uses 1024 as the maximum buffer size internally. +constexpr size_t strerror_buff_size = 1024; + +string do_strerror_r(int ev); + +#if defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) +// GNU Extended version +string do_strerror_r(int ev) { + char buffer[strerror_buff_size]; + char* ret = ::strerror_r(ev, buffer, strerror_buff_size); + return string(ret); +} +#else +// POSIX version +string do_strerror_r(int ev) { + char buffer[strerror_buff_size]; + const int old_errno = errno; + if (::strerror_r(ev, buffer, strerror_buff_size) == -1) { + const int new_errno = errno; + errno = old_errno; + if (new_errno == EINVAL) { + std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev); + return string(buffer); + } else { + assert(new_errno == ERANGE); + // FIXME maybe? 'strerror_buff_size' is likely to exceed the + // maximum error size so ERANGE shouldn't be returned. + 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 diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp index 972299936ddf..7afbbd06980f 100644 --- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp +++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp @@ -16,10 +16,21 @@ #include #include #include +#include + +void test_message_leaves_errno_unchanged() { + errno = E2BIG; // something that message will never generate + const std::error_category& e_cat1 = std::generic_category(); + e_cat1.message(-1); + assert(errno == E2BIG); +} int main() { const std::error_category& e_cat1 = std::generic_category(); std::string m1 = e_cat1.name(); assert(m1 == "generic"); + { + test_message_leaves_errno_unchanged(); + } } diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp index b5cb18ad7653..8c1e8029252d 100644 --- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp +++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp @@ -16,6 +16,15 @@ #include #include #include +#include + + +void test_message_leaves_errno_unchanged() { + errno = E2BIG; // something that message will never generate + const std::error_category& e_cat1 = std::system_category(); + e_cat1.message(-1); + assert(errno == E2BIG); +} int main() { @@ -26,4 +35,7 @@ int main() e_cond = e_cat1.default_error_condition(5000); assert(e_cond.value() == 5000); assert(e_cond.category() == std::system_category()); + { + test_message_leaves_errno_unchanged(); + } }