[libcxx-commits] [libcxx] d0fcdcd - [libc++] Fix the LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT setting
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Fri May 29 03:14:57 PDT 2020
Author: Louis Dionne
Date: 2020-05-29T06:14:30-04:00
New Revision: d0fcdcd28f95d699b27d2026ede964a7f9cff9dd
URL: https://github.com/llvm/llvm-project/commit/d0fcdcd28f95d699b27d2026ede964a7f9cff9dd
DIFF: https://github.com/llvm/llvm-project/commit/d0fcdcd28f95d699b27d2026ede964a7f9cff9dd.diff
LOG: [libc++] Fix the LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT setting
When the __config_site header is generated, but LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
wasn't specified, _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT would be defined
to 0, which was the NonUnique RTTI comparison implementation. The intent
was to use the Unique RTTI comparison implementation in that case, which
caused https://llvm.org/PR45549.
Instead, use a proper "switch" to select the RTTI comparison implementation.
Note that 0 can't be used as a value, because that is treated the same
by CMake as a variable that is just not defined.
Differential Revision: https://reviews.llvm.org/D80037
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
libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.apple.compile.pass.cpp
libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.merged.sh.cpp
libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.unmerged.sh.cpp
Removed:
################################################################################
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index bdb2d56da853..1a7e0a0bc759 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -154,14 +154,13 @@ option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the Itan
option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.")
-set(LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT "" CACHE STRING
- "Whether typeinfo names are expected to be unique. Defining this option overrides the default configuration in the library.")
-set(MERGED_TYPEINFO_VALUES ";ON;OFF")
-set_property(CACHE LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT PROPERTY STRINGS ${MERGED_TYPEINFO_DEFAULTS})
-list(FIND MERGED_TYPEINFO_VALUES "${LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT}" IS_VALID_DEFAULT)
+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)
- message(FATAL_ERROR "Value '${LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT}' is not a valid value for
- LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT")
+ message(FATAL_ERROR "Value '${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}' is not a valid value for
+ 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)
@@ -840,8 +839,8 @@ 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_HAS_MERGED_TYPEINFO_NAMES_DEFAULT STREQUAL "")
- config_define("${LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT}" _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT)
+if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "")
+ config_define("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
endif()
config_define_if(LIBCXX_HAS_PTHREAD_API _LIBCPP_HAS_THREAD_API_PTHREAD)
diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
index 26985c9f335e..622a3af84f2b 100644
--- a/libcxx/cmake/caches/Apple.cmake
+++ b/libcxx/cmake/caches/Apple.cmake
@@ -7,7 +7,7 @@ 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_HAS_MERGED_TYPEINFO_NAMES_DEFAULT ON CACHE STRING "")
+set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "1" CACHE STRING "")
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index 262558e79add..0fbb6693099c 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -380,18 +380,24 @@ The following options allow building libc++ for a
diff erent ABI version.
See ``include/__config`` for the list of ABI macros.
-.. option:: LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
+.. option:: LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION
- **Default**: ``None``. When defined this option overrides the libraries default configuration
- for whether merged type info names are present.
+ **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).
- Build ``std::type_info`` with 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 ``ON`` typeinfo comparisons compare only the pointer value, otherwise ``strcmp``
- is used as a fallback.
+ 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:
diff --git a/libcxx/include/__config b/libcxx/include/__config
index bfde9e9c9895..cf596a7872ab 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -758,13 +758,13 @@ typedef __char32_t char32_t;
# endif
#endif
-#ifndef _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
+#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
# ifdef _LIBCPP_OBJECT_FORMAT_COFF // Windows binaries can't merge typeinfos.
-# define _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT 0
+# 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_HAS_MERGED_TYPEINFO_NAMES_DEFAULT 1
+# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 1
# endif
#endif
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index 4da11a9e6744..a6984b2eefc1 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -27,8 +27,8 @@
#cmakedefine _LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL
#cmakedefine _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
#cmakedefine _LIBCPP_NO_VCRUNTIME
-#ifndef _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
-#cmakedefine01 _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
+#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
diff --git a/libcxx/include/typeinfo b/libcxx/include/typeinfo
index 8b86c61b974d..7e76da538796 100644
--- a/libcxx/include/typeinfo
+++ b/libcxx/include/typeinfo
@@ -121,6 +121,7 @@ public:
// ========================================================================== //
// ------------------------------------------------------------------------- //
// Unique
+// (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 1)
// ------------------------------------------------------------------------- //
// This implementation of type_info assumes a unique copy of the RTTI for a
// given type inside a program. This is a valid assumption when abiding to
@@ -130,6 +131,7 @@ public:
// a deep string comparison.
// -------------------------------------------------------------------------- //
// NonUnique
+// (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 2)
// -------------------------------------------------------------------------- //
// This implementation of type_info does not assume there is always a unique
// copy of the RTTI for a given type inside a program. For various reasons
@@ -139,6 +141,7 @@ public:
// comparison is equal.
// -------------------------------------------------------------------------- //
// NonUniqueARMRTTIBit
+// (selected on ARM64 regardless of _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
// -------------------------------------------------------------------------- //
// 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
@@ -256,12 +259,12 @@ struct __type_info_implementations {
typedef
#if defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
__non_unique_arm_rtti_bit_impl
-#elif _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT == 0
- __non_unique_impl
-#elif _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT == 1
+#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 1
__unique_impl
+#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 2
+ __non_unique_impl
#else
-# error invalid configuration for _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
+# error invalid configuration for _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
#endif
__impl;
};
diff --git a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.apple.compile.pass.cpp b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.apple.compile.pass.cpp
index d6fd0fba31d3..68066416fd36 100644
--- a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.apple.compile.pass.cpp
+++ b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.apple.compile.pass.cpp
@@ -18,10 +18,10 @@
#include <typeinfo>
-#if !defined(_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT)
-# error "_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT should be defined on Apple platforms"
+#if !defined(_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
+# error "_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION should be defined on Apple platforms"
#endif
-#if _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT != 1
-# error "_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT should be 1 (assume RTTI is merged) on Apple platforms"
+#if _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION != 1
+# error "_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION should be 1 (assume RTTI is merged) on Apple platforms"
#endif
diff --git a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.merged.sh.cpp b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.merged.sh.cpp
index a69d53fc9414..08e3eb67af8b 100644
--- a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.merged.sh.cpp
+++ b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.merged.sh.cpp
@@ -9,9 +9,9 @@
// UNSUPPORTED: -fno-rtti
// FILE_DEPENDENCIES: %t.exe
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu1.o -DTU1 -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=1
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu2.o -DTU2 -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=1
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.main.o -DMAIN -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=1
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu1.o -DTU1 -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=1
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu2.o -DTU2 -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=1
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.main.o -DMAIN -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=1
// RUN: %{cxx} %t.tu1.o %t.tu2.o %t.main.o %{flags} %{link_flags} -o %t.exe
// RUN: %{exec} %t.exe
diff --git a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.unmerged.sh.cpp b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.unmerged.sh.cpp
index c7213b7748c8..e81a1b0e3437 100644
--- a/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.unmerged.sh.cpp
+++ b/libcxx/test/libcxx/language.support/support.rtti/type.info/type_info.comparison.unmerged.sh.cpp
@@ -9,9 +9,9 @@
// UNSUPPORTED: -fno-rtti
// FILE_DEPENDENCIES: %t.exe
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu1.o -DTU1 -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=0
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu2.o -DTU2 -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=0
-// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.main.o -DMAIN -D_LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT=0
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu1.o -DTU1 -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=2
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.tu2.o -DTU2 -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=2
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.main.o -DMAIN -D_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION=2
// RUN: %{cxx} %t.tu1.o %t.tu2.o %t.main.o %{flags} %{link_flags} -o %t.exe
// RUN: %{exec} %t.exe
More information about the libcxx-commits
mailing list