Merge bitcoin/bitcoin#31998: depends: patch around PlacementNew issue in capnp

1ef22ce335 depends: patch around PlacementNew issue in capnp (fanquake)

Pull request description:

  See #31772 and https://github.com/capnproto/capnproto/pull/2235.

  Given there isn't agreement in #29796, pulled this out so it could be merged separately, and it's easier to run different test configurations externally.

  Closes #31772.

ACKs for top commit:
  ryanofsky:
    Code review ACK 1ef22ce335. Confirmed patch is identical to one merged upstream. Only change since last review was tweaking the file paths and commit data in the patch.
  TheCharlatan:
    ACK 1ef22ce335

Tree-SHA512: 9c9ecf50c43cf74315f6659afab55aeafb436f70e83b328016ad574136dce46867220c6e1a85aefd8d3d3d027cd94cc807c79721a4983da9428de70f11224e52
This commit is contained in:
merge-script 2025-03-13 08:52:36 +08:00
commit a5a582d852
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 76 additions and 0 deletions

View file

@ -4,6 +4,7 @@ $(package)_download_path=$(native_$(package)_download_path)
$(package)_download_file=$(native_$(package)_download_file) $(package)_download_file=$(native_$(package)_download_file)
$(package)_file_name=$(native_$(package)_file_name) $(package)_file_name=$(native_$(package)_file_name)
$(package)_sha256_hash=$(native_$(package)_sha256_hash) $(package)_sha256_hash=$(native_$(package)_sha256_hash)
$(package)_patches=abi_placement_new.patch
define $(package)_set_vars := define $(package)_set_vars :=
$(package)_config_opts := -DBUILD_TESTING=OFF $(package)_config_opts := -DBUILD_TESTING=OFF
@ -12,6 +13,10 @@ define $(package)_set_vars :=
$(package)_cxxflags += -fdebug-prefix-map=$($(package)_extract_dir)=/usr -fmacro-prefix-map=$($(package)_extract_dir)=/usr $(package)_cxxflags += -fdebug-prefix-map=$($(package)_extract_dir)=/usr -fmacro-prefix-map=$($(package)_extract_dir)=/usr
endef endef
define $(package)_preprocess_cmds
patch -p2 < $($(package)_patch_dir)/abi_placement_new.patch
endef
define $(package)_config_cmds define $(package)_config_cmds
$($(package)_cmake) . $($(package)_cmake) .
endef endef

View file

@ -0,0 +1,71 @@
From 74560f26f75dda4257dce541ca362a1e763b2971 Mon Sep 17 00:00:00 2001
From: Ryan Ofsky <ryan@ofsky.org>
Date: Thu, 6 Feb 2025 08:39:05 -0500
Subject: [PATCH 1/1] Avoid gcc/clang ABI incompatibility caused by
PlacementNew
GCC and clang do not use same calling convention for passing empty struct
parameters. There is more information about this in
https://itanium-cxx-abi.github.io/cxx-abi/cxx-abi-dev/archives/2015-December/002869.html
Unfortunately this can create an issue in capnproto if it is built without
optimizations in GCC, and the resulting static libraries are used in a clang
program, or vice versa.
Depending on what order libraries are specified on the linker command line, and
whether code compiled with the other compiler is calling any header functions
that cause weak a `operator new(unsigned int, kj::_::PlacementNew, void*)`
symbol to be defined in its own objects, this can cause the linker to link a
GCC-generated `kj::ctor` with a clang-generated `operator new`, and the
resulting program to crash due to the compilers using different calling
conventions for `operator new`.
This problem is difficult to avoid in general, but pretty easy to avoid here by
changing `operator new` parameter order so the empty struct parameter is last.
This change should be beneficial for capnproto users that may be compiling it
without optimizations, and not necessarily using a single compiler to build all
their dependencies.
The problem does not occur if any optimizations are enabled because `operator
new` calls are inlined in that case.
---
c++/src/kj/common.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/c++/src/kj/common.h b/c++/src/kj/common.h
index b8edde3c..28ab11d6 100644
--- a/c++/src/kj/common.h
+++ b/c++/src/kj/common.h
@@ -1034,24 +1034,27 @@ private:
// We want placement new, but we don't want to #include <new>. operator new cannot be defined in
// a namespace, and defining it globally conflicts with the definition in <new>. So we have to
-// define a dummy type and an operator new that uses it.
+// define a dummy type and an operator new that uses it. The dummy type is intentionally passed
+// as the last parameter so clang and GCC ABI calling conventions for empty struct struct parameters
+// are compatible, and there are not segfaults trying to call clang operator new/delete from GCC or
+// vice versa.
namespace _ { // private
struct PlacementNew {};
} // namespace _ (private)
} // namespace kj
-inline void* operator new(size_t, kj::_::PlacementNew, void* __p) noexcept {
+inline void* operator new(size_t, void* __p, kj::_::PlacementNew) noexcept {
return __p;
}
-inline void operator delete(void*, kj::_::PlacementNew, void* __p) noexcept {}
+inline void operator delete(void*, void* __p, kj::_::PlacementNew) noexcept {}
namespace kj {
template <typename T, typename... Params>
inline void ctor(T& location, Params&&... params) {
- new (_::PlacementNew(), &location) T(kj::fwd<Params>(params)...);
+ new (&location, _::PlacementNew()) T(kj::fwd<Params>(params)...);
}
template <typename T>