[libcxx-commits] [libcxx] [libc++][hardening] Finish documenting hardening. (PR #92021)

[Konstantin Varlamov via libcxx-commits]

https://github.com/var-const updated https://github.com/llvm/llvm-project/pull/92021

>From 075384ed0a9f76df7e6159444869d8c478b6bd67 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Mon, 13 May 2024 13:10:19 -0700
Subject: [PATCH 1/6] [libc++][hardening] Finish documenting hardening.

---
 libcxx/docs/Hardening.rst       | 308 ++++++++++++++++++++++++++++++--
 libcxx/docs/ReleaseNotes/18.rst |   2 +-
 2 files changed, 299 insertions(+), 11 deletions(-)

diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index 0761f42368e92..d5a80f9840d11 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -1,4 +1,4 @@
-.. _hardening-modes:
+.. _hardening:
 
 ===============
 Hardening Modes
@@ -29,8 +29,11 @@ modes are:
   rigour impacts performance more than fast mode: we recommend benchmarking to
   determine if that is acceptable for your program.
 - **Debug mode**, which enables all the available checks in the library,
-  including internal assertions, some of which might be very expensive. This
-  mode is intended to be used for testing, not in production.
+  including heuristic checks that might have significant performance overhead as
+  well as internal library assertions. This mode should be used in
+  non-production environments (such as test suites, CI, local development). We
+  don’t commit to a particular level of performance in this mode and it’s *not*
+  intended to be used in production.
 
 .. note::
 
@@ -72,17 +75,302 @@ to control the level by passing **one** of the following options to the compiler
 Notes for vendors
 -----------------
 
-Vendors can set the default hardening mode by providing ``LIBCXX_HARDENING_MODE``
-as a configuration option, with the possible values of ``none``, ``fast``,
-``extensive`` and ``debug``. The default value is ``none`` which doesn't enable
-any hardening checks (this mode is sometimes called the ``unchecked`` mode).
+Vendors can set the default hardening mode by providing
+``LIBCXX_HARDENING_MODE`` as a configuration option, with the possible values of
+``none``, ``fast``, ``extensive`` and ``debug``. The default value is ``none``
+which doesn't enable any hardening checks (this mode is sometimes called the
+``unchecked`` mode).
 
 This option controls both the hardening mode that the precompiled library is
 built with and the default hardening mode that users will build with. If set to
 ``none``, the precompiled library will not contain any assertions, and user code
 will default to building without assertions.
 
-Iterator bounds checking
-------------------------
+Vendors can also override the termination handler by :ref:`providing a custom
+header <override-assertion-handler>`.
 
