[libcxx-commits] [libcxx] 61d417c - Revert "[libc++] Use ABI tags instead of internal linkage to provide per-TU insulation"

Jonas Devlieghere via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 7 08:59:10 PDT 2022


Author: Jonas Devlieghere
Date: 2022-07-07T08:58:55-07:00
New Revision: 61d417ceff90c4bd99bfed082d81bcc33ecf5a45

URL: https://github.com/llvm/llvm-project/commit/61d417ceff90c4bd99bfed082d81bcc33ecf5a45
DIFF: https://github.com/llvm/llvm-project/commit/61d417ceff90c4bd99bfed082d81bcc33ecf5a45.diff

LOG: Revert "[libc++] Use ABI tags instead of internal linkage to provide per-TU insulation"

This reverts commit 9ee97ce3b8305c5762ec34eecb4daf379984c95b.

Added: 
    libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 3d63bb0c8fbf..b118007eacbc 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -205,6 +205,7 @@ if (NOT ("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IN_LIST TYPEINFO_COMPARI
                        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)
@@ -846,6 +847,7 @@ config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION)
 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")

diff  --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
index ea701a43f96a..db567bd394b1 100644
--- a/libcxx/cmake/caches/Apple.cmake
+++ b/libcxx/cmake/caches/Apple.cmake
@@ -8,6 +8,7 @@ set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
 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 "")

diff  --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index 5069c7fe0692..3f4e314382d8 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -412,6 +412,15 @@ libc++ Feature Options
   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
 --------------------------

diff  --git a/libcxx/docs/DesignDocs/VisibilityMacros.rst b/libcxx/docs/DesignDocs/VisibilityMacros.rst
index d1c162682b66..a165fc49afcf 100644
--- a/libcxx/docs/DesignDocs/VisibilityMacros.rst
+++ b/libcxx/docs/DesignDocs/VisibilityMacros.rst
@@ -65,6 +65,41 @@ Visibility Macros
   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 
diff erent 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 
diff erent 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 
diff erent 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.
@@ -159,6 +194,22 @@ Visibility Macros
   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
 =====
 

diff  --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
index c6eb835f9739..c2917d4710df 100644
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -224,8 +224,3 @@ Build System Changes
   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 
diff erent versions of libc++ in the same program has
-  been mitigated with a 
diff erent technique that is simpler and does not have the drawbacks of
-  using internal linkage.

diff  --git a/libcxx/include/__config b/libcxx/include/__config
index 22c2ed7fd87b..e4b7d25edf34 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -26,13 +26,6 @@
 
 #  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
@@ -575,6 +568,12 @@ typedef __char32_t char32_t;
 
 #  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
@@ -584,35 +583,20 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION _LIBCPP_ALWAYS_INLINE
 #  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 
diff erent 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
+#  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
 #  endif
 
 #  ifdef _LIBCPP_BUILDING_LIBRARY

diff  --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index 97892e99fa49..c615575226ea 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -13,6 +13,7 @@
 #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

diff  --git a/libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp b/libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
new file mode 100644
index 000000000000..217091562b46
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
@@ -0,0 +1,22 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+}


        


More information about the libcxx-commits mailing list