Skip to content

[libcxx] Handle windows system error code mapping in std::error_code. #93101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions libcxx/docs/ReleaseNotes/20.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,39 @@ Improvements and New Features
optimized, resulting in a performance improvement of up to 2x for trivial element types (e.g., `std::vector<int>`),
and up to 3.4x for non-trivial element types (e.g., `std::vector<std::vector<int>>`).

- On Windows, ``<system_error>``'s ``std::system_category`` is now distinct from ``std::generic_category``. The behavior
on other operating systems is unchanged.

On Windows -- unlike on Unix systems -- the libc and system APIs use distinct error codes. The libc functions return
``errno.h`` error codes via the ``errno`` global, while Win32 API functions return ``winerror.h`` error codes via
``GetLastError()``.

The C++ standard's ``std::error_code`` and ``std::error_category`` functionality was designed to support multiple
error domains, precisely in order to handle situations such as this. However, libc++ formerly treated
``generic_category()`` and ``system_category()`` as equivalent, even on Windows. It now implements the intended split,
where ``system_category`` represents native ``winerror.h`` error codes, and ``generic_category`` represents libc error
codes (and, equivalently, ``std::errc::*`` errors).

This change enables code like ``std::error_code(GetLastError(), std::system_category()) ==
std::errc::invalid_argument`` to function as desired: constructing an ``error_code`` with the Windows error number in
the "system" category, and then mapping it to a generic code with ``error_condition``, for comparison with the
``std::errc`` constant.

This is an incompatible change: ``std::error_code(ENOSYS, std::system_category()) ==
std::errc::function_not_supported`` would formerly have returned true, but now returns false on Windows. Code
providing a number from the ``errno.h`` domain should be migrated to construct a ``generic_category`` error_code,
instead. (E.g., use ``std::error_code(ENOSYS, std::generic_category())``). The new behavior matches MSVC.

- On Windows, the ``std::filesystem`` library now returns the Win32 ``system_category`` error codes, where it's feasible
to do so. This allows interrogation and reporting of the original error code, which is useful if multiple Windows
errors map to a single generic error (such as with ``std::errc::no_such_file_or_directory``).

This is also a slightly-incompatible API change: code inspecting the raw integer value from the returned error_code
expecting an integer from ``generic_category`` (e.g. ``err.value() == ENOTDIR``) will not work as desired. Instead,
such code should use the comparison operators which implicitly handle eror mappings, ``err ==
std::errc::not_a_directory``, or use ``err.default_error_condition()`` to map to an ``error_condition``, and then test
its ``value()`` and ``category()``.

Deprecations and Removals
-------------------------

