This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use _LIBCPP_VERBOSE_ABORT in a few remaining __throw_FOO functions
ClosedPublic

Authored by ldionne on Jul 11 2023, 11:42 AM.

Details

Summary

This provides better error messages when the program terminates due to
an exception being thrown in -fno-exceptions mode. Those seem to have
been missed in https://reviews.llvm.org/D141222.

Diff Detail

Event Timeline

ldionne created this revision.Jul 11 2023, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 11:42 AM
ldionne requested review of this revision.Jul 11 2023, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 11:42 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne updated this revision to Diff 539229.Jul 11 2023, 12:02 PM

Adjust error messages.

ldionne updated this revision to Diff 539714.Jul 12 2023, 1:35 PM

Rebase and poke CI.

philnik accepted this revision.Jul 12 2023, 1:41 PM
philnik added a subscriber: philnik.
philnik added inline comments.
libcxx/src/system_error.cpp
288

Maybe we should add an overload for std::error_code and replace uses, so we can use error_code::message() for a nicer error message.

This revision is now accepted and ready to land.Jul 12 2023, 1:41 PM
Mordante accepted this revision.Jul 12 2023, 2:07 PM
Mordante added a subscriber: Mordante.

LGTM modulo one nit.

libcxx/include/__format/format_error.h
15

I assume this header is no longer needed. At least for format and the dylib we can remove them unconditionally.

ldionne marked an inline comment as done.Jul 12 2023, 3:40 PM
ldionne added inline comments.
libcxx/src/system_error.cpp
288

Would you put that into the dylib? So you'd do this from uses:

if (__m_ == nullptr)
    __throw_system_error(std::make_error_code(std::errc::operation_not_permitted), "unique_lock::lock: references null mutex");

// instead of

if (__m_ == nullptr)
    __throw_system_error(EPERM, "unique_lock::lock: references null mutex");

and then the overload would look like this?

_LIBCPP_NORETURN void __throw_system_error(error_code const& __code, char const* __what_arg) {
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
    throw system_error(__code, what_arg);
#else
    _LIBCPP_VERBOSE_ABORT("system_error was thrown in -fno-exceptions mode with error_code \"%s\" and message \"%s\"", __code.message(), what_arg);
#endif
}
ldionne updated this revision to Diff 539774.Jul 12 2023, 3:43 PM

Address comment.

philnik added inline comments.Jul 12 2023, 3:43 PM
libcxx/src/system_error.cpp
288

Yeah, something like that. I wouldn't bother putting it in the dylib though. I don't even know why the have __throw_system_error in the dylib.

ldionne updated this revision to Diff 541017.Jul 17 2023, 7:21 AM

Adjust transitive includes