Skip to content

Commit a5a582d

Browse files
committed
Merge bitcoin/bitcoin#31998: depends: patch around PlacementNew issue in capnp
1ef22ce depends: patch around PlacementNew issue in capnp (fanquake) Pull request description: See #31772 and capnproto/capnproto#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 1ef22ce. 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 1ef22ce Tree-SHA512: 9c9ecf50c43cf74315f6659afab55aeafb436f70e83b328016ad574136dce46867220c6e1a85aefd8d3d3d027cd94cc807c79721a4983da9428de70f11224e52
2 parents eb9730a + 1ef22ce commit a5a582d

File tree

2 files changed

+76
-0
lines changed

2 files changed

+76
-0
lines changed

depends/packages/capnp.mk

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ $(package)_download_path=$(native_$(package)_download_path)
44
$(package)_download_file=$(native_$(package)_download_file)
55
$(package)_file_name=$(native_$(package)_file_name)
66
$(package)_sha256_hash=$(native_$(package)_sha256_hash)
7+
$(package)_patches=abi_placement_new.patch
78

89
define $(package)_set_vars :=
910
$(package)_config_opts := -DBUILD_TESTING=OFF
@@ -12,6 +13,10 @@ define $(package)_set_vars :=
1213
$(package)_cxxflags += -fdebug-prefix-map=$($(package)_extract_dir)=/usr -fmacro-prefix-map=$($(package)_extract_dir)=/usr
1314
endef
1415

16+
define $(package)_preprocess_cmds
17+
patch -p2 < $($(package)_patch_dir)/abi_placement_new.patch
18+
endef
19+
1520
define $(package)_config_cmds
1621
$($(package)_cmake) .
1722
endef
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
From 74560f26f75dda4257dce541ca362a1e763b2971 Mon Sep 17 00:00:00 2001
2+
From: Ryan Ofsky <[email protected]>
3+
Date: Thu, 6 Feb 2025 08:39:05 -0500
4+
Subject: [PATCH 1/1] Avoid gcc/clang ABI incompatibility caused by
5+
PlacementNew
6+
7+
GCC and clang do not use same calling convention for passing empty struct
8+
parameters. There is more information about this in
9+
https://itanium-cxx-abi.github.io/cxx-abi/cxx-abi-dev/archives/2015-December/002869.html
10+
11+
Unfortunately this can create an issue in capnproto if it is built without
12+
optimizations in GCC, and the resulting static libraries are used in a clang
13+
program, or vice versa.
14+
15+
Depending on what order libraries are specified on the linker command line, and
16+
whether code compiled with the other compiler is calling any header functions
17+
that cause weak a `operator new(unsigned int, kj::_::PlacementNew, void*)`
18+
symbol to be defined in its own objects, this can cause the linker to link a
19+
GCC-generated `kj::ctor` with a clang-generated `operator new`, and the
20+
resulting program to crash due to the compilers using different calling
21+
conventions for `operator new`.
22+
23+
This problem is difficult to avoid in general, but pretty easy to avoid here by
24+
changing `operator new` parameter order so the empty struct parameter is last.
25+
26+
This change should be beneficial for capnproto users that may be compiling it
27+
without optimizations, and not necessarily using a single compiler to build all
28+
their dependencies.
29+
30+
The problem does not occur if any optimizations are enabled because `operator
31+
new` calls are inlined in that case.
32+
---
33+
c++/src/kj/common.h | 11 +++++++----
34+
1 file changed, 7 insertions(+), 4 deletions(-)
35+
36+
diff --git a/c++/src/kj/common.h b/c++/src/kj/common.h
37+
index b8edde3c..28ab11d6 100644
38+
--- a/c++/src/kj/common.h
39+
+++ b/c++/src/kj/common.h
40+
@@ -1034,24 +1034,27 @@ private:
41+
42+
// We want placement new, but we don't want to #include <new>. operator new cannot be defined in
43+
// a namespace, and defining it globally conflicts with the definition in <new>. So we have to
44+
-// define a dummy type and an operator new that uses it.
45+
+// define a dummy type and an operator new that uses it. The dummy type is intentionally passed
46+
+// as the last parameter so clang and GCC ABI calling conventions for empty struct struct parameters
47+
+// are compatible, and there are not segfaults trying to call clang operator new/delete from GCC or
48+
+// vice versa.
49+
50+
namespace _ { // private
51+
struct PlacementNew {};
52+
} // namespace _ (private)
53+
} // namespace kj
54+
55+
-inline void* operator new(size_t, kj::_::PlacementNew, void* __p) noexcept {
56+
+inline void* operator new(size_t, void* __p, kj::_::PlacementNew) noexcept {
57+
return __p;
58+
}
59+
60+
-inline void operator delete(void*, kj::_::PlacementNew, void* __p) noexcept {}
61+
+inline void operator delete(void*, void* __p, kj::_::PlacementNew) noexcept {}
62+
63+
namespace kj {
64+
65+
template <typename T, typename... Params>
66+
inline void ctor(T& location, Params&&... params) {
67+
- new (_::PlacementNew(), &location) T(kj::fwd<Params>(params)...);
68+
+ new (&location, _::PlacementNew()) T(kj::fwd<Params>(params)...);
69+
}
70+
71+
template <typename T>

0 commit comments

Comments
 (0)