Skip to content

WIP [libc++][hardening] Overhaul the termination mechanism for hardening #77823

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

WIP [libc++][hardening] Overhaul the termination mechanism for hardening #77823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions libcxx/include/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,7 @@ set(files
__utility/unreachable.h
__variant/monostate.h
__verbose_abort
__verbose_trap
algorithm
any
array
Expand Down
25 changes: 24 additions & 1 deletion libcxx/include/__assert
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,40 @@

#include <__config>
#include <__verbose_abort>
#include <__verbose_trap>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

// _LIBCPP_ASSERT_IMPL

#if defined(_LIBCPP_VERBOSE_TRAP_IMPL_OVERRIDE)

#define _LIBCPP_ASSERT_IMPL(error_message, ...) _LIBCPP_VERBOSE_TRAP_IMPL_OVERRIDE(__VA_ARGS__)

#elif _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG

#define _LIBCPP_ASSERT_IMPL(error_message, ...) ((void)error_message, _LIBCPP_VERBOSE_ABORT(__VA_ARGS__))

#else

void __use(const char*, ...);
#define _LIBCPP_ASSERT_IMPL(error_message, ...) (decltype(__use(__VA_ARGS__))(), _LIBCPP_VERBOSE_TRAP(error_message))

#endif

// _LIBCPP_ASSERT
Copy link
Member

Choose a reason for hiding this comment

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

We should add a bit of documentation here that explains that _LIBCPP_ASSERT(expr, msg) is a no-op if expr is true, and it's library-level UB in case expr is false.


#define _LIBCPP_ASSERT(expression, message) \
(__builtin_expect(static_cast<bool>(expression), 1) \
? (void)0 \
: _LIBCPP_VERBOSE_ABORT( \
: _LIBCPP_ASSERT_IMPL( \
message, \
"%s:%d: assertion %s failed: %s\n", __builtin_FILE(), __builtin_LINE(), #expression, message))

// _LIBCPP_ASSUME

// TODO: __builtin_assume can currently inhibit optimizations. Until this has been fixed and we can add
// assumptions without a clear optimization intent, disable that to avoid worsening the code generation.
// See https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 for a discussion.
Expand Down
1 change: 1 addition & 0 deletions libcxx/include/__config_site.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

// Hardening.
#cmakedefine _LIBCPP_HARDENING_MODE_DEFAULT @_LIBCPP_HARDENING_MODE_DEFAULT@
#cmakedefine _LIBCPP_VERBOSE_TRAP_IMPL_OVERRIDE(...) @_LIBCPP_VERBOSE_TRAP_IMPL_OVERRIDE@

// __USE_MINGW_ANSI_STDIO gets redefined on MinGW
#ifdef __clang__
Expand Down
23 changes: 23 additions & 0 deletions libcxx/include/__verbose_trap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// -*- C++ -*-
Copy link
Member

Choose a reason for hiding this comment

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

Per our live review just now, we explored the following alternatives (using chrome as an example since we know they override verbose abort right now):

Option #1
=======================
$ cmake ... -DLIBCXX_VERBOSE_TRAP_OVERRIDE="void __chromium_trap(char const*);\n#define _LIBCPP_VERBOSE_TRAP(message) __chromium_trap(message)"

// __verbose_trap.in
#ifndef _LIBCPP___VERBOSE_TRAP
#define _LIBCPP___VERBOSE_TRAP

#include <__availability>
#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#  pragma GCC system_header
#endif

#if @LIBCXX_VERBOSE_TRAP_OVERRIDE@
    @LIBCXX_VERBOSE_TRAP_OVERRIDE@
#elif HARDENING_MODE == DEBUG
#   define _LIBCPP_VERBOSE_TRAP(message) ::verbose_abort(...)
#else
#   define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
#endif

#endif // _LIBCPP___VERBOSE_TRAP



Option #2
=======================
$ cmake ... -DLIBCXX_VERBOSE_TRAP_OVERRIDE=libcxx/vendor/chrome/verbose_trap.in

// __verbose_trap.in
#ifndef _LIBCPP___VERBOSE_TRAP
#define _LIBCPP___VERBOSE_TRAP

#include <__availability>
#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#  pragma GCC system_header
#endif

#if @LIBCXX_VERBOSE_TRAP_OVERRIDE@
    copy-paste the contents of @LIBCXX_VERBOSE_TRAP_OVERRIDE@ at CMake configuration time
#elif HARDENING_MODE == DEBUG
#   define _LIBCPP_VERBOSE_TRAP(message) ::verbose_abort(...)
#else
#   define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
#endif

#endif // _LIBCPP___VERBOSE_TRAP


// in libcxx/vendor/chrome/verbose_trap.in
#define _LIBCPP_VERBOSE_TRAP(message) ::whatever()

Copy link
Member

Choose a reason for hiding this comment

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

Further discussion: Since we may want to have use cases where the "verbose trap handler" doesn't actually abort the program (effectively deciding to ignore library-level UB), it would probably make sense to rename this. Suggestions:

  • _LIBCPP_LIBRARY_UB_HANDLER(message)
  • _LIBCPP_UB_HANDLER(message)
  • _LIBCPP_ASSERTION_HANDLER(message)
  • _LIBCPP_ASSERTION_FAILURE(message)

There's probably more. The point is that if we use one of these names, using an implementation where don't actually terminate the program is now more natural.

Copy link
Member

Choose a reason for hiding this comment

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

More discussion!

We could actually always use the overriding mechanism but provide our default definition for the assertion handler. That way, the code path for customizing the handler would always be tested, by definition. Something like:

// in CMakeLists.txt
set(LIBCXX_ASSERTION_HANDLER "libcxx/vendor/llvm/assertion_handler.in" CACHE STR)

// in libcxx/include/__assertion_handler.in
#ifndef _LIBCPP___ASSERTION_HANDLER
#define _LIBCPP___ASSERTION_HANDLER

#include <__availability>
#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#  pragma GCC system_header
#endif

copy-paste the contents of @LIBCXX_ASSERTION_HANDLER@ at CMake configuration time

#endif // _LIBCPP___ASSERTION_HANDLER

// in libcxx/vendor/llvm/assertion_handler.in
#if HARDENING_MODE == DEBUG
#   define _LIBCPP_VERBOSE_TRAP(message) ::std::__libcpp_verbose_abort(...)
#else
#   define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
#endif

//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___VERBOSE_TRAP
#define _LIBCPP___VERBOSE_TRAP

#include <__availability>
#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

// TODO: use `__builtin_verbose_trap(message) once available
#define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())

#endif // _LIBCPP___VERBOSE_TRAP
4 changes: 4 additions & 0 deletions libcxx/include/module.modulemap.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,10 @@ module std_private_verbose_abort [system] {
header "__verbose_abort"
export *
}
module std_private_verbose_trap [system] {
header "__verbose_trap"
export *
}

module std_private_algorithm_adjacent_find [system] { header "__algorithm/adjacent_find.h" }
module std_private_algorithm_all_of [system] { header "__algorithm/all_of.h" }
Expand Down
2 changes: 2 additions & 0 deletions libcxx/utils/generate_iwyu_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ def generate_map(include):
continue
elif i == "__verbose_abort":
continue
elif i == "__verbose_trap":
continue
else:
panic(i)

Expand Down