[libcxx-commits] [libcxx] d0fcdcd - [libc++] Fix the LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT setting
David Zarzycki via libcxx-commits
libcxx-commits at lists.llvm.org
Fri May 29 08:29:27 PDT 2020
Hi Louis,
I think some mailing list limit is clogging my email to you. This is breaking Fedora 32 with clang-10 (as provided by Fedora). Can we revert this or commit a quick fix? The log message should be viewable on D80037.
Thanks,
Dave
On Fri, May 29, 2020, at 6:14 AM, Louis Dionne via libcxx-commits wrote:
>
> 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
>
>
>
>
> _______________________________________________
> libcxx-commits mailing list
> libcxx-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits
>
More information about the libcxx-commits
mailing list