ldionne updated this revision to Diff 443066.
ldionne added a comment.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.
Band-aid for the LLDB issue, and also include 6148c79a
<https://reviews.llvm.org/rG6148c79a64ff03e5876882dd90d895ddb1783778> in the
commit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127444/new/
https://reviews.llvm.org/D127444
Files:
libcxx/CMakeLists.txt
libcxx/cmake/caches/Apple.cmake
libcxx/docs/BuildingLibcxx.rst
libcxx/docs/DesignDocs/VisibilityMacros.rst
libcxx/docs/ReleaseNotes.rst
libcxx/include/__config
libcxx/include/__config_site.in
libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
llvm/utils/gn/secondary/libcxx/include/BUILD.gn
Index: llvm/utils/gn/secondary/libcxx/include/BUILD.gn
===================================================================
--- llvm/utils/gn/secondary/libcxx/include/BUILD.gn
+++ llvm/utils/gn/secondary/libcxx/include/BUILD.gn
@@ -19,7 +19,6 @@
"_LIBCPP_ABI_FORCE_ITANIUM=",
"_LIBCPP_ABI_FORCE_MICROSOFT=",
"_LIBCPP_EXTRA_SITE_DEFINES=",
- "_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT=",
"_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY=",
"_LIBCPP_HAS_NO_INCOMPLETE_FORMAT=",
"_LIBCPP_HAS_NO_INCOMPLETE_RANGES=",
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===================================================================
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -387,7 +387,8 @@
endif
ifeq (1,$(USE_LIBCPP))
- CXXFLAGS += -DLLDB_USING_LIBCPP
+ # TODO: Teach LLDB to handle ABI tags in libc++ namespaces.
+ CXXFLAGS += -DLLDB_USING_LIBCPP -D_LIBCPP_NO_ABI_TAG
ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
Index: libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
===================================================================
--- libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// Regression test for PR42676.
-
-// RUN: %{cxx} %{flags} %s -o %t.exe %{compile_flags} %{link_flags} -D_LIBCPP_HIDE_FROM_ABI_PER_TU
-// RUN: %{run}
-
-#include <memory>
-#include <string>
-
-int main(int, char**) {
- std::string s1(10u, '-', std::allocator<char>()); (void)s1;
- std::string s2("hello", std::allocator<char>()); (void)s2;
- std::string s3("hello"); (void)s3;
- return 0;
-}
Index: libcxx/include/__config_site.in
===================================================================
--- libcxx/include/__config_site.in
+++ libcxx/include/__config_site.in
@@ -13,7 +13,6 @@
#cmakedefine _LIBCPP_ABI_NAMESPACE @_LIBCPP_ABI_NAMESPACE@
#cmakedefine _LIBCPP_ABI_FORCE_ITANIUM
#cmakedefine _LIBCPP_ABI_FORCE_MICROSOFT
-#cmakedefine _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
#cmakedefine _LIBCPP_HAS_NO_THREADS
#cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK
#cmakedefine _LIBCPP_HAS_MUSL_LIBC
Index: libcxx/include/__config
===================================================================
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -26,6 +26,13 @@
# define _LIBCPP_VERSION 15000
+# define _LIBCPP_CONCAT_IMPL(_X, _Y) _X##_Y
+# define _LIBCPP_CONCAT(_X, _Y) _LIBCPP_CONCAT_IMPL(_X, _Y)
+
+// Valid C++ identifier that revs with every libc++ version. This can be used to
+// generate identifiers that must be unique for every released libc++ version.
+# define _LIBCPP_VERSIONED_IDENTIFIER _LIBCPP_CONCAT(v, _LIBCPP_VERSION)
+
# if __STDC_HOSTED__ == 0
# define _LIBCPP_FREESTANDING
# endif
@@ -568,12 +575,6 @@
# endif // defined(_LIBCPP_OBJECT_FORMAT_COFF)
-# if __has_attribute(internal_linkage)
-# define _LIBCPP_INTERNAL_LINKAGE __attribute__((internal_linkage))
-# else
-# define _LIBCPP_INTERNAL_LINKAGE _LIBCPP_ALWAYS_INLINE
-# endif
-
# if __has_attribute(exclude_from_explicit_instantiation)
# define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((__exclude_from_explicit_instantiation__))
# else
@@ -583,20 +584,35 @@
# define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION _LIBCPP_ALWAYS_INLINE
# endif
-# ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU
-# ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
-# define _LIBCPP_HIDE_FROM_ABI_PER_TU 0
-# else
-# define _LIBCPP_HIDE_FROM_ABI_PER_TU 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
-# else
-# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
-# endif
+// This macro marks a symbol as being hidden from libc++'s ABI. This is achieved
+// on two levels:
+// 1. The symbol is given hidden visibility, which ensures that users won't start exporting
+// symbols from their dynamic library by means of using the libc++ headers. This ensures
+// that those symbols stay private to the dynamic library in which it is defined.
+//
+// 2. The symbol is given an ABI tag that changes with each version of libc++. This ensures
+// that no ODR violation can arise from mixing two TUs compiled with different versions
+// of libc++ where we would have changed the definition of a symbol. If the symbols shared
+// the same name, the ODR would require that their definitions be token-by-token equivalent,
+// which basically prevents us from being able to make any change to any function in our
+// headers. Using this ABI tag ensures that the symbol name is "bumped" artificially at
+// each release, which lets us change the definition of these symbols at our leisure.
+// Note that historically, this has been achieved in various ways, including force-inlining
+// all functions or giving internal linkage to all functions. Both these (previous) solutions
+// suffer from drawbacks that lead notably to code bloat.
+//
+// Note that we use _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION to ensure that we don't depend
+// on _LIBCPP_HIDE_FROM_ABI methods of classes explicitly instantiated in the dynamic library.
+//
+// TODO: We provide a escape hatch with _LIBCPP_NO_ABI_TAG for folks who want to avoid increasing
+// the length of symbols with an ABI tag. In practice, we should remove the escape hatch and
+// use compression mangling instead, see https://github.com/itanium-cxx-abi/cxx-abi/issues/70.
+# ifndef _LIBCPP_NO_ABI_TAG
+# define _LIBCPP_HIDE_FROM_ABI \
+ _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION \
+ __attribute__((__abi_tag__(_LIBCPP_TOSTRING(_LIBCPP_VERSIONED_IDENTIFIER))))
+# else
+# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
# endif
# ifdef _LIBCPP_BUILDING_LIBRARY
Index: libcxx/docs/ReleaseNotes.rst
===================================================================
--- libcxx/docs/ReleaseNotes.rst
+++ libcxx/docs/ReleaseNotes.rst
@@ -224,3 +224,8 @@
means that the same set of installed headers works for both DLL and static
linkage. This means that distributors finally can build both library
versions with a single CMake invocation.
+
+- The ``LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT`` configuration option has been removed. Indeed,
+ the risk of ODR violations from mixing different versions of libc++ in the same program has
+ been mitigated with a different technique that is simpler and does not have the drawbacks of
+ using internal linkage.
Index: libcxx/docs/DesignDocs/VisibilityMacros.rst
===================================================================
--- libcxx/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/docs/DesignDocs/VisibilityMacros.rst
@@ -65,41 +65,6 @@
ABI, we should create a new _LIBCPP_HIDE_FROM_ABI_AFTER_XXX macro, and we can
use it to start removing symbols from the ABI after that stable version.
-**_LIBCPP_HIDE_FROM_ABI_PER_TU**
- This macro controls whether symbols hidden from the ABI with `_LIBCPP_HIDE_FROM_ABI`
- are local to each translation unit in addition to being local to each final
- linked image. This macro is defined to either 0 or 1. When it is defined to
- 1, translation units compiled with different versions of libc++ can be linked
- together, since all non ABI-facing functions are local to each translation unit.
- This allows static archives built with different versions of libc++ to be linked
- together. This also means that functions marked with `_LIBCPP_HIDE_FROM_ABI`
- are not guaranteed to have the same address across translation unit boundaries.
-
- When the macro is defined to 0, there is no guarantee that translation units
- compiled with different versions of libc++ can interoperate. However, this
- leads to code size improvements, since non ABI-facing functions can be
- deduplicated across translation unit boundaries.
-
- This macro can be defined by users to control the behavior they want from
- libc++. The default value of this macro (0 or 1) is controlled by whether
- `_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT` is defined, which is intended to
- be used by vendors only (see below).
-
-**_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT**
- This macro controls the default value for `_LIBCPP_HIDE_FROM_ABI_PER_TU`.
- When the macro is defined, per TU ABI insulation is enabled by default, and
- `_LIBCPP_HIDE_FROM_ABI_PER_TU` is defined to 1 unless overridden by users.
- Otherwise, per TU ABI insulation is disabled by default, and
- `_LIBCPP_HIDE_FROM_ABI_PER_TU` is defined to 0 unless overridden by users.
-
- This macro is intended for vendors to control whether they want to ship
- libc++ with per TU ABI insulation enabled by default. Users can always
- control the behavior they want by defining `_LIBCPP_HIDE_FROM_ABI_PER_TU`
- appropriately.
-
- By default, this macro is not defined, which means that per TU ABI insulation
- is not provided unless explicitly overridden by users.
-
**_LIBCPP_TYPE_VIS**
Mark a type's typeinfo, vtable and members as having default visibility.
This attribute cannot be used on class templates.
@@ -194,22 +159,6 @@
versioning namespace. This allows throwing and catching some exception types
between libc++ and libstdc++.
-**_LIBCPP_INTERNAL_LINKAGE**
- Mark the affected entity as having internal linkage (i.e. the `static`
- keyword in C). This is only a best effort: when the `internal_linkage`
- attribute is not available, we fall back to forcing the function to be
- inlined, which approximates internal linkage since an externally visible
- symbol is never generated for that function. This is an internal macro
- used as an implementation detail by other visibility macros. Never mark
- a function or a class with this macro directly.
-
-**_LIBCPP_ALWAYS_INLINE**
- Forces inlining of the function it is applied to. For visibility purposes,
- this macro is used to make sure that an externally visible symbol is never
- generated in an object file when the `internal_linkage` attribute is not
- available. This is an internal macro used by other visibility macros, and
- it should not be used directly.
-
Links
=====
Index: libcxx/docs/BuildingLibcxx.rst
===================================================================
--- libcxx/docs/BuildingLibcxx.rst
+++ libcxx/docs/BuildingLibcxx.rst
@@ -412,15 +412,6 @@
Use the specified GCC toolchain and standard library when building the native
stdlib benchmark tests.
-.. option:: LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT:BOOL
-
- **Default**: ``OFF``
-
- Pick the default for whether to constrain ABI-unstable symbols to
- each individual translation unit. This setting controls whether
- `_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT` is defined by default --
- see the documentation of that macro for details.
-
libc++ ABI Feature Options
--------------------------
Index: libcxx/cmake/caches/Apple.cmake
===================================================================
--- libcxx/cmake/caches/Apple.cmake
+++ libcxx/cmake/caches/Apple.cmake
@@ -8,7 +8,6 @@
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_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 "")
Index: libcxx/CMakeLists.txt
===================================================================
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -205,7 +205,6 @@
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)
set(LIBCXX_ABI_DEFINES "" CACHE STRING "A semicolon separated list of ABI macros to define in the site config header.")
option(LIBCXX_EXTRA_SITE_DEFINES "Extra defines to add into __config_site")
option(LIBCXX_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF)
@@ -847,7 +846,6 @@
config_define(${LIBCXX_ABI_NAMESPACE} _LIBCPP_ABI_NAMESPACE)
config_define_if(LIBCXX_ABI_FORCE_ITANIUM _LIBCPP_ABI_FORCE_ITANIUM)
config_define_if(LIBCXX_ABI_FORCE_MICROSOFT _LIBCPP_ABI_FORCE_MICROSOFT)
-config_define_if(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT)
config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "default")
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