-TODO(hardening)
+Assertion categories
+====================
+
+Inside the library, individual assertions are grouped into different
+_categories_. Each hardening mode enables a different set of assertion
+categories; categories provide an additional layer of abstraction that makes it
+easier to reason about the high-level semantics of a hardening mode.
+
+- ``valid-element-access`` -- checks that any attempts to access a container
+  element, whether through the container object or through an iterator, are
+  valid and do not attempt to go out of bounds or otherwise access
+  a non-existent element. For iterator checks to work, bounded iterators must be
+  enabled in the ABI. Types like ``optional`` and ``function`` are considered
+  one-element containers for the purposes of this check.
+
+- ``valid-input-range`` -- checks that ranges (whether expressed as an iterator
+  pair, an iterator and a sentinel, an iterator and a count, or
+  a ``std::range``) given as input to library functions are valid:
+  - the sentinel is reachable from the begin iterator;
+  - TODO(hardening): both iterators refer to the same container.
+
+  ("input" here refers to "an input given to an algorithm", not to an iterator
+  category)
+
+  Violating assertions in this category leads to an out-of-bounds access.
+
+- ``non-null`` -- checks that the pointer being dereferenced is not null. On
+  most modern platforms zero address does not refer to an actual location in
+  memory, so a null pointer dereference would not compromize the memory security
+  of a program (however, it is still undefined behavior that can result in
+  strange errors due to compiler optimizations).
+
+- ``non-overlapping-ranges`` -- for functions that take several ranges as
+  arguments, checks that the given ranges do not overlap.
+
+- ``valid-deallocation`` -- checks that an attempt to deallocate memory is valid
+  (e.g. the given object was allocated by the given allocator). Violating this
+  category typically results in a memory leak.
+
+- ``valid-external-api-call`` -- checks that a call to an external API doesn't
+  fail in an unexpected manner. This includes triggering documented cases of
+  undefined behavior in an external library (like attempting to unlock an
+  unlocked mutex in pthreads). Any API external to the library falls under this
+  category (from system calls to compiler intrinsics). We generally don't expect
+  these failures to compromize memory safety or otherwise create an immediate
+  security issue.
+
+- ``compatible-allocator`` -- checks any operations that exchange nodes between
+  containers to make sure the containers have compatible allocators.
+
+- ``argument-within-domain`` -- checks that the given argument is within the
+  domain of valid arguments for the function. Violating this typically produces
+  an incorrect result (e.g. the clamp algorithm returns the original value
+  without clamping it due to incorrect functors) or puts an object into an
+  invalid state (e.g. a string view where only a subset of elements is possible
+  to access). This category is for assertions violating which doesn't cause any
+  immediate issues in the library -- whatever the consequences are, they will
+  happen in the user code.
+
+- ``pedantic`` -- checks prerequisites that are imposed by the Standard, but
+  violating which happens to be benign in our implementation.
+
+- ``semantic-requirement`` -- checks that the given argument satisfies the
+  semantic requirements imposed by the Standard. Typically, there is no simple
+  way to completely prove that a semantic requirement is satisfied; thus, this
+  would often be a heuristic check and it might be quite expensive.
+
+- ``internal`` -- checks that internal invariants of the library hold. These
+  assertions don't depend on user input.
+
+- ``uncategorized`` -- for assertions that haven't been properly classified yet.
+  This is an escape hatch used for some existing assertions in the library; all
+  new code should have its assertions properly classified.
+
+Mapping between the hardening modes and the assertion categories
+================================================================
+
+.. list-table::
+    :header-rows: 1
+    :widths: auto
+
+    * -
+      - ``fast``
+      - ``extensive``
+      - ``debug``
+    * - ``valid-element-access``
+      - ✅
+      - ✅
+      - ✅
+    * - ``valid-input-range``
+      - ✅
+      - ✅
+      - ✅
+    * - ``non-null``
+      - ❌
+      - ✅
+      - ✅
+    * - ``non-overlapping-ranges``
+      - ❌
+      - ✅
+      - ✅
+    * - ``valid-deallocation``
+      - ❌
+      - ✅
+      - ✅
+    * - ``valid-external-api-call``
+      - ❌
+      - ✅
+      - ✅
+    * - ``compatible-allocator``
+      - ❌
+      - ✅
+      - ✅
+    * - ``argument-within-domain``
+      - ❌
+      - ✅
+      - ✅
+    * - ``pedantic``
+      - ❌
+      - ✅
+      - ✅
+    * - ``semantic-requirement``
+      - ❌
+      - ❌
+      - ✅
+    * - ``internal``
+      - ❌
+      - ❌
+      - ✅
+    * - ``uncategorized``
+      - ❌
+      - ✅
+      - ✅
+
+.. note::
+
+  At the moment, each subsequent hardening mode is a strict superset of the
+  previous one (in other words, each subsequent mode only enables additional
+  assertion categories without disabling any), but this won't necessarily be
+  true for any hardening modes that might potentially be added in the future.
+
+Hardening assertion failure
+===========================
+
+In production modes (``fast`` and ``extensive``), a hardening assertion failure
+immediately traps the program. This is the safest approach that also minimizes
+the code size penalty as the failure handler maps to a single instruction. The
+downside is that the failure provides no additional details other than the stack
+trace (which might also be affected by optimizations).
+TODO(hardening): describe ``__builtin_verbose_trap`` once we can use it.
+
+In the ``debug`` mode, an assertion failure terminates the program in an
+unspecified manner and also outputs the associated error message to the error
+output. This is less secure and increases the size of the binary (among other
+things, to store the error message strings) but makes the failure easier to
+debug. It also allows us to test the error messages in our test suite.
+
+.. _override-assertion-handler:
+
+Overriding the assertion failure handler
+----------------------------------------
+
+Vendors can override the default termination handler mechanism by following
+these steps:
+
+- create a header file that provides a definition of a macro called
+  ``_LIBCPP_ASSERTION_HANDLER``. The macro will be invoked when a hardening
+  assertion fails, with a single parameter containing a null-terminated string
+  with the error message.
+- when configuring the library, provide the path to custom header (relative to
+  the root of the repository) via the CMake variable
+  ``LIBCXX_ASSERTION_HANDLER_FILE``.
+
+ABI
+===
+
+Setting a hardening mode does **not** affect the ABI. Each mode uses the subset
+of checks available in the current ABI configuration which is determined by the
+platform.
+
+It is important to stress that whether a particular check is enabled depends on
+the combination of the selected hardening mode and the hardening-related ABI
+options. Some checks require changing the ABI from the "default" to store
+additional information in the library classes -- e.g. checking whether an
+iterator is valid upon dereference generally requires storing data about bounds
+inside the iterator object. Using ``std::span`` as an example, setting the
+hardening mode to ``fast`` will always enable the ``valid-element-access``
+checks when accessing elements via a ``span`` object, but whether dereferencing
+a ``span`` iterator does the equivalent check depends on the ABI configuration.
+
+ABI options
+-----------
+
+Vendors can use the following ABI options to enable additional hardening checks:
+
+- ``_LIBCPP_ABI_BOUNDED_ITERATORS`` -- changes the iterator type of select
+  containers (see below) to a bounded iterator that keeps track of whether it's
+  within the bounds of the original container and asserts it on every
+  dereference.
+
+  ABI impact: changes the iterator type of the relevant containers.
+
+  Supported containers:
+  - ``span``;
+  - ``string_view``;
+  - ``array``.
+
+ABI tags
+--------
+
+We use ABI tags to allow translation units built with different hardening modes
+to interact with each other without causing ODR violations. Knowing how
+hardening modes are encoded into the ABI tags might be useful to examine
+a binary and determine whether it was built with hardening enabled.
+
+.. warning::
+  We don't commit to the ABI tags being stable between different releases of
+  libc++. The following describes the state of the latest release and is for
+  informational purposes only.
+
+The first character of an ABI tag encodes the hardening mode:
+- ``f`` -- [f]ast mode;
+- ``s`` -- extensive ("[s]afe") mode;
+- ``d`` -- [d]ebug mode;
+- ``n`` -- [n]one mode.
+
+Hardened containers
+===================
+
+.. list-table::
+    :header-rows: 1
+    :widths: auto
+
+    * - Name
+      - Member functions
+      - Iterators (ABI-dependent)
+    * - ``span``
+      - ✅
+      - ✅
+    * - ``string_view``
+      - ✅
+      - ✅
+    * - ``array``
+      - ✅
+      - ❌
+    * - ``vector``
+      - ✅
+      - ❌
+    * - ``string``
+      - ✅
+      - ❌
+    * - ``list``
+      - ✅
+      - ❌
+    * - ``forward_list``
+      - ✅
+      - ❌
+    * - ``deque``
+      - ✅
+      - ❌
+    * - ``mdspan``
+      - ✅
+      - ❌
+    * - ``optional``
+      - ✅
+      - N/A
+
+TODO(hardening): make this table exhaustive.
+
+Testing
+=======
+
+Each hardening assertion should be tested using death tests (via the
+``TEST_LIBCPP_ASSERT_FAILURE`` macro). Use the ``libcpp-hardening-mode`` Lit
+feature to make sure the assertion is enabled in (and only in) the intended
+modes. Note that error messages are only tested (matched) if the ``debug``
+hardening mode is used.
+
+Further reading
+===============
+
+- ``_Hardening RFC <https://discourse.llvm.org/t/rfc-hardening-in-libc/73925>``:
+  contains some of the design rationale.
+
+:ref:`hardening mode <using-hardening-modes>`
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index fcd630e09b449..4f7b9b362e5e6 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -40,7 +40,7 @@ and C++26 features.
 
 New hardened modes for the library have been added, replacing the legacy debug mode that was
 removed in the LLVM 17 release. Unlike the legacy debug mode, some of these hardening modes are