Expand Down
12 changes: 3 additions & 9 deletions libcxx/include/__filesystem/directory_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
#include <__filesystem/perms.h>
#include <__fwd/ostream.h>
#include <__system_error/errc.h>
#include <__system_error/error_category.h>
#include <__system_error/error_code.h>
#include <__system_error/error_condition.h>
#include <__utility/move.h>
#include <__utility/unreachable.h>
#include <cstdint>
Expand Down Expand Up @@ -274,15 +276,7 @@ class directory_entry {
_LIBCPP_EXPORTED_FROM_ABI error_code __do_refresh() noexcept;

_LIBCPP_HIDE_FROM_ABI static bool __is_dne_error(error_code const& __ec) {
if (!__ec)
return true;
switch (static_cast<errc>(__ec.value())) {
case errc::no_such_file_or_directory:
case errc::not_a_directory:
return true;
default:
return false;
}
return !__ec || __ec == errc::no_such_file_or_directory || __ec == errc::not_a_directory;
}

_LIBCPP_HIDE_FROM_ABI void
Expand Down
4 changes: 4 additions & 0 deletions libcxx/include/__system_error/system_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class _LIBCPP_EXPORTED_FROM_ABI system_error : public runtime_error {
_LIBCPP_HIDE_FROM_ABI const error_code& code() const _NOEXCEPT { return __ec_; }
};

// __ev is expected to be an error in the generic_category domain (e.g. from
// errno, or std::errc::*), not system_category (e.g. from windows syscalls).
[[__noreturn__]] _LIBCPP_EXPORTED_FROM_ABI void __throw_system_error(int __ev, const char* __what_arg);

[[__noreturn__]] _LIBCPP_HIDE_FROM_ABI inline void __throw_system_error(error_code __ec, const char* __what_arg) {
#if _LIBCPP_HAS_EXCEPTIONS
throw system_error(__ec, __what_arg);
Expand Down
10 changes: 5 additions & 5 deletions libcxx/src/filesystem/directory_iterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ class __dir_stream {
}
__stream_ = ::FindFirstFileW((root / "*").c_str(), &__data_);
if (__stream_ == INVALID_HANDLE_VALUE) {
ec = detail::make_windows_error(GetLastError());
ec = detail::get_last_error();
const bool ignore_permission_denied = bool(opts & directory_options::skip_permission_denied);
if (ignore_permission_denied && ec.value() == static_cast<int>(errc::permission_denied))
if (ignore_permission_denied && ec == errc::permission_denied)
ec.clear();
return;
}
Expand Down Expand Up @@ -91,7 +91,7 @@ class __dir_stream {
error_code close() noexcept {
error_code ec;
if (!::FindClose(__stream_))
ec = detail::make_windows_error(GetLastError());
ec = detail::get_last_error();
__stream_ = INVALID_HANDLE_VALUE;
return ec;
}
Expand All @@ -118,7 +118,7 @@ class __dir_stream {
if ((__stream_ = ::opendir(root.c_str())) == nullptr) {
ec = detail::capture_errno();
const bool allow_eacces = bool(opts & directory_options::skip_permission_denied);
if (allow_eacces && ec.value() == EACCES)
if (allow_eacces && ec == errc::permission_denied)
ec.clear();
return;
}
Expand Down Expand Up @@ -307,7 +307,7 @@ bool recursive_directory_iterator::__try_recursion(error_code* ec) {
}
if (m_ec) {
const bool allow_eacess = bool(__imp_->__options_ & directory_options::skip_permission_denied);
if (m_ec.value() == EACCES && allow_eacess) {
if (m_ec == errc::permission_denied && allow_eacess) {
if (ec)
ec->clear();
} else {
Expand Down
73 changes: 7 additions & 66 deletions libcxx/src/filesystem/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,80 +32,21 @@ _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM

namespace detail {

#if defined(_LIBCPP_WIN32API)

inline errc __win_err_to_errc(int err) {
constexpr struct {
DWORD win;
errc errc;
} win_error_mapping[] = {
{ERROR_ACCESS_DENIED, errc::permission_denied},
{ERROR_ALREADY_EXISTS, errc::file_exists},
{ERROR_BAD_NETPATH, errc::no_such_file_or_directory},
{ERROR_BAD_PATHNAME, errc::no_such_file_or_directory},
{ERROR_BAD_UNIT, errc::no_such_device},
{ERROR_BROKEN_PIPE, errc::broken_pipe},
{ERROR_BUFFER_OVERFLOW, errc::filename_too_long},
{ERROR_BUSY, errc::device_or_resource_busy},
{ERROR_BUSY_DRIVE, errc::device_or_resource_busy},
{ERROR_CANNOT_MAKE, errc::permission_denied},
{ERROR_CANTOPEN, errc::io_error},
{ERROR_CANTREAD, errc::io_error},
{ERROR_CANTWRITE, errc::io_error},
{ERROR_CURRENT_DIRECTORY, errc::permission_denied},
{ERROR_DEV_NOT_EXIST, errc::no_such_device},
{ERROR_DEVICE_IN_USE, errc::device_or_resource_busy},
{ERROR_DIR_NOT_EMPTY, errc::directory_not_empty},
{ERROR_DIRECTORY, errc::invalid_argument},
{ERROR_DISK_FULL, errc::no_space_on_device},
{ERROR_FILE_EXISTS, errc::file_exists},
{ERROR_FILE_NOT_FOUND, errc::no_such_file_or_directory},
{ERROR_HANDLE_DISK_FULL, errc::no_space_on_device},
{ERROR_INVALID_ACCESS, errc::permission_denied},
{ERROR_INVALID_DRIVE, errc::no_such_device},
{ERROR_INVALID_FUNCTION, errc::function_not_supported},
{ERROR_INVALID_HANDLE, errc::invalid_argument},
{ERROR_INVALID_NAME, errc::no_such_file_or_directory},
{ERROR_INVALID_PARAMETER, errc::invalid_argument},
{ERROR_LOCK_VIOLATION, errc::no_lock_available},
{ERROR_LOCKED, errc::no_lock_available},
{ERROR_NEGATIVE_SEEK, errc::invalid_argument},
{ERROR_NOACCESS, errc::permission_denied},
{ERROR_NOT_ENOUGH_MEMORY, errc::not_enough_memory},
{ERROR_NOT_READY, errc::resource_unavailable_try_again},
{ERROR_NOT_SAME_DEVICE, errc::cross_device_link},
{ERROR_NOT_SUPPORTED, errc::not_supported},
{ERROR_OPEN_FAILED, errc::io_error},
{ERROR_OPEN_FILES, errc::device_or_resource_busy},
{ERROR_OPERATION_ABORTED, errc::operation_canceled},
{ERROR_OUTOFMEMORY, errc::not_enough_memory},
{ERROR_PATH_NOT_FOUND, errc::no_such_file_or_directory},
{ERROR_READ_FAULT, errc::io_error},
{ERROR_REPARSE_TAG_INVALID, errc::invalid_argument},
{ERROR_RETRY, errc::resource_unavailable_try_again},
{ERROR_SEEK, errc::io_error},
{ERROR_SHARING_VIOLATION, errc::permission_denied},
{ERROR_TOO_MANY_OPEN_FILES, errc::too_many_files_open},
{ERROR_WRITE_FAULT, errc::io_error},
{ERROR_WRITE_PROTECT, errc::permission_denied},
};

for (const auto& pair : win_error_mapping)
if (pair.win == static_cast<DWORD>(err))
return pair.errc;
return errc::invalid_argument;
}

#endif // _LIBCPP_WIN32API
// On windows, libc functions use errno, but system functions use GetLastError.
// So, callers need to be careful which of these next functions they call!

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

inline error_code get_last_error() {
#if defined(_LIBCPP_WIN32API)
inline error_code make_windows_error(int err) { return make_error_code(__win_err_to_errc(err)); }
return std::error_code(GetLastError(), std::system_category());
#else
return capture_errno();
#endif
}

template <class T>
T error_value();
Expand Down
12 changes: 6 additions & 6 deletions libcxx/src/filesystem/file_descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ inline perms posix_get_perms(const StatT& st) noexcept { return static_cast<perm
inline file_status create_file_status(error_code& m_ec, path const& p, const StatT& path_stat, error_code* ec) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not attached to this line: Can you add a release note in libcxx/docs/ReleaseNotes/20.rst?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (ec)
*ec = m_ec;
if (m_ec && (m_ec.value() == ENOENT || m_ec.value() == ENOTDIR)) {
if (m_ec && (m_ec == errc::no_such_file_or_directory || m_ec == errc::not_a_directory)) {
return file_status(file_type::not_found);
} else if (m_ec) {
ErrorHandler<void> err("posix_stat", ec, &p);
Expand Down Expand Up @@ -236,7 +236,7 @@ inline file_status create_file_status(error_code& m_ec, path const& p, const Sta
inline file_status posix_stat(path const& p, StatT& path_stat, error_code* ec) {
error_code m_ec;
if (detail::stat(p.c_str(), &path_stat) == -1)
m_ec = detail::capture_errno();
m_ec = detail::get_last_error();
return create_file_status(m_ec, p, path_stat, ec);
}

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

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

inline bool posix_fchmod(const FileDescriptor& fd, const StatT& st, error_code& ec) {
if (detail::fchmod(fd.fd, st.st_mode) == -1) {
ec = capture_errno();
ec = get_last_error();
return true;
}
ec.clear();
Expand All @@ -286,7 +286,7 @@ inline file_status FileDescriptor::refresh_status(error_code& ec) {
m_stat = {};
error_code m_ec;
if (detail::fstat(fd, &m_stat) == -1)
m_ec = capture_errno();
m_ec = get_last_error();
m_status = create_file_status(m_ec, name, m_stat, &ec);
return m_status;
}
Expand Down
Loading
Loading