[libcxx-commits] [libcxx] [libc++] Encode additional ODR-affecting properties in the ABI tag (PR #69669)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Oct 24 12:17:44 PDT 2023
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/69669
>From 4a47a7c6576b6e79d671a0824998875b1c1b247c Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 19 Oct 2023 17:03:49 -0700
Subject: [PATCH 1/4] [libc++] Encode additional ODR-affecting properties in
the ABI tag
As explained in __config, we have an ABI tag that we use to ensure that
we don't run into ODR issues when mixing different versions of libc++
in multiple TUs. However, the reasoning behind that extends not only to
different versions of libc++, but also to different configurations of
the same version of libc++. In fact, we've been aware of this for a while
but never really bothered to make the change because ODR issues are often
thought to be benign.
Well, it turns out that I just spent over an hour banging my head against
an issue that boils down to our lack of encoding some ODR properties in
the ABI tag, so here's the patch we should have done a long time ago.
For now, the ODR properties we encode in the ABI tag are:
- library version
- exceptions vs no-exceptions
- hardening mode
Those are all things that we support different values for on a per-TU
basis and they definitely affect ODR in a meaningful way. We can add
more properties later as we see fit.
---
libcxx/include/__config | 58 ++++++++++++-----
.../libcxx/odr_signature.exceptions.sh.cpp | 43 +++++++++++++
.../libcxx/odr_signature.hardening.sh.cpp | 63 +++++++++++++++++++
3 files changed, 149 insertions(+), 15 deletions(-)
create mode 100644 libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
create mode 100644 libcxx/test/libcxx/odr_signature.hardening.sh.cpp
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 65ce6d6a27f8326..2fa548132bba569 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -56,10 +56,6 @@
# 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
@@ -734,22 +730,54 @@ typedef __char32_t char32_t;
# define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION _LIBCPP_ALWAYS_INLINE
# endif
+# if _LIBCPP_ENABLE_HARDENED_MODE
+# define _LIBCPP_HARDENING_SIG h
+# elif _LIBCPP_ENABLE_SAFE_MODE
+# define _LIBCPP_HARDENING_SIG s
+# elif _LIBCPP_ENABLE_DEBUG_MODE
+# define _LIBCPP_HARDENING_SIG d
+# else
+# define _LIBCPP_HARDENING_SIG u // for unchecked
+# endif
+
+# ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+# define _LIBCPP_EXCEPTIONS_SIG n
+# else
+# define _LIBCPP_EXCEPTIONS_SIG e
+# endif
+
+# define _LIBCPP_ODR_SIGNATURE \
+ _LIBCPP_CONCAT(_LIBCPP_CONCAT(_LIBCPP_CONCAT(v, _LIBCPP_VERSION), _LIBCPP_HARDENING_SIG), _LIBCPP_EXCEPTIONS_SIG)
+
// 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.
+// 2. The symbol is given an ABI tag that encodes the ODR-relevant properties of the library.
+// This ensures that no ODR violation can arise from mixing two TUs compiled with different
+// versions or configurations of libc++ (such as exceptions vs no-exceptions). Indeed, if the
+// program contains two definitions of a function, the ODR requires them to be token-by-token
+// equivalent, and the linker is allowed to pick either definition and discard the other one.
+//
+// For example, if a program contains a copy of `vector::at()` compiled with exceptions enabled
+// *and* a copy of `vector::at()` compiled with exceptions disabled (by means of having two TUs
+// compiled with different settings), the two definitions are both visible by the linker and they
+// have the same name, but they have a meaningfully different implementation (one throws an exception
+// and the other aborts the program). This violates the ODR and makes the program ill-formed, and in
+// practice what will happen is that the linker will pick one of the definitions at random and will
+// discard the other one. This can quite clearly lead to incorrect program behavior.
+//
+// A similar reasoning holds for many other properties that are ODR-affecting. Essentially any
+// property that causes the code of a function to differ from the code in another configuration
+// can be considered ODR-affecting. In practice, we don't encode all such properties in the ABI
+// tag, but we encode the ones that we think are most important: library version, exceptions, and
+// hardening mode.
+//
+// Note that historically, solving this problem 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.
@@ -769,7 +797,7 @@ typedef __char32_t char32_t;
# 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))))
+ __attribute__((__abi_tag__(_LIBCPP_TOSTRING(_LIBCPP_ODR_SIGNATURE))))
# else
# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
# endif
diff --git a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
new file mode 100644
index 000000000000000..4796f4070f86300
--- /dev/null
+++ b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Test that we encode whether exceptions are supported in an ABI tag to avoid
+// ODR violations when linking TUs that have different values for it.
+
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU1 -fno-exceptions -o %t.tu1.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU2 -fexceptions -o %t.tu2.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DMAIN -o %t.main.o
+// RUN: %{cxx} %t.tu1.o %t.tu2.o %t.main.o %{flags} %{link_flags} -o %t.exe
+// RUN: %{exec} %t.exe
+
+// -fno-exceptions
+#ifdef TU1
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 1; }
+int tu1() { return f(); }
+#endif // TU1
+
+// -fexceptions
+#ifdef TU2
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 2; }
+int tu2() { return f(); }
+#endif // TU2
+
+#ifdef MAIN
+# include <cassert>
+
+int tu1();
+int tu2();
+
+int main(int, char**) {
+ assert(tu1() == 1);
+ assert(tu2() == 2);
+ return 0;
+}
+#endif // MAIN
diff --git a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
new file mode 100644
index 000000000000000..b9965030966509d
--- /dev/null
+++ b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Test that we encode the hardening mode in an ABI tag to avoid ODR violations
+// when linking TUs that have different values for it.
+
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU1 -D_LIBCPP_ENABLE_HARDENED_MODE -o %t.tu1.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU2 -D_LIBCPP_ENABLE_SAFE_MODE -o %t.tu2.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU3 -D_LIBCPP_ENABLE_DEBUG_MODE -o %t.tu3.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DTU4 -o %t.tu4.o
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -DMAIN -o %t.main.o
+// RUN: %{cxx} %t.tu1.o %t.tu2.o %t.tu3.o %t.tu4.o %t.main.o %{flags} %{link_flags} -o %t.exe
+// RUN: %{exec} %t.exe
+
+// hardened mode
+#ifdef TU1
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 1; }
+int tu1() { return f(); }
+#endif // TU1
+
+// safe mode
+#ifdef TU2
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 2; }
+int tu2() { return f(); }
+#endif // TU2
+
+// debug mode
+#ifdef TU3
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 3; }
+int tu3() { return f(); }
+#endif // TU3
+
+// unchecked mode
+#ifdef TU4
+# include <__config>
+_LIBCPP_HIDE_FROM_ABI inline int f() { return 4; }
+int tu4() { return f(); }
+#endif // TU4
+
+#ifdef MAIN
+# include <cassert>
+
+int tu1();
+int tu2();
+int tu3();
+int tu4();
+
+int main(int, char**) {
+ assert(tu1() == 1);
+ assert(tu2() == 2);
+ assert(tu3() == 3);
+ assert(tu4() == 4);
+ return 0;
+}
+#endif // MAIN
>From 9be41171fdb9389a979d086b0e3036c91cb96527 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 23 Oct 2023 15:48:23 -0700
Subject: [PATCH 2/4] Reorder ABI tag components to get rid of one byte
---
libcxx/include/__config | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 2fa548132bba569..4bf171f998c6f05 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -747,7 +747,7 @@ typedef __char32_t char32_t;
# endif
# define _LIBCPP_ODR_SIGNATURE \
- _LIBCPP_CONCAT(_LIBCPP_CONCAT(_LIBCPP_CONCAT(v, _LIBCPP_VERSION), _LIBCPP_HARDENING_SIG), _LIBCPP_EXCEPTIONS_SIG)
+ _LIBCPP_CONCAT(_LIBCPP_CONCAT(_LIBCPP_HARDENING_SIG, _LIBCPP_EXCEPTIONS_SIG), _LIBCPP_VERSION)
// This macro marks a symbol as being hidden from libc++'s ABI. This is achieved
// on two levels:
>From 80cdeb11da5fce5a8fc3ced1dae9e03de8fb719b Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 23 Oct 2023 16:14:01 -0700
Subject: [PATCH 3/4] Adjust XFAILs
---
libcxx/test/libcxx/odr_signature.exceptions.sh.cpp | 3 +++
libcxx/test/libcxx/odr_signature.hardening.sh.cpp | 9 +++++++++
2 files changed, 12 insertions(+)
diff --git a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
index 4796f4070f86300..fb94ecd0244a271 100644
--- a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
@@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===//
+// TODO: Investigate
+// XFAIL: target={{.+}}-windows-{{.+}}
+
// Test that we encode whether exceptions are supported in an ABI tag to avoid
// ODR violations when linking TUs that have different values for it.
diff --git a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
index b9965030966509d..0c939b4c4654228 100644
--- a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
@@ -6,6 +6,15 @@
//
//===----------------------------------------------------------------------===//
+// TODO: Remove these UNSUPPORTED lines once we change how hardening is enabled to avoid
+// mutually exclusive modes being enabled at the same time.
+// UNSUPPORTED: libcpp-hardening-mode=hardened
+// UNSUPPORTED: libcpp-hardening-mode=safe
+// UNSUPPORTED: libcpp-hardening-mode=debug
+
+// TODO: Investigate
+// XFAIL: target={{.+}}-windows-{{.+}}
+
// Test that we encode the hardening mode in an ABI tag to avoid ODR violations
// when linking TUs that have different values for it.
>From 7faee0fdef95af5bf44cfcbfdb2e3e9e30ba1e79 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Tue, 24 Oct 2023 12:17:17 -0700
Subject: [PATCH 4/4] Change XFAILs for MSVC since it appears to only fail with
clang-cl
---
libcxx/test/libcxx/odr_signature.exceptions.sh.cpp | 2 +-
libcxx/test/libcxx/odr_signature.hardening.sh.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
index fb94ecd0244a271..6bf60b5e82d3c28 100644
--- a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
@@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
// TODO: Investigate
-// XFAIL: target={{.+}}-windows-{{.+}}
+// XFAIL: msvc
// Test that we encode whether exceptions are supported in an ABI tag to avoid
// ODR violations when linking TUs that have different values for it.
diff --git a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
index 0c939b4c4654228..3ae95c8910a9296 100644
--- a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
@@ -13,7 +13,7 @@
// UNSUPPORTED: libcpp-hardening-mode=debug
// TODO: Investigate
-// XFAIL: target={{.+}}-windows-{{.+}}
+// XFAIL: msvc
// Test that we encode the hardening mode in an ABI tag to avoid ODR violations
// when linking TUs that have different values for it.
More information about the libcxx-commits
mailing list