Skip to content

Commit cddb9c1

Browse files
authored
Disable unsafe identical code folding in BOLT (#622)
astral-sh/uv#13610 reported a misbehavior that is the result of a subclass of str incorrectly having its ->tp_as_number->nb_add slot filled in with the value of PyUnicode_Type->tp_as_sequence->sq_concat. There are some times when this is an appropriate thing to do iwhen subclassing, but this is not one of them. The logic to prevent it in this case relies on two helper functions in the file, wrap_binaryfunc and wrap_binaryfunc_l, having different addresses, even though they contain identical code. For some reason BOLT does not do this optimization in the shared library (even though those are static functions and not exported), so we only started seeing this in the static build. BOLT in LLVM 20+ supports "safe" code folding, which uses heuristics about relocations to determine whether a function's address is used in any way other than a call. This seems to be enough to fix the issue. Add a patch to switch to -icf=safe, submitted upstream as python/cpython#134642
1 parent bb8404c commit cddb9c1

File tree

2 files changed

+90
-0
lines changed

2 files changed

+90
-0
lines changed

cpython-unix/build-cpython.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,12 @@ if [ -n "${PYTHON_MEETS_MINIMUM_VERSION_3_12}" ]; then
274274
# https://github.com/python/cpython/issues/128514
275275
patch -p1 -i ${ROOT}/patch-configure-bolt-apply-flags-128514.patch
276276

277+
# Disable unsafe identical code folding. Objects/typeobject.c
278+
# update_one_slot requires that wrap_binaryfunc != wrap_binaryfunc_l,
279+
# despite the functions being identical.
280+
# https://github.com/python/cpython/pull/134642
281+
patch -p1 -i ${ROOT}/patch-configure-bolt-icf-safe.patch
282+
277283
# Tweak --skip-funcs to work with our toolchain.
278284
patch -p1 -i ${ROOT}/patch-configure-bolt-skip-funcs.patch
279285
fi
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
From 91fc5ae4a5a66a03931f8cd383abd2aa062bb0e9 Mon Sep 17 00:00:00 2001
2+
From: Geoffrey Thomas <geofft@ldpreload.com>
3+
Date: Sat, 24 May 2025 19:04:09 -0400
4+
Subject: [PATCH 1/1] Use only safe identical code folding with BOLT
5+
6+
"Identical code folding" (ICF) is the feature of an optimizer to find that two
7+
functions have the same code and that they can therefore be deduplicated
8+
in the binary. While this is usually safe, it can cause observable
9+
behavior differences if the program relies on the fact that the two
10+
functions have different addresses.
11+
12+
CPython relies on this in (at least) Objects/typeobject.c, which defines
13+
two functions wrap_binaryfunc() and wrap_binaryfunc_l() with the same
14+
implementation, and stores their addresses in the slotdefs array. If
15+
these two functions have the same address, update_one_slot() in that
16+
file will fill in slots it shouldn't, causing, for instances,
17+
classes defined in Python that inherit from some built-in types to
18+
misbehave.
19+
20+
As of LLVM 20 (llvm/llvm-project#116275), BOLT has a "safe ICF" mode,
21+
where it looks to see if there are any uses of a function symbol outside
22+
function calls (e.g., relocations in data sections) and skips ICF on
23+
such functions. The intent is that this avoids observable behavior
24+
differences but still saves storage as much as possible.
25+
26+
This version is about two months old at the time of writing. To support
27+
older LLVM versions, we have to turn off ICF entirely.
28+
29+
This problem was previously noticed for Windows/MSVC in #53093 (and
30+
again in #24098), where the default behavior of PGO is to enable ICF
31+
(which they expand to "identical COMDAT folding") and we had to turn it
32+
off.
33+
---
34+
configure | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
35+
configure.ac | 25 ++++++++++++++++++++++++-
36+
2 files changed, 73 insertions(+), 2 deletions(-)
37+
38+
diff --git a/configure.ac b/configure.ac
39+
index 8d939f07505..25737e3f9d6 100644
40+
--- a/configure.ac
41+
+++ b/configure.ac
42+
@@ -2129,6 +2129,29 @@ if test "$Py_BOLT" = 'true' ; then
43+
else
44+
AC_MSG_ERROR([merge-fdata is required for a --enable-bolt build but could not be found.])
45+
fi
46+
+
47+
+ py_bolt_icf_flag="-icf=safe"
48+
+ AC_CACHE_CHECK(
49+
+ [whether ${LLVM_BOLT} supports safe identical code folding],
50+
+ [py_cv_bolt_icf_safe],
51+
+ [
52+
+ saved_cflags="$CFLAGS"
53+
+ saved_ldflags="$LDFLAGS"
54+
+ CFLAGS="$CFLAGS_NODIST"
55+
+ LDFLAGS="$LDFLAGS_NODIST"
56+
+ AC_LINK_IFELSE(
57+
+ [AC_LANG_PROGRAM([[]], [[]])],
58+
+ [py_cv_bolt_icf_safe=no
59+
+ ${LLVM_BOLT} -icf=safe -o conftest.bolt conftest$EXEEXT >&AS_MESSAGE_LOG_FD 2>&1 dnl
60+
+ && py_cv_bolt_icf_safe=yes],
61+
+ [AC_MSG_FAILURE([could not compile empty test program])])
62+
+ CFLAGS="$saved_cflags"
63+
+ LDFLAGS="$saved_ldflags"
64+
+ ]
65+
+ )
66+
+ if test "$py_cv_bolt_icf_safe" = no; then
67+
+ py_bolt_icf_flag=""
68+
+ fi
69+
fi
70+
71+
dnl Enable BOLT of libpython if built.
72+
@@ -2184,7 +2207,7 @@ then
73+
-reorder-blocks=ext-tsp
74+
-reorder-functions=cdsort
75+
-split-functions
76+
-split-strategy=cdsplit
77+
- -icf=1
78+
+ ${py_bolt_icf_flag}
79+
-inline-all
80+
-split-eh
81+
-reorder-functions-use-hot-size
82+
--
83+
2.39.5 (Apple Git-154)
84+

0 commit comments

Comments
 (0)