[libc++] Make the Debug mode a configuration-time only option

The debug mode has been broken pretty much ever since it was shipped because it was possible to enable the debug mode in user code without actually enabling it in the dylib, leading to ODR violations that caused various kinds of failures. This commit makes the debug mode a knob that is configured when building the library and which can't be changed afterwards. This is less flexible for users, however it will actually work as intended and it will allow us, in the future, to add various kinds of checks that do not assume the same ABI as the normal library. Furthermore, this will make the debug mode more robust, which means that vendors might be more tempted to support it properly, which hasn't been the case with the current debug mode. This patch shouldn't break any user code, except folks who are building against a library that doesn't have the debug mode enabled and who try to enable the debug mode in their code. Such users will get a compile-time error explaining that this configuration isn't supported anymore. In the future, we should further increase the granularity of the debug mode checks so that we can cherry-pick which checks to enable, like we do for unspecified behavior randomization. Differential Revision: https://reviews.llvm.org/D122941
intel · Jun 7, 2022 · f3966ea · f3966ea
1 parent 89c4b29
commit f3966ea
Show file tree

Hide file tree

Showing 122 changed files with 319 additions and 409 deletions.
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
@@ -68,11 +68,11 @@ option(LIBCXX_ENABLE_FILESYSTEM "Build filesystem as part of the main libc++ lib
     ${ENABLE_FILESYSTEM_DEFAULT})
 option(LIBCXX_INCLUDE_TESTS "Build the libc++ tests." ${LLVM_INCLUDE_TESTS})
 option(LIBCXX_ENABLE_PARALLEL_ALGORITHMS "Enable the parallel algorithms library. This requires the PSTL to be available." OFF)
-option(LIBCXX_ENABLE_DEBUG_MODE_SUPPORT
-  "Whether to include support for libc++'s debugging mode in the library.
-   By default, this is turned on. If you turn it off and try to enable the
-   debug mode when compiling a program against libc++, it will fail to link
-   since the required support isn't provided in the library." ON)
+option(LIBCXX_ENABLE_DEBUG_MODE
+  "Whether to build libc++ with the debug mode enabled.
+   By default, this is turned off. Turning it on results in a different ABI (additional
+   symbols but also potentially different layouts of types), and one should not mix code
+   built against a dylib that has debug mode and code built against a regular dylib." OFF)
 option(LIBCXX_ENABLE_RANDOM_DEVICE
   "Whether to include support for std::random_device in the library. Disabling
    this can be useful when building the library for platforms that don't have
@@ -210,6 +210,13 @@ set(LIBCXX_ABI_DEFINES "" CACHE STRING "A semicolon separated list of ABI macros
 option(LIBCXX_EXTRA_SITE_DEFINES "Extra defines to add into __config_site")
 option(LIBCXX_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF)
 
+cmake_dependent_option(LIBCXX_ENABLE_BACKWARDS_COMPATIBILITY_DEBUG_MODE_SYMBOLS
+  "Whether to include the old Debug mode symbols in the compiled library. This
+   is provided for backwards compatibility since the compiled library used to
+   always contain those symbols, regardless of whether the library was built
+   with the debug mode enabled." ON
+   "LIBCXX_ABI_VERSION EQUAL 1" OFF) # Always OFF in ABI version != 1
+
 # ABI Library options ---------------------------------------------------------
 if (LIBCXX_TARGETING_MSVC)
   set(LIBCXX_DEFAULT_ABI_LIBRARY "vcruntime")
@@ -854,6 +861,7 @@ config_define_if_not(LIBCXX_ENABLE_LOCALIZATION _LIBCPP_HAS_NO_LOCALIZATION)
 config_define_if_not(LIBCXX_ENABLE_UNICODE _LIBCPP_HAS_NO_UNICODE)
 config_define_if_not(LIBCXX_ENABLE_WIDE_CHARACTERS _LIBCPP_HAS_NO_WIDE_CHARACTERS)
 config_define_if_not(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS)
+config_define_if(LIBCXX_ENABLE_DEBUG_MODE _LIBCPP_ENABLE_DEBUG_MODE)
 if (LIBCXX_ENABLE_ASSERTIONS)
   config_define(1 _LIBCPP_ENABLE_ASSERTIONS_DEFAULT)
 else()

diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
@@ -9,7 +9,7 @@ set(LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
 set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
 set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
 set(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_DEBUG_MODE_SUPPORT OFF CACHE BOOL "")
+set(LIBCXX_ENABLE_BACKWARDS_COMPATIBILITY_DEBUG_MODE_SYMBOLS OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS ON CACHE BOOL "")
 set(LIBCXX_ENABLE_INCOMPLETE_FEATURES OFF CACHE BOOL "")
 

diff --git a/libcxx/cmake/caches/Generic-debug-iterators.cmake b/libcxx/cmake/caches/Generic-debug-iterators.cmake
diff --git a/libcxx/cmake/caches/Generic-debug-mode.cmake b/libcxx/cmake/caches/Generic-debug-mode.cmake
@@ -0,0 +1 @@
+set(LIBCXX_ENABLE_DEBUG_MODE ON CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-debug.cmake b/libcxx/cmake/caches/Generic-no-debug.cmake
diff --git a/libcxx/docs/DesignDocs/DebugMode.rst b/libcxx/docs/DesignDocs/DebugMode.rst
@@ -12,35 +12,33 @@ Using the debug mode
 
 Libc++ provides a debug mode that enables special debugging checks meant to detect
 incorrect usage of the standard library. These checks are disabled by default, but
-they can be enabled using the ``_LIBCPP_DEBUG`` macro.
+they can be enabled by vendors when building the library by using ``LIBCXX_ENABLE_DEBUG_MODE``.
 
-Note that using the debug mode discussed in this document requires that the library
-has been compiled with support for the debug mode (see ``LIBCXX_ENABLE_DEBUG_MODE_SUPPORT``).
+Since the debug mode has ABI implications, users should compile their whole program,
+including any dependent libraries, against a Standard library configured identically
+with respect to the debug mode. In other words, they should not mix code built against
+a Standard library with the debug mode enabled with code built against a Standard library
+where the debug mode is disabled.
 
-Also note that while the debug mode has no effect on libc++'s ABI, it does have broad ODR
-implications. Users should compile their whole program at the same debugging level.
+Furthermore, users should not rely on a stable ABI being provided when the debug mode is
+enabled -- we reserve the right to change the ABI at any time. If you need a stable ABI
+and still want some level of hardening, you should look into enabling :ref:`assertions <assertions-mode>`
+instead.
 
-The various levels of checking provided by the debug mode follow.
+The debug mode provides various checks to aid application debugging.
 
-No debugging checks (``_LIBCPP_DEBUG`` not defined)
----------------------------------------------------
-When ``_LIBCPP_DEBUG`` is not defined, there are no debugging checks performed by
-the library. This is the default.
-
-Comparator consistency checks (``_LIBCPP_DEBUG == 1``)
-------------------------------------------------------
+Comparator consistency checks
+-----------------------------
 Libc++ provides some checks for the consistency of comparators passed to algorithms. Specifically,
 many algorithms such as ``binary_search``, ``merge``, ``next_permutation``, and ``sort``, wrap the
 user-provided comparator to assert that `!comp(y, x)` whenever `comp(x, y)`. This can cause the
 user-provided comparator to be evaluated up to twice as many times as it would be without the
 debug mode, and causes the library to violate some of the Standard's complexity clauses.
 
-Iterator debugging checks (``_LIBCPP_DEBUG == 1``)
---------------------------------------------------
-Defining ``_LIBCPP_DEBUG`` to ``1`` enables "iterator debugging", which provides
-additional assertions about the validity of iterators used by the program.
-
-The following containers and classes support iterator debugging:
+Iterator debugging checks
+-------------------------
+The library contains various assertions to check the validity of iterators used
+by the program. The following containers and classes support iterator debugging:
 
 - ``std::string``
 - ``std::vector<T>`` (``T != bool``)
@@ -53,34 +51,11 @@ The following containers and classes support iterator debugging:
 The remaining containers do not currently support iterator debugging.
 Patches welcome.
 
-Randomizing Unspecified Behavior (``_LIBCPP_DEBUG == 1``)
----------------------------------------------------------
-This also enables the randomization of unspecified behavior, for
-example, for equal elements in ``std::sort`` or randomizing both parts of
-the partition after ``std::nth_element`` call. This effort helps you to migrate
-to potential future faster versions of these algorithms and deflake your tests
-which depend on such behavior. To fix the seed, use
-``_LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY_SEED=seed`` definition.
-
-Handling Assertion Failures
-===========================
-When a debug assertion fails the assertion handler is called via the
-``std::__libcpp_debug_function`` function pointer. It is possible to override
-this function pointer using a different handler function. Libc++ provides a
-the default handler, ``std::__libcpp_abort_debug_handler``, which aborts the
-program. The handler may not return. Libc++ can be changed to use a custom
-assertion handler as follows.
-
-.. code-block:: cpp
-
-  #define _LIBCPP_DEBUG 1
-  #include <string>
-  void my_handler(std::__libcpp_debug_info const&);
-  int main(int, char**) {
-    std::__libcpp_debug_function = &my_handler;
-
-    std::string::iterator bad_it;
-    std::string str("hello world");
-    str.insert(bad_it, '!'); // causes debug assertion
-    // control flow doesn't return
-  }
+Randomizing unspecified behavior
+--------------------------------
+The library supports the randomization of unspecified behavior. For example, randomizing
+the relative order of equal elements in ``std::sort`` or randomizing both parts of the
+partition after calling ``std::nth_element``. This effort helps migrating to potential
+future faster versions of these algorithms that might not have the exact same behavior.
+In particular, it makes it easier to deflake tests that depend on unspecified behavior.
+A seed can be used to make such failures reproducible: use ``_LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY_SEED=seed``.
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
@@ -113,6 +113,13 @@ API Changes
 - ``vector<bool>::const_reference``, ``vector<bool>::const_iterator::reference``
   and ``bitset::const_reference`` are now aliases for `bool` in the unstable ABI.
 
+- The ``_LIBCPP_DEBUG`` macro is not supported anymore. It will be honoured until
+  LLVM 16, and then it will be an error to define that macro. To enable basic
+  assertions (previously ``_LIBCPP_DEBUG=0``), please use ``_LIBCPP_ENABLE_ASSERTIONS=1``.
+  To enable the debug mode (previously ``_LIBCPP_DEBUG=1|2``), please ensure that
+  the library has been built with support for the debug mode, and it will be
+  enabled automatically (no need to define ``_LIBCPP_DEBUG``).
+
 ABI Changes
 -----------
 
@@ -168,3 +175,7 @@ Build System Changes
   configuration and isn't supported by one of the configurations in ``libcxx/test/configs``,
   ``libcxxabi/test/configs`` or ``libunwind/test/configs``, please move to one of those
   configurations or define your own.
+
+- The ``LIBCXX_ENABLE_DEBUG_MODE_SUPPORT`` CMake configuration is not supported anymore. If you
+  were disabling support for the debug mode with that flag, please use ``LIBCXX_ENABLE_BACKWARDS_COMPATIBILITY_DEBUG_MODE_SYMBOLS=OFF``
+  instead.
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
@@ -158,13 +158,6 @@ default.
   still be used to specify the path of the library to link to and run against,
   respectively.
 
-.. option:: debug_level=<level>
-
-  **Values**: 0, 1
-
-  Enable the use of debug mode. Level 0 enables assertions and level 1 enables
-  assertions and debugging of iterator misuse.
-
 .. option:: use_sanitizer=<sanitizer name>
 
   **Values**: Memory, MemoryWithOrigins, Address, Undefined

diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
@@ -214,9 +214,6 @@ Libc++ provides a number of configuration macros which can be used to enable
 or disable extended libc++ behavior, including enabling "debug mode" or
 thread safety annotations.
 
-**_LIBCPP_DEBUG**:
-  See :ref:`using-debug-mode` for more information.
-
 **_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS**:
   This macro is used to enable -Wthread-safety annotations on libc++'s
   ``std::mutex`` and ``std::lock_guard``. By default, these annotations are

diff --git a/libcxx/include/__algorithm/comp_ref_type.h b/libcxx/include/__algorithm/comp_ref_type.h
@@ -68,7 +68,7 @@ template <class _Comp>
 struct __comp_ref_type {
   // Pass the comparator by lvalue reference. Or in debug mode, using a
   // debugging wrapper that stores a reference.
-#if _LIBCPP_DEBUG_LEVEL == 2
+#ifdef _LIBCPP_ENABLE_DEBUG_MODE
   typedef __debug_less<_Comp> type;
 #else
   typedef _Comp& type;

diff --git a/libcxx/include/__algorithm/nth_element.h b/libcxx/include/__algorithm/nth_element.h
@@ -13,6 +13,7 @@
 #include <__algorithm/comp_ref_type.h>
 #include <__algorithm/sort.h>
 #include <__config>
+#include <__debug>
 #include <__iterator/iterator_traits.h>
 #include <__utility/swap.h>
 

diff --git a/libcxx/include/__algorithm/partial_sort.h b/libcxx/include/__algorithm/partial_sort.h
@@ -15,6 +15,7 @@
 #include <__algorithm/sift_down.h>
 #include <__algorithm/sort_heap.h>
 #include <__config>
+#include <__debug>
 #include <__iterator/iterator_traits.h>
 #include <__utility/swap.h>
 

diff --git a/libcxx/include/__algorithm/shuffle.h b/libcxx/include/__algorithm/shuffle.h
@@ -10,6 +10,7 @@
 #define _LIBCPP___ALGORITHM_SHUFFLE_H
 
 #include <__config>
+#include <__debug>
 #include <__iterator/iterator_traits.h>
 #include <__random/uniform_int_distribution.h>
 #include <__utility/swap.h>

diff --git a/libcxx/include/__algorithm/sort.h b/libcxx/include/__algorithm/sort.h
@@ -16,6 +16,7 @@
 #include <__algorithm/unwrap_iter.h>
 #include <__bits>
 #include <__config>
+#include <__debug>
 #include <__functional/operations.h>
 #include <__utility/swap.h>
 #include <climits>

diff --git a/libcxx/include/__algorithm/unwrap_iter.h b/libcxx/include/__algorithm/unwrap_iter.h
@@ -43,7 +43,7 @@ struct __unwrap_iter_impl {
     }
 };
 
-#if _LIBCPP_DEBUG_LEVEL < 2
+#ifndef _LIBCPP_ENABLE_DEBUG_MODE
 
 template <class _Iter>
 struct __unwrap_iter_impl<_Iter, true> {
@@ -53,7 +53,7 @@ struct __unwrap_iter_impl<_Iter, true> {
     }
 };
 
-#endif // _LIBCPP_DEBUG_LEVEL < 2
+#endif // !_LIBCPP_ENABLE_DEBUG_MODE
 
 template<class _Iter, class _Impl = __unwrap_iter_impl<_Iter> >
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR

diff --git a/libcxx/include/__assert b/libcxx/include/__assert
@@ -19,7 +19,15 @@
 
 // This is for backwards compatibility with code that might have been enabling
 // assertions through the Debug mode previously.
-#if _LIBCPP_DEBUG_LEVEL >= 1
+// TODO: In LLVM 16, make it an error to define _LIBCPP_DEBUG
+#if defined(_LIBCPP_DEBUG)
+# ifndef _LIBCPP_ENABLE_ASSERTIONS
+#   define _LIBCPP_ENABLE_ASSERTIONS 1
+# endif
+#endif
+
+// Automatically enable assertions when the debug mode is enabled.
+#if defined(_LIBCPP_ENABLE_DEBUG_MODE)
 # ifndef _LIBCPP_ENABLE_ASSERTIONS
 #   define _LIBCPP_ENABLE_ASSERTIONS 1
 # endif

diff --git a/libcxx/include/__config b/libcxx/include/__config
@@ -792,55 +792,14 @@ typedef unsigned int   char32_t;
 #  define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
 #endif // _LIBCPP_CXX03_LANG
 
-// _LIBCPP_DEBUG potential values:
-//  - undefined: No assertions. This is the default.
-//  - 0:         Basic assertions
-//  - 1:         Basic assertions + iterator validity checks + unspecified behavior randomization.
-#  if !defined(_LIBCPP_DEBUG)
-#    define _LIBCPP_DEBUG_LEVEL 0
-#  elif _LIBCPP_DEBUG == 0
-#    define _LIBCPP_DEBUG_LEVEL 1
-#  elif _LIBCPP_DEBUG == 1
-#    define _LIBCPP_DEBUG_LEVEL 2
-#  else
-#    error Supported values for _LIBCPP_DEBUG are 0 and 1
-#  endif
-
-#  if _LIBCPP_DEBUG_LEVEL >= 2 && !defined(_LIBCPP_CXX03_LANG)
-#    define _LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY
-#  endif
-
-#  if defined(_LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY)
-#    if defined(_LIBCPP_CXX03_LANG)
-#      error Support for unspecified stability is only for C++11 and higher
-#    endif
-#    define _LIBCPP_DEBUG_RANDOMIZE_RANGE(__first, __last)                                                             \
-      do {                                                                                                             \
-        if (!__builtin_is_constant_evaluated())                                                                        \
-          _VSTD::shuffle(__first, __last, __libcpp_debug_randomizer());                                                \
-      } while (false)
-#  else
-#    define _LIBCPP_DEBUG_RANDOMIZE_RANGE(__first, __last)                                                             \
-      do {                                                                                                             \
-      } while (false)
-#  endif
-
 // Libc++ allows disabling extern template instantiation declarations by
 // means of users defining _LIBCPP_DISABLE_EXTERN_TEMPLATE.
 //
-// Furthermore, when the Debug mode is enabled, we disable extern declarations
-// when building user code because we don't want to use the functions compiled
-// in the library, which might not have had the debug mode enabled when built.
-// However, some extern declarations need to be used, because code correctness
-// depends on it (several instances in <locale>). Those special declarations
-// are declared with _LIBCPP_EXTERN_TEMPLATE_EVEN_IN_DEBUG_MODE, which is enabled
-// even when the debug mode is enabled.
+// TODO: Remove _LIBCPP_EXTERN_TEMPLATE_EVEN_IN_DEBUG_MODE, which is not needed
+//       since the Debug mode is a configuration-time property.
 #if defined(_LIBCPP_DISABLE_EXTERN_TEMPLATE)
 #   define _LIBCPP_EXTERN_TEMPLATE(...) /* nothing */
 #   define _LIBCPP_EXTERN_TEMPLATE_EVEN_IN_DEBUG_MODE(...) /* nothing */
-#elif _LIBCPP_DEBUG_LEVEL >= 1 && !defined(_LIBCPP_BUILDING_LIBRARY)
-#   define _LIBCPP_EXTERN_TEMPLATE(...) /* nothing */
-#   define _LIBCPP_EXTERN_TEMPLATE_EVEN_IN_DEBUG_MODE(...) extern template __VA_ARGS__;
 #else
 #   define _LIBCPP_EXTERN_TEMPLATE(...) extern template __VA_ARGS__;
 #   define _LIBCPP_EXTERN_TEMPLATE_EVEN_IN_DEBUG_MODE(...) extern template __VA_ARGS__;

diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
@@ -33,6 +33,7 @@
 #cmakedefine _LIBCPP_HAS_NO_INCOMPLETE_FORMAT
 #cmakedefine _LIBCPP_HAS_NO_INCOMPLETE_RANGES
 #cmakedefine01 _LIBCPP_ENABLE_ASSERTIONS_DEFAULT
+#cmakedefine _LIBCPP_ENABLE_DEBUG_MODE
 
 // __USE_MINGW_ANSI_STDIO gets redefined on MinGW
 #ifdef __clang__