Skip to content

Commit 2b26ee6

Browse files
authored
[libcxx] Handle windows system error code mapping in std::error_code. (#93101)
The `std::error_code`/`std::error_category` functionality is designed to support multiple error domains. On Unix, both system calls and libc functions return the same error codes, and thus, libc++ today treats `generic_category()` and `system_category()` as being equivalent. However, on Windows, libc functions return `errno.h` error codes in the `errno` global, but system calls return the very different `winerror.h` error codes via `GetLastError()`. As such, there is a need to map the winerror.h error codes into generic errno codes. In libc++, however, the system_error facility does not implement this mapping; instead the mapping is hidden inside libc++, used directly by the std::filesystem implementation. That has a few problems: 1. For std::filesystem APIs, the concrete windows error number is lost, before users can see it. The intent of the distinction between std::error_code and std::error_condition is that the error_code return has the original (potentially more detailed) error code. 2. User-written code which calls Windows system APIs requires this same mapping, so it also can also return error_code objects that other (cross-platform) code can understand. After this commit, an `error_code` with `generic_category()` is used to report an error from `errno`, and, on Windows only, an `error_code` with `system_category()` is used to report an error from `GetLastError()`. On Unix, system_category remains identity-mapped to generic_category, but is never used by libc++ itself. The windows error code mapping is moved into system_error, so that conversion of an `error_code` to `error_condition` correctly translates the `system_category()` code into a `generic_category()` code, when appropriate. This allows code like: `error_code(GetLastError(), system_category()) == errc::invalid_argument` to work as expected -- as it does with MSVC STL. (Continued from old phabricator review [D151493](https://reviews.llvm.org/D151493))
1 parent 0d921f9 commit 2b26ee6

File tree

18 files changed

+330
-161
lines changed

18 files changed

+330
-161
lines changed

libcxx/docs/ReleaseNotes/20.rst

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,39 @@ Improvements and New Features
7373
optimized, resulting in a performance improvement of up to 2x for trivial element types (e.g., `std::vector<int>`),
7474
and up to 3.4x for non-trivial element types (e.g., `std::vector<std::vector<int>>`).
7575

76+
- On Windows, ``<system_error>``'s ``std::system_category`` is now distinct from ``std::generic_category``. The behavior
77+
on other operating systems is unchanged.
78+
79+
On Windows -- unlike on Unix systems -- the libc and system APIs use distinct error codes. The libc functions return
80+
``errno.h`` error codes via the ``errno`` global, while Win32 API functions return ``winerror.h`` error codes via
81+
``GetLastError()``.
82+
83+
The C++ standard's ``std::error_code`` and ``std::error_category`` functionality was designed to support multiple
84+
error domains, precisely in order to handle situations such as this. However, libc++ formerly treated
85+
``generic_category()`` and ``system_category()`` as equivalent, even on Windows. It now implements the intended split,
86+
where ``system_category`` represents native ``winerror.h`` error codes, and ``generic_category`` represents libc error
87+
codes (and, equivalently, ``std::errc::*`` errors).
88+
89+
This change enables code like ``std::error_code(GetLastError(), std::system_category()) ==
90+
std::errc::invalid_argument`` to function as desired: constructing an ``error_code`` with the Windows error number in
91+
the "system" category, and then mapping it to a generic code with ``error_condition``, for comparison with the
92+
``std::errc`` constant.
93+
94+
This is an incompatible change: ``std::error_code(ENOSYS, std::system_category()) ==
95+
std::errc::function_not_supported`` would formerly have returned true, but now returns false on Windows. Code
96+
providing a number from the ``errno.h`` domain should be migrated to construct a ``generic_category`` error_code,
97+
instead. (E.g., use ``std::error_code(ENOSYS, std::generic_category())``). The new behavior matches MSVC.
98+
99+
- On Windows, the ``std::filesystem`` library now returns the Win32 ``system_category`` error codes, where it's feasible
100+
to do so. This allows interrogation and reporting of the original error code, which is useful if multiple Windows
101+
errors map to a single generic error (such as with ``std::errc::no_such_file_or_directory``).
102+
103+
This is also a slightly-incompatible API change: code inspecting the raw integer value from the returned error_code
104+
expecting an integer from ``generic_category`` (e.g. ``err.value() == ENOTDIR``) will not work as desired. Instead,
105+
such code should use the comparison operators which implicitly handle eror mappings, ``err ==
106+
std::errc::not_a_directory``, or use ``err.default_error_condition()`` to map to an ``error_condition``, and then test
107+
its ``value()`` and ``category()``.
108+
76109
Deprecations and Removals
77110
-------------------------
78111

libcxx/include/__filesystem/directory_entry.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
#include <__filesystem/perms.h>
2323
#include <__fwd/ostream.h>
2424
#include <__system_error/errc.h>
25+
#include <__system_error/error_category.h>
2526
#include <__system_error/error_code.h>
27+
#include <__system_error/error_condition.h>
2628
#include <__utility/move.h>
2729
#include <__utility/unreachable.h>
2830
#include <cstdint>
@@ -274,15 +276,7 @@ class directory_entry {
274276
_LIBCPP_EXPORTED_FROM_ABI error_code __do_refresh() noexcept;
275277

276278
_LIBCPP_HIDE_FROM_ABI static bool __is_dne_error(error_code const& __ec) {
277-
if (!__ec)
278-
return true;
279-
switch (static_cast<errc>(__ec.value())) {
280-
case errc::no_such_file_or_directory:
281-
case errc::not_a_directory:
282-
return true;
283-
default:
284-
return false;
285-
}
279+
return !__ec || __ec == errc::no_such_file_or_directory || __ec == errc::not_a_directory;
286280
}
287281

288282
_LIBCPP_HIDE_FROM_ABI void

libcxx/include/__system_error/system_error.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ class _LIBCPP_EXPORTED_FROM_ABI system_error : public runtime_error {
3939
_LIBCPP_HIDE_FROM_ABI const error_code& code() const _NOEXCEPT { return __ec_; }
4040
};
4141

42+
// __ev is expected to be an error in the generic_category domain (e.g. from
43+
// errno, or std::errc::*), not system_category (e.g. from windows syscalls).
44+
[[__noreturn__]] _LIBCPP_EXPORTED_FROM_ABI void __throw_system_error(int __ev, const char* __what_arg);
45+
4246
[[__noreturn__]] _LIBCPP_HIDE_FROM_ABI inline void __throw_system_error(error_code __ec, const char* __what_arg) {
4347
#if _LIBCPP_HAS_EXCEPTIONS
4448
throw system_error(__ec, __what_arg);

libcxx/src/filesystem/directory_iterator.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ class __dir_stream {
4747
}
4848
__stream_ = ::FindFirstFileW((root / "*").c_str(), &__data_);
4949
if (__stream_ == INVALID_HANDLE_VALUE) {
50-
ec = detail::make_windows_error(GetLastError());
50+
ec = detail::get_last_error();
5151
const bool ignore_permission_denied = bool(opts & directory_options::skip_permission_denied);
52-
if (ignore_permission_denied && ec.value() == static_cast<int>(errc::permission_denied))
52+
if (ignore_permission_denied && ec == errc::permission_denied)
5353
ec.clear();
5454
return;
5555
}
@@ -91,7 +91,7 @@ class __dir_stream {
9191
error_code close() noexcept {
9292
error_code ec;
9393
if (!::FindClose(__stream_))
94-
ec = detail::make_windows_error(GetLastError());
94+
ec = detail::get_last_error();
9595
__stream_ = INVALID_HANDLE_VALUE;
9696
return ec;
9797
}
@@ -118,7 +118,7 @@ class __dir_stream {
118118
if ((__stream_ = ::opendir(root.c_str())) == nullptr) {
119119
ec = detail::capture_errno();
120120
const bool allow_eacces = bool(opts & directory_options::skip_permission_denied);
121-
if (allow_eacces && ec.value() == EACCES)
121+
if (allow_eacces && ec == errc::permission_denied)
122122
ec.clear();
123123
return;
124124
}
@@ -307,7 +307,7 @@ bool recursive_directory_iterator::__try_recursion(error_code* ec) {
307307
}
308308
if (m_ec) {
309309
const bool allow_eacess = bool(__imp_->__options_ & directory_options::skip_permission_denied);
310-
if (m_ec.value() == EACCES && allow_eacess) {
310+
if (m_ec == errc::permission_denied && allow_eacess) {
311311
if (ec)
312312
ec->clear();
313313
} else {

libcxx/src/filesystem/error.h

Lines changed: 7 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -32,80 +32,21 @@ _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
3232

3333
namespace detail {
3434

35-
#if defined(_LIBCPP_WIN32API)
36-
37-
inline errc __win_err_to_errc(int err) {
38-
constexpr struct {
39-
DWORD win;
40-
errc errc;
41-
} win_error_mapping[] = {
42-
{ERROR_ACCESS_DENIED, errc::permission_denied},
43-
{ERROR_ALREADY_EXISTS, errc::file_exists},
44-
{ERROR_BAD_NETPATH, errc::no_such_file_or_directory},
45-
{ERROR_BAD_PATHNAME, errc::no_such_file_or_directory},
46-
{ERROR_BAD_UNIT, errc::no_such_device},
47-
{ERROR_BROKEN_PIPE, errc::broken_pipe},
48-
{ERROR_BUFFER_OVERFLOW, errc::filename_too_long},
49-
{ERROR_BUSY, errc::device_or_resource_busy},
50-
{ERROR_BUSY_DRIVE, errc::device_or_resource_busy},
51-
{ERROR_CANNOT_MAKE, errc::permission_denied},
52-
{ERROR_CANTOPEN, errc::io_error},
53-
{ERROR_CANTREAD, errc::io_error},
54-
{ERROR_CANTWRITE, errc::io_error},
55-
{ERROR_CURRENT_DIRECTORY, errc::permission_denied},
56-
{ERROR_DEV_NOT_EXIST, errc::no_such_device},
57-
{ERROR_DEVICE_IN_USE, errc::device_or_resource_busy},
58-
{ERROR_DIR_NOT_EMPTY, errc::directory_not_empty},
59-
{ERROR_DIRECTORY, errc::invalid_argument},
60-
{ERROR_DISK_FULL, errc::no_space_on_device},
61-
{ERROR_FILE_EXISTS, errc::file_exists},
62-
{ERROR_FILE_NOT_FOUND, errc::no_such_file_or_directory},
63-
{ERROR_HANDLE_DISK_FULL, errc::no_space_on_device},
64-
{ERROR_INVALID_ACCESS, errc::permission_denied},
65-
{ERROR_INVALID_DRIVE, errc::no_such_device},
66-
{ERROR_INVALID_FUNCTION, errc::function_not_supported},
67-
{ERROR_INVALID_HANDLE, errc::invalid_argument},
68-
{ERROR_INVALID_NAME, errc::no_such_file_or_directory},
69-
{ERROR_INVALID_PARAMETER, errc::invalid_argument},
70-
{ERROR_LOCK_VIOLATION, errc::no_lock_available},
71-
{ERROR_LOCKED, errc::no_lock_available},
72-
{ERROR_NEGATIVE_SEEK, errc::invalid_argument},
73-
{ERROR_NOACCESS, errc::permission_denied},
74-
{ERROR_NOT_ENOUGH_MEMORY, errc::not_enough_memory},
75-
{ERROR_NOT_READY, errc::resource_unavailable_try_again},
76-
{ERROR_NOT_SAME_DEVICE, errc::cross_device_link},
77-
{ERROR_NOT_SUPPORTED, errc::not_supported},
78-
{ERROR_OPEN_FAILED, errc::io_error},
79-
{ERROR_OPEN_FILES, errc::device_or_resource_busy},
80-
{ERROR_OPERATION_ABORTED, errc::operation_canceled},
81-
{ERROR_OUTOFMEMORY, errc::not_enough_memory},
82-
{ERROR_PATH_NOT_FOUND, errc::no_such_file_or_directory},
83-
{ERROR_READ_FAULT, errc::io_error},
84-
{ERROR_REPARSE_TAG_INVALID, errc::invalid_argument},
85-
{ERROR_RETRY, errc::resource_unavailable_try_again},
86-
{ERROR_SEEK, errc::io_error},
87-
{ERROR_SHARING_VIOLATION, errc::permission_denied},
88-
{ERROR_TOO_MANY_OPEN_FILES, errc::too_many_files_open},
89-
{ERROR_WRITE_FAULT, errc::io_error},
90-
{ERROR_WRITE_PROTECT, errc::permission_denied},
91-
};
92-
93-
for (const auto& pair : win_error_mapping)
94-
if (pair.win == static_cast<DWORD>(err))
95-
return pair.errc;
96-
return errc::invalid_argument;
97-
}
98-
99-
#endif // _LIBCPP_WIN32API
35+
// On windows, libc functions use errno, but system functions use GetLastError.
36+
// So, callers need to be careful which of these next functions they call!
10037

10138
inline error_code capture_errno() {
10239
_LIBCPP_ASSERT_INTERNAL(errno != 0, "Expected errno to be non-zero");
10340
return error_code(errno, generic_category());
10441
}
10542

43+
inline error_code get_last_error() {
10644
#if defined(_LIBCPP_WIN32API)
107-
inline error_code make_windows_error(int err) { return make_error_code(__win_err_to_errc(err)); }
45+
return std::error_code(GetLastError(), std::system_category());
46+
#else
47+
return capture_errno();
10848
#endif
49+
}
10950

11051
template <class T>
11152
T error_value();

libcxx/src/filesystem/file_descriptor.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ inline perms posix_get_perms(const StatT& st) noexcept { return static_cast<perm
201201
inline file_status create_file_status(error_code& m_ec, path const& p, const StatT& path_stat, error_code* ec) {
202202
if (ec)
203203
*ec = m_ec;
204-
if (m_ec && (m_ec.value() == ENOENT || m_ec.value() == ENOTDIR)) {
204+
if (m_ec && (m_ec == errc::no_such_file_or_directory || m_ec == errc::not_a_directory)) {
205205
return file_status(file_type::not_found);
206206
} else if (m_ec) {
207207
ErrorHandler<void> err("posix_stat", ec, &p);
@@ -236,7 +236,7 @@ inline file_status create_file_status(error_code& m_ec, path const& p, const Sta
236236
inline file_status posix_stat(path const& p, StatT& path_stat, error_code* ec) {
237237
error_code m_ec;
238238
if (detail::stat(p.c_str(), &path_stat) == -1)
239-
m_ec = detail::capture_errno();
239+
m_ec = detail::get_last_error();
240240
return create_file_status(m_ec, p, path_stat, ec);
241241
}
242242

@@ -248,7 +248,7 @@ inline file_status posix_stat(path const& p, error_code* ec) {
248248
inline file_status posix_lstat(path const& p, StatT& path_stat, error_code* ec) {
249249
error_code m_ec;
250250
if (detail::lstat(p.c_str(), &path_stat) == -1)
251-
m_ec = detail::capture_errno();
251+
m_ec = detail::get_last_error();
252252
return create_file_status(m_ec, p, path_stat, ec);
253253
}
254254

@@ -260,7 +260,7 @@ inline file_status posix_lstat(path const& p, error_code* ec) {
260260
// http://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html
261261
inline bool posix_ftruncate(const FileDescriptor& fd, off_t to_size, error_code& ec) {
262262
if (detail::ftruncate(fd.fd, to_size) == -1) {
263-
ec = capture_errno();
263+
ec = get_last_error();
264264
return true;
265265
}
266266
ec.clear();
@@ -269,7 +269,7 @@ inline bool posix_ftruncate(const FileDescriptor& fd, off_t to_size, error_code&
269269

270270
inline bool posix_fchmod(const FileDescriptor& fd, const StatT& st, error_code& ec) {
271271
if (detail::fchmod(fd.fd, st.st_mode) == -1) {
272-
ec = capture_errno();
272+
ec = get_last_error();
273273
return true;
274274
}
275275
ec.clear();
@@ -286,7 +286,7 @@ inline file_status FileDescriptor::refresh_status(error_code& ec) {
286286
m_stat = {};
287287
error_code m_ec;
288288
if (detail::fstat(fd, &m_stat) == -1)
289-
m_ec = capture_errno();
289+
m_ec = get_last_error();
290290
m_status = create_file_status(m_ec, name, m_stat, &ec);
291291
return m_status;
292292
}

0 commit comments

Comments
 (0)