-also intended to be used in production. See :ref:`hardening-modes` for more details.
+also intended to be used in production. See :ref:`hardening` for more details.
 
 Work on the ranges support has progressed. See
 :ref:`ranges-status` for the current status.

>From 7c2030b44a541fbeaf3c83461bbc1316154f6bc2 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Mon, 13 May 2024 13:17:47 -0700
Subject: [PATCH 2/6] Small fixes

---
 libcxx/docs/Hardening.rst            | 17 ++++++++---------
 libcxx/include/__config              | 10 +++++-----
 libcxx/include/__configuration/abi.h |  3 +--
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index d5a80f9840d11..59ff8cd84248f 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -93,7 +93,7 @@ Assertion categories
 ====================
 
 Inside the library, individual assertions are grouped into different
-_categories_. Each hardening mode enables a different set of assertion
+*categories*. Each hardening mode enables a different set of assertion
 categories; categories provide an additional layer of abstraction that makes it
 easier to reason about the high-level semantics of a hardening mode.
 
@@ -170,7 +170,7 @@ Mapping between the hardening modes and the assertion categories
     :header-rows: 1
     :widths: auto
 
-    * -
+    * - Category name
       - ``fast``
       - ``extensive``
       - ``debug``
@@ -238,6 +238,7 @@ immediately traps the program. This is the safest approach that also minimizes
 the code size penalty as the failure handler maps to a single instruction. The
 downside is that the failure provides no additional details other than the stack
 trace (which might also be affected by optimizations).
+
 TODO(hardening): describe ``__builtin_verbose_trap`` once we can use it.
 
 In the ``debug`` mode, an assertion failure terminates the program in an
@@ -293,8 +294,7 @@ Vendors can use the following ABI options to enable additional hardening checks:
 
   Supported containers:
   - ``span``;
-  - ``string_view``;
-  - ``array``.
+  - ``string_view``.
 
 ABI tags
 --------
@@ -310,13 +310,14 @@ a binary and determine whether it was built with hardening enabled.
   informational purposes only.
 
 The first character of an ABI tag encodes the hardening mode:
+
 - ``f`` -- [f]ast mode;
 - ``s`` -- extensive ("[s]afe") mode;
 - ``d`` -- [d]ebug mode;
 - ``n`` -- [n]one mode.
 
-Hardened containers
-===================
+Hardened containers status
+==========================
 
 .. list-table::
     :header-rows: 1
@@ -344,7 +345,7 @@ Hardened containers
       - ✅
       - ❌
     * - ``forward_list``
-      - ✅
+      - ❌
       - ❌
     * - ``deque``
       - ✅
@@ -372,5 +373,3 @@ Further reading
 
 - ``_Hardening RFC <https://discourse.llvm.org/t/rfc-hardening-in-libc/73925>``:
   contains some of the design rationale.
-
-:ref:`hardening mode <using-hardening-modes>`
diff --git a/libcxx/include/__config b/libcxx/include/__config
index aac8c70c74a33..87ce1ca87ba33 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -583,11 +583,11 @@ typedef __char32_t char32_t;
 #  endif
 
 // TODO: Remove this workaround once we drop support for Clang 16
-#if __has_warning("-Wc++23-extensions")
-#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++23-extensions")
-#else
-#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++2b-extensions")
-#endif
+#  if __has_warning("-Wc++23-extensions")
+#    define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++23-extensions")
+#  else
+#    define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++2b-extensions")
+#  endif
 
 // Clang modules take a significant compile time hit when pushing and popping diagnostics.
 // Since all the headers are marked as system headers in the modulemap, we can simply disable this
diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h
index 17aceb042f524..73375fa47d07f 100644
--- a/libcxx/include/__configuration/abi.h
+++ b/libcxx/include/__configuration/abi.h
@@ -127,8 +127,7 @@
 //
 // Supported containers:
 // - `span`;
-// - `string_view`;
-// - `array`.
+// - `string_view`.
 // #define _LIBCPP_ABI_BOUNDED_ITERATORS
 
 #if defined(_LIBCPP_COMPILER_CLANG_BASED)

>From b2b46889ed84946d3de61dd58ead68930013c203 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Mon, 13 May 2024 13:20:35 -0700
Subject: [PATCH 3/6] Undo formatting

---
 libcxx/include/__config | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libcxx/include/__config b/libcxx/include/__config
index 87ce1ca87ba33..aac8c70c74a33 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -583,11 +583,11 @@ typedef __char32_t char32_t;
 #  endif
 
 // TODO: Remove this workaround once we drop support for Clang 16
-#  if __has_warning("-Wc++23-extensions")
-#    define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++23-extensions")
-#  else
-#    define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++2b-extensions")
-#  endif
+#if __has_warning("-Wc++23-extensions")
+#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++23-extensions")
+#else
+#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++2b-extensions")
+#endif
 
 // Clang modules take a significant compile time hit when pushing and popping diagnostics.
 // Since all the headers are marked as system headers in the modulemap, we can simply disable this

>From c95f9a7def9ceb978fae2cf44135653010000c40 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Wed, 15 May 2024 14:23:28 -0700
Subject: [PATCH 4/6] Address feedback

---
 libcxx/docs/Hardening.rst | 105 +++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 35 deletions(-)

diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index 59ff8cd84248f..96ea6477ea67f 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -31,9 +31,9 @@ modes are:
 - **Debug mode**, which enables all the available checks in the library,
   including heuristic checks that might have significant performance overhead as
   well as internal library assertions. This mode should be used in
-  non-production environments (such as test suites, CI, local development). We
-  don’t commit to a particular level of performance in this mode and it’s *not*
-  intended to be used in production.
+  non-production environments (such as test suites, CI, or local development).
+  We don’t commit to a particular level of performance in this mode and it’s
+  *not* intended to be used in production.
 
 .. note::
 
@@ -101,8 +101,8 @@ easier to reason about the high-level semantics of a hardening mode.
   element, whether through the container object or through an iterator, are
   valid and do not attempt to go out of bounds or otherwise access
   a non-existent element. For iterator checks to work, bounded iterators must be
-  enabled in the ABI. Types like ``optional`` and ``function`` are considered
-  one-element containers for the purposes of this check.
+  enabled in the ABI. Types like ``std::optional`` and ``std::function`` are
+  considered one-element containers for the purposes of this check.
 
 - ``valid-input-range`` -- checks that ranges (whether expressed as an iterator
   pair, an iterator and a sentinel, an iterator and a count, or
@@ -116,13 +116,13 @@ easier to reason about the high-level semantics of a hardening mode.
   Violating assertions in this category leads to an out-of-bounds access.
 
 - ``non-null`` -- checks that the pointer being dereferenced is not null. On
-  most modern platforms zero address does not refer to an actual location in
-  memory, so a null pointer dereference would not compromize the memory security
-  of a program (however, it is still undefined behavior that can result in
-  strange errors due to compiler optimizations).
+  most modern platforms, the zero address does not refer to an actual location
+  in memory, so a null pointer dereference would not compromise the memory
+  security of a program (however, it is still undefined behavior that can result
+  in strange errors due to compiler optimizations).
 
 - ``non-overlapping-ranges`` -- for functions that take several ranges as
-  arguments, checks that the given ranges do not overlap.
+  arguments, checks that those ranges do not overlap.
 
 - ``valid-deallocation`` -- checks that an attempt to deallocate memory is valid
   (e.g. the given object was allocated by the given allocator). Violating this
@@ -133,7 +133,7 @@ easier to reason about the high-level semantics of a hardening mode.
   undefined behavior in an external library (like attempting to unlock an
   unlocked mutex in pthreads). Any API external to the library falls under this
   category (from system calls to compiler intrinsics). We generally don't expect
-  these failures to compromize memory safety or otherwise create an immediate
+  these failures to compromise memory safety or otherwise create an immediate
   security issue.
 
 - ``compatible-allocator`` -- checks any operations that exchange nodes between
@@ -141,14 +141,14 @@ easier to reason about the high-level semantics of a hardening mode.
 
 - ``argument-within-domain`` -- checks that the given argument is within the
   domain of valid arguments for the function. Violating this typically produces
-  an incorrect result (e.g. the clamp algorithm returns the original value
-  without clamping it due to incorrect functors) or puts an object into an
-  invalid state (e.g. a string view where only a subset of elements is possible
-  to access). This category is for assertions violating which doesn't cause any
-  immediate issues in the library -- whatever the consequences are, they will
-  happen in the user code.
-
-- ``pedantic`` -- checks prerequisites that are imposed by the Standard, but
+  an incorrect result (e.g. ``std::clamp`` returns the original value without
+  clamping it due to incorrect functors) or puts an object into an invalid state
+  (e.g. a string view where only a subset of elements is accessible). This
+  category is for assertions violating which doesn't cause any immediate issues
+  in the library -- whatever the consequences are, they will happen in the user
+  code.
+
+- ``pedantic`` -- checks preconditions that are imposed by the Standard, but
   violating which happens to be benign in our implementation.
 
 - ``semantic-requirement`` -- checks that the given argument satisfies the
@@ -160,8 +160,8 @@ easier to reason about the high-level semantics of a hardening mode.
   assertions don't depend on user input.
 
 - ``uncategorized`` -- for assertions that haven't been properly classified yet.
-  This is an escape hatch used for some existing assertions in the library; all
-  new code should have its assertions properly classified.
+  This category is an escape hatch used for some existing assertions in the
+  library; all new code should have its assertions properly classified.
 
 Mapping between the hardening modes and the assertion categories
 ================================================================
@@ -228,24 +228,25 @@ Mapping between the hardening modes and the assertion categories
   At the moment, each subsequent hardening mode is a strict superset of the
   previous one (in other words, each subsequent mode only enables additional
   assertion categories without disabling any), but this won't necessarily be
-  true for any hardening modes that might potentially be added in the future.
+  true for any hardening modes that might be added in the future.
 
 Hardening assertion failure
 ===========================
 
 In production modes (``fast`` and ``extensive``), a hardening assertion failure
-immediately traps the program. This is the safest approach that also minimizes
-the code size penalty as the failure handler maps to a single instruction. The
-downside is that the failure provides no additional details other than the stack
-trace (which might also be affected by optimizations).
+immediately ``_traps <https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic>``
+the program. This is the safest approach that also minimizes the code size
+penalty as the failure handler maps to a single instruction. The downside is
+that the failure provides no additional details other than the stack trace
+(which might also be affected by optimizations).
 
 TODO(hardening): describe ``__builtin_verbose_trap`` once we can use it.
 
 In the ``debug`` mode, an assertion failure terminates the program in an
 unspecified manner and also outputs the associated error message to the error
 output. This is less secure and increases the size of the binary (among other
-things, to store the error message strings) but makes the failure easier to
-debug. It also allows us to test the error messages in our test suite.
+things, it has to store the error message strings) but makes the failure easier
+to debug. It also allows us to test the error messages in our test suite.
 
 .. _override-assertion-handler:
 
@@ -277,8 +278,9 @@ additional information in the library classes -- e.g. checking whether an
 iterator is valid upon dereference generally requires storing data about bounds
 inside the iterator object. Using ``std::span`` as an example, setting the
 hardening mode to ``fast`` will always enable the ``valid-element-access``
-checks when accessing elements via a ``span`` object, but whether dereferencing
-a ``span`` iterator does the equivalent check depends on the ABI configuration.
+checks when accessing elements via a ``std::span`` object, but whether
+dereferencing a ``std::span`` iterator does the equivalent check depends on the
+ABI configuration.
 
 ABI options
 -----------
@@ -287,7 +289,7 @@ Vendors can use the following ABI options to enable additional hardening checks:
 
 - ``_LIBCPP_ABI_BOUNDED_ITERATORS`` -- changes the iterator type of select
   containers (see below) to a bounded iterator that keeps track of whether it's
-  within the bounds of the original container and asserts it on every
+  within the bounds of the original container and asserts valid bounds on every
   dereference.
 
   ABI impact: changes the iterator type of the relevant containers.
@@ -305,9 +307,10 @@ hardening modes are encoded into the ABI tags might be useful to examine
 a binary and determine whether it was built with hardening enabled.
 
 .. warning::
-  We don't commit to the ABI tags being stable between different releases of
-  libc++. The following describes the state of the latest release and is for
-  informational purposes only.
+  We don't commit to the encoding scheme used by the ABI tags being stable
+  between different releases of libc++. The tags themselves are never stable, by
+  design -- new releases increase the version number. The following describes
+  the state of the latest release and is for informational purposes only.
 
 The first character of an ABI tag encodes the hardening mode:
 
@@ -365,7 +368,39 @@ Testing
 Each hardening assertion should be tested using death tests (via the
 ``TEST_LIBCPP_ASSERT_FAILURE`` macro). Use the ``libcpp-hardening-mode`` Lit
 feature to make sure the assertion is enabled in (and only in) the intended
-modes. Note that error messages are only tested (matched) if the ``debug``
+modes. The convention is to use `assert.` in the name of the test file to make
+it easier to identify as a hardening test, e.g. ``assert.my_func.pass.cpp``.
+A toy example:
+
+.. code-block:: cpp
+
+  // Note: the following three annotations are currently needed to use the
+  // `TEST_LIBCPP_ASSERT_FAILURE`.
+  // REQUIRES: has-unix-headers
+  // UNSUPPORTED: c++03
+  // XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+  // Example: only run this test in `fast`/`extensive`/`debug` modes.
+  // UNSUPPORTED: libcpp-hardening-mode=none
+  // Example: only run this test in the `debug` mode.
+  // REQUIRES: libcpp-hardening-mode=debug
+  // Example: only run this test in `extensive`/`debug` modes.
+  // REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+
+  #include <header_being_tested>
+
+  #include "check_assertion.h" // Contains the `TEST_LIBCPP_ASSERT_FAILURE` macro
+
+  int main(int, char**) {
+    std::type_being_tested foo;
+    int bad_input = -1;
+    TEST_LIBCPP_ASSERT_FAILURE(foo.some_function_that_asserts(bad_input),
+        "The expected assertion message");
+
+    return 0;
+  }
+
+Note that error messages are only tested (matched) if the ``debug``
 hardening mode is used.
 
 Further reading

>From c2442a36468e580f952fe9c1b1bb36e8ebb49ae5 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Wed, 22 May 2024 16:05:46 -0700
Subject: [PATCH 5/6] Feedback

---
 libcxx/docs/Hardening.rst | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index 96ea6477ea67f..9a36778e3512f 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -100,9 +100,11 @@ easier to reason about the high-level semantics of a hardening mode.
 - ``valid-element-access`` -- checks that any attempts to access a container
   element, whether through the container object or through an iterator, are
   valid and do not attempt to go out of bounds or otherwise access
-  a non-existent element. For iterator checks to work, bounded iterators must be
-  enabled in the ABI. Types like ``std::optional`` and ``std::function`` are
-  considered one-element containers for the purposes of this check.
+  a non-existent element. This also includes operations that set up an imminent
+  invalid access (e.g. incrementing an end iterator). For iterator checks to
+  work, bounded iterators must be enabled in the ABI. Types like
+  ``std::optional`` and ``std::function`` are considered containers (with at
+  most one element) for the purposes of this check.
 
 - ``valid-input-range`` -- checks that ranges (whether expressed as an iterator
   pair, an iterator and a sentinel, an iterator and a count, or
@@ -264,6 +266,8 @@ these steps:
   the root of the repository) via the CMake variable
   ``LIBCXX_ASSERTION_HANDLER_FILE``.
 
+There is no existing mechanism for users to override the termination handler.
+
 ABI
 ===
 

>From b211decee2e502434537db5fc3b7ba7d318b7f68 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Tue, 4 Jun 2024 18:05:16 -0700
Subject: [PATCH 6/6] Address feedback

---
 libcxx/docs/Hardening.rst     | 74 ++++++++++++++---------------------
 libcxx/docs/TestingLibcxx.rst | 45 +++++++++++++++++++++
 2 files changed, 75 insertions(+), 44 deletions(-)

diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index 9a36778e3512f..b0d15b7e050e3 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -86,8 +86,8 @@ built with and the default hardening mode that users will build with. If set to
 ``none``, the precompiled library will not contain any assertions, and user code
 will default to building without assertions.
 
-Vendors can also override the termination handler by :ref:`providing a custom
-header <override-assertion-handler>`.
+Vendors can also override the way the program is terminated when an assertion
+fails by :ref:`providing a custom header <override-assertion-handler>`.
 
 Assertion categories
 ====================
@@ -97,6 +97,11 @@ Inside the library, individual assertions are grouped into different
 categories; categories provide an additional layer of abstraction that makes it
 easier to reason about the high-level semantics of a hardening mode.
 
+.. note::
+
+  Users are not intended to interact with these categories directly -- the
+  categories are considered internal to the library and subject to change.
+
 - ``valid-element-access`` -- checks that any attempts to access a container
   element, whether through the container object or through an iterator, are
   valid and do not attempt to go out of bounds or otherwise access
@@ -151,7 +156,7 @@ easier to reason about the high-level semantics of a hardening mode.
   code.
 
 - ``pedantic`` -- checks preconditions that are imposed by the Standard, but
-  violating which happens to be benign in our implementation.
+  violating which happens to be benign in libc++.
 
 - ``semantic-requirement`` -- checks that the given argument satisfies the
   semantic requirements imposed by the Standard. Typically, there is no simple
@@ -232,6 +237,13 @@ Mapping between the hardening modes and the assertion categories
   assertion categories without disabling any), but this won't necessarily be
   true for any hardening modes that might be added in the future.
 
+.. note::
+
+  The categories enabled by each mode are subject to change and users should not
+  rely on the precise assertions enabled by a mode at a given point in time.
+  However, the library does guarantee to keep the hardening modes stable and
+  to fulfill the semantics documented here.
+
 Hardening assertion failure
 ===========================
 
@@ -248,15 +260,15 @@ In the ``debug`` mode, an assertion failure terminates the program in an
 unspecified manner and also outputs the associated error message to the error
 output. This is less secure and increases the size of the binary (among other
 things, it has to store the error message strings) but makes the failure easier
-to debug. It also allows us to test the error messages in our test suite.
+to debug. It also allows testing the error messages in our test suite.
 
 .. _override-assertion-handler:
 
 Overriding the assertion failure handler
 ----------------------------------------
 
-Vendors can override the default termination handler mechanism by following
-these steps:
+Vendors can override the default assertion handler mechanism by following these
+steps:
 
 - create a header file that provides a definition of a macro called
   ``_LIBCPP_ASSERTION_HANDLER``. The macro will be invoked when a hardening
@@ -266,7 +278,17 @@ these steps:
   the root of the repository) via the CMake variable
   ``LIBCXX_ASSERTION_HANDLER_FILE``.
 
-There is no existing mechanism for users to override the termination handler.
+Note that almost all libc++ headers include the assertion handler header which
+means it should not include anything from the standard library to avoid creating
+circular dependencies.
+
+There is no existing mechanism for users to override the assertion handler
+because the ability to do the override other than at configure-time carries an
+unavoidable code size penalty that would otherwise be imposed on all users,
+whether they require such customization or not. Instead, we let vendors decide
+what's right on their platform for their users -- a vendor who wishes to provide
+this capability is free to do so, e.g. by declaring the assertion handler as an
+overridable function.
 
 ABI
 ===
@@ -369,43 +391,7 @@ TODO(hardening): make this table exhaustive.
 Testing
 =======
 
-Each hardening assertion should be tested using death tests (via the
-``TEST_LIBCPP_ASSERT_FAILURE`` macro). Use the ``libcpp-hardening-mode`` Lit
-feature to make sure the assertion is enabled in (and only in) the intended
-modes. The convention is to use `assert.` in the name of the test file to make
-it easier to identify as a hardening test, e.g. ``assert.my_func.pass.cpp``.
-A toy example:
-
-.. code-block:: cpp
-
-  // Note: the following three annotations are currently needed to use the
-  // `TEST_LIBCPP_ASSERT_FAILURE`.
-  // REQUIRES: has-unix-headers
-  // UNSUPPORTED: c++03
-  // XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
-
-  // Example: only run this test in `fast`/`extensive`/`debug` modes.
-  // UNSUPPORTED: libcpp-hardening-mode=none
-  // Example: only run this test in the `debug` mode.
-  // REQUIRES: libcpp-hardening-mode=debug
-  // Example: only run this test in `extensive`/`debug` modes.
-  // REQUIRES: libcpp-hardening-mode={{extensive|debug}}
-
-  #include <header_being_tested>
-
-  #include "check_assertion.h" // Contains the `TEST_LIBCPP_ASSERT_FAILURE` macro
-
-  int main(int, char**) {
-    std::type_being_tested foo;
-    int bad_input = -1;
-    TEST_LIBCPP_ASSERT_FAILURE(foo.some_function_that_asserts(bad_input),
-        "The expected assertion message");
-
-    return 0;
-  }
-
-Note that error messages are only tested (matched) if the ``debug``
-hardening mode is used.
+Please see :ref:`Testing documentation <testing-hardening-assertions>`.
 
 Further reading
 ===============
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 50ee9d4ee400b..d9f4fe467fe36 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -480,3 +480,48 @@ For example:
   $ ./algorithms.libcxx.out --benchmark_filter=BM_Sort.* # Only runs the sort benchmarks
 
 For more information about running benchmarks see `Google Benchmark`_.
+
+
+.. _testing-hardening-assertions:
+
+Testing hardening assertions
+============================
+
+Each hardening assertion should be tested using death tests (via the
+``TEST_LIBCPP_ASSERT_FAILURE`` macro). Use the ``libcpp-hardening-mode`` Lit
+feature to make sure the assertion is enabled in (and only in) the intended
+modes. The convention is to use `assert.` in the name of the test file to make
+it easier to identify as a hardening test, e.g. ``assert.my_func.pass.cpp``.
+A toy example:
+
+.. code-block:: cpp
+
+  // Note: the following three annotations are currently needed to use the
+  // `TEST_LIBCPP_ASSERT_FAILURE`.
+  // REQUIRES: has-unix-headers
+  // UNSUPPORTED: c++03
+  // XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+  // Example: only run this test in `fast`/`extensive`/`debug` modes.
+  // UNSUPPORTED: libcpp-hardening-mode=none
+  // Example: only run this test in the `debug` mode.
+  // REQUIRES: libcpp-hardening-mode=debug
+  // Example: only run this test in `extensive`/`debug` modes.
+  // REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+
+  #include <header_being_tested>
+
+  #include "check_assertion.h" // Contains the `TEST_LIBCPP_ASSERT_FAILURE` macro
+
+  int main(int, char**) {
+    std::type_being_tested foo;
+    int bad_input = -1;
+    TEST_LIBCPP_ASSERT_FAILURE(foo.some_function_that_asserts(bad_input),
+        "The expected assertion message");
+
+    return 0;
+  }
+
+Note that error messages are only tested (matched) if the ``debug``
+hardening mode is used.
+