[libcxx-commits] [libcxx] be00e88 - [libc++] Clarify how we pick the typeinfo comparison
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Nov 18 13:58:54 PST 2020
Author: Louis Dionne
Date: 2020-11-18T16:58:45-05:00
New Revision: be00e8893fdb814e67d8f06401afe869f878c4cf
URL: https://github.com/llvm/llvm-project/commit/be00e8893fdb814e67d8f06401afe869f878c4cf
DIFF: https://github.com/llvm/llvm-project/commit/be00e8893fdb814e67d8f06401afe869f878c4cf.diff
LOG: [libc++] Clarify how we pick the typeinfo comparison
This commit makes it clear that the typeinfo comparison implementation
is automatically selected by default, and that the CMake option only
overrides the value. This has been a source of confusion and bugs ever
since we've introduced complexity in that area, so I'm trying to simplify
it while still allowing for some control on the implementation.
Differential Revision: https://reviews.llvm.org/D91574
Added:
Modified:
libcxx/CMakeLists.txt
libcxx/cmake/caches/Apple.cmake
libcxx/docs/BuildingLibcxx.rst
libcxx/include/__config
libcxx/include/__config_site.in
libcxx/include/typeinfo
Removed:
################################################################################
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 840bc5d97a515..dd4c93b59d330 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -175,14 +175,17 @@ option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the Itanium ABI.")
option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.")
-
-set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "" CACHE STRING "The implementation of typeinfo comparison to use.")
-set(TYPEINFO_COMPARISON_VALUES ";1;2")
-set_property(CACHE LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION PROPERTY STRINGS ${TYPEINFO_COMPARISON_VALUES})
-list(FIND TYPEINFO_COMPARISON_VALUES "${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IS_VALID_DEFAULT)
-if (${IS_VALID_DEFAULT} EQUAL -1)
+set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "default" CACHE STRING
+ "Override the implementation to use for comparing typeinfos. By default, this
+ is detected automatically by the library, but this option allows overriding
+ which implementation is used unconditionally.
+
+ See the documentation in <libcxx/include/typeinfo> for details on what each
+ value means.")
+set(TYPEINFO_COMPARISON_VALUES "default;1;2;3")
+if (NOT ("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IN_LIST TYPEINFO_COMPARISON_VALUES))
message(FATAL_ERROR "Value '${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}' is not a valid value for
- LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION")
+ LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION")
endif()
option(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT "Enable per TU ABI insulation by default. To be used by vendors." OFF)
@@ -841,10 +844,9 @@ config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)
config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
-if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "")
+if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "default")
config_define("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
endif()
-
config_define_if(LIBCXX_HAS_PTHREAD_API _LIBCPP_HAS_THREAD_API_PTHREAD)
config_define_if(LIBCXX_HAS_EXTERNAL_THREAD_API _LIBCPP_HAS_THREAD_API_EXTERNAL)
config_define_if(LIBCXX_HAS_WIN32_THREAD_API _LIBCPP_HAS_THREAD_API_WIN32)
diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
index fbe85b1e9cafd..0c01bb77ce7b9 100644
--- a/libcxx/cmake/caches/Apple.cmake
+++ b/libcxx/cmake/caches/Apple.cmake
@@ -7,7 +7,6 @@ set(LIBCXX_ABI_VERSION "1" CACHE STRING "")
set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
set(LIBCXX_ENABLE_STATIC OFF CACHE BOOL "")
set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
-set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "1" CACHE STRING "")
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 "")
diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index c85c3d7fbcb6a..67ce03dd4b669 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -378,26 +378,6 @@ The following options allow building libc++ for a
diff erent ABI version.
See ``include/__config`` for the list of ABI macros.
-.. option:: LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION
-
- **Default**: ``None``, which lets the library figure out which implementation
- to use based on the object format.
-
- This setting defines what implementation to use for comparing typeinfo objects.
- There are two main implementations, which
diff er on whether we make the assumption
- that type info names for a type have been fully merged are unique across the entire
- program. This may not be the case for libraries built with ``-Bsymbolic`` or due to
- compiler or linker bugs (Ex. llvm.org/PR37398).
-
-
- When the value is set to ``1``, we assume that typeinfos are unique across the
- whole program, and typeinfo comparisons compare only the pointer value.
-
- When the value is set to ``2``, we do not assume that typeinfos are unique across
- the whole program. We first compare the pointers, and then use ``strcmp`` on the
- typeinfo names as a fallback.
-
-
.. _LLVM-specific variables:
LLVM-specific options
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 59aa922df5da9..413931a7a897f 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -755,16 +755,6 @@ typedef __char32_t char32_t;
# endif
#endif
-#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
-# ifdef _LIBCPP_OBJECT_FORMAT_COFF // Windows binaries can't merge typeinfos.
-# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2
-# else
- // TODO: This isn't strictly correct on ELF platforms due to llvm.org/PR37398
- // And we should consider defaulting to OFF.
-# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 1
-# endif
-#endif
-
#ifndef _LIBCPP_HIDE_FROM_ABI
# if _LIBCPP_HIDE_FROM_ABI_PER_TU
# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_INTERNAL_LINKAGE
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index 7f4a429f17882..6089fb7d01331 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -28,9 +28,7 @@
#cmakedefine _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
#cmakedefine _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS
#cmakedefine _LIBCPP_NO_VCRUNTIME
-#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
#cmakedefine _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION @_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION@
-#endif
#cmakedefine _LIBCPP_ABI_NAMESPACE @_LIBCPP_ABI_NAMESPACE@
#cmakedefine _LIBCPP_HAS_PARALLEL_ALGORITHMS
#cmakedefine _LIBCPP_HAS_NO_RANDOM_DEVICE
diff --git a/libcxx/include/typeinfo b/libcxx/include/typeinfo
index 7818ed37d0c8a..6c955f53ac87d 100644
--- a/libcxx/include/typeinfo
+++ b/libcxx/include/typeinfo
@@ -142,7 +142,7 @@ public:
// comparison is equal.
// -------------------------------------------------------------------------- //
// NonUniqueARMRTTIBit
-// (selected on ARM64 regardless of _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
+// (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 3)
// -------------------------------------------------------------------------- //
// This implementation of type_info does not assume always a unique copy of
// the RTTI for a given type inside a program. It packs the pointer to the
@@ -161,6 +161,24 @@ public:
// the pointer when it constructs the type_info, depending on whether it can
// guarantee uniqueness for that specific type_info.
+// This value can be overriden in the __config_site. When it's not overriden,
+// we pick a default implementation based on the platform here.
+#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
+
+ // Windows binaries can't merge typeinfos, so use the NonUnique implementation.
+# ifdef _LIBCPP_OBJECT_FORMAT_COFF
+# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2
+
+ // On arm64 on Apple platforms, use the special NonUniqueARMRTTIBit implementation.
+# elif defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
+# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 3
+
+ // On all other platforms, assume the Itanium C++ ABI and use the Unique implementation.
+# else
+# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 1
+# endif
+#endif
+
struct __type_info_implementations {
struct __string_impl_base {
typedef const char* __type_name_t;
@@ -258,12 +276,12 @@ struct __type_info_implementations {
};
typedef
-#if defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
- __non_unique_arm_rtti_bit_impl
-#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 1
+#if _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 1
__unique_impl
#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 2
__non_unique_impl
+#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 3
+ __non_unique_arm_rtti_bit_impl
#else
# error invalid configuration for _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
#endif
More information about the libcxx-commits
mailing list