I'm not sure if this is a good reduction, we'll see. We keep getting reports of LTO causing undefined references with gcc+bfd when testsuites use -Wl,--wrap. I picked OpenVPN to reduce after a comment from one of its upstream developers: https://bugs.gentoo.org/877741#c4. argv.i: ``` void _cmocka_run_group_tests() {} void parse_line(); void argv_parse_cmd() { parse_line(); } ``` argv_testdriver-test_argv.i: ``` struct CMUnitTest { void *test_func; } _cmocka_run_group_tests(); void argv_parse_cmd(); void __wrap_parse_line() {}; void argv_parse_cmd__command_and_extra_options__argc_correct() { argv_parse_cmd(); } struct CMUnitTest main_tests = { argv_parse_cmd__command_and_extra_options__argc_correct }; int main() { _cmocka_run_group_tests(main_tests); } ``` ``` $ gcc argv_testdriver-test_argv.i argv.i -Wl,-wrap=parse_line -O3 -flto -fuse-ld=bld # broken /usr/lib/gcc/x86_64-pc-linux-gnu/15/../../../../x86_64-pc-linux-gnu/bin/ld.bfd: /tmp/ccjmsCeq.ltrans0.ltrans.o: in function `argv_parse_cmd__command_and_extra_options__argc_correct': <artificial>:(.text+0x13): undefined reference to `__wrap_parse_line' $ gcc argv_testdriver-test_argv.i argv.i -Wl,-wrap=parse_line -O3 -flto -fuse-ld=mold # fine $ clang argv_testdriver-test_argv.i argv.i -Wl,-wrap=parse_line -O3 -flto -fuse-ld=mold # fine argv_testdriver-test_argv.i:16:37: warning: passing arguments to '_cmocka_run_group_tests' without a prototype is deprecated in all versions of C and is not supported in C23 [-Wdeprecated-non-prototype] 16 | int main() { _cmocka_run_group_tests(main_tests); } | ^ 1 warning generated. $ clang argv_testdriver-test_argv.i argv.i -Wl,-wrap=parse_line -O3 -flto -fuse-ld=lld # fine argv_testdriver-test_argv.i:16:37: warning: passing arguments to '_cmocka_run_group_tests' without a prototype is deprecated in all versions of C and is not supported in C23 [-Wdeprecated-non-prototype] 16 | int main() { _cmocka_run_group_tests(main_tests); } | ^ 1 warning generated. ```
H.J, I'd appreciate your advice on - if it's not a bug - how to handle this in packages (what should I recommend upstreams do instead?) Thanks!
Alan's comment at https://sourceware.org/bugzilla/show_bug.cgi?id=22360#c3 and yours at https://sourceware.org/bugzilla/show_bug.cgi?id=25389#c1 make me think this is invalid, but it's not clear to me how to cleanly obscure the reference so it gets left unresolved.
A patch is posted at https://sourceware.org/pipermail/binutils/2024-July/135445.html
Thank you! That fixes linking OpenVPN tests. They *do* fail at runtime which I haven't yet investigated, but they fail in the same way with mold, so not caused by the patch. I'll try the other packages tonight.
(In reply to Sam James from comment #4) > Thank you! That fixes linking OpenVPN tests. They *do* fail at runtime which > I haven't yet investigated, but they fail in the same way with mold, so not > caused by the patch. I'll try the other packages tonight. iwd works but mold+bfd both fail with openvpn's tests at runtime as the wrong function gets called. Please wait a little bit while I reduce it into a clean testcase.
(In reply to Sam James from comment #5) Here's a failing runtime test case with the patch. tls_crypt_testdriver-buffer.c: ``` void buffer_write_file(void) { __builtin_printf("Called real buffer_write_file :(\n"); __builtin_abort(); } ``` tls_crypt_testdriver-crypto.c: ``` void buffer_write_file(void); void write_pem_key_file(const char *filename, const char *key_name) { buffer_write_file(); } ``` tls_crypt_testdriver-test_tls_crypt.c: ``` void write_pem_key_file(const char *filename, const char *key_name); void __wrap_buffer_write_file(void) { __builtin_printf("Called wrapped buffer_write_file :)\n"); } void test_tls_crypt_v2_write_server_key_file(void) { write_pem_key_file("testfilename.key", "OpenVPN tls-crypt-v2 server key"); } int main() { test_tls_crypt_v2_write_server_key_file(); } ``` gcc -Og -flto -Wl,--wrap=buffer_write_file -o tls_crypt_testdriver tls_crypt_testdriver-test_tls_crypt.c tls_crypt_testdriver-buffer.cc tls_crypt_testdriver-crypto.c It passes with lld but fails with mold and bfd.
(In reply to Sam James from comment #6) > (In reply to Sam James from comment #5) > > Here's a failing runtime test case with the patch. > > tls_crypt_testdriver-buffer.c: > ``` > void buffer_write_file(void) { > __builtin_printf("Called real buffer_write_file :(\n"); > __builtin_abort(); > } > ``` > > tls_crypt_testdriver-crypto.c: > ``` > void buffer_write_file(void); > > void write_pem_key_file(const char *filename, const char *key_name) { > buffer_write_file(); > } > ``` > > tls_crypt_testdriver-test_tls_crypt.c: > ``` > void write_pem_key_file(const char *filename, const char *key_name); > > void __wrap_buffer_write_file(void) { > __builtin_printf("Called wrapped buffer_write_file :)\n"); > } > > void test_tls_crypt_v2_write_server_key_file(void) { > write_pem_key_file("testfilename.key", "OpenVPN tls-crypt-v2 server key"); > } > > int main() { > test_tls_crypt_v2_write_server_key_file(); > } > ``` > > gcc -Og -flto -Wl,--wrap=buffer_write_file -o tls_crypt_testdriver > tls_crypt_testdriver-test_tls_crypt.c tls_crypt_testdriver-buffer.cc > tls_crypt_testdriver-crypto.c > > It passes with lld but fails with mold and bfd. I don't think this testcase is valid. LTO knows nothing about symbol wrap. When LTO sees a buffer_write_file call, it can inline it. To support symbol wrap, the original definition, buffer_write_file, shouldn't be visible to LTO.
Thanks H.J, I fear you're (obviously) right. I'll propose a shared library firewall for the real implementation when they run tests to OpenVPN. For my own curiosity: do you know why it works with clang -fuse-ld=lld but not clang -fuse-ld=mold or -fuse-ld=bfd? I accept that the principle of it is unsound but wondering if there's a latent bug it's showing?
The v2 patch is at https://sourceware.org/pipermail/binutils/2024-July/135471.html
(In reply to Sam James from comment #8) > Thanks H.J, I fear you're (obviously) right. > > I'll propose a shared library firewall for the real implementation when they > run tests to OpenVPN. > > For my own curiosity: do you know why it works with clang -fuse-ld=lld but > not clang -fuse-ld=mold or -fuse-ld=bfd? I accept that the principle of it > is unsound but wondering if there's a latent bug it's showing? It works with lld by accident.
Thanks!
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=eb7892c4019bd5d00175c0eb80eb0c5a47a42ca1 commit eb7892c4019bd5d00175c0eb80eb0c5a47a42ca1 Author: H.J. Lu <hjl.tools@gmail.com> Date: Tue Jul 9 01:30:19 2024 -0700 LTO: Properly check wrapper symbol Add wrapper_symbol to bfd_link_hash_entry and set it to true for wrapper symbol. Set wrap_status to wrapper if wrapper_symbol is true in LTO. Note: Calling unwrap_hash_lookup to check for the wrapper symbol works only when there is a definition for the wrapped symbol since references to the wrapped symbol have been redirected to the wrapper symbol. bfd/ PR ld/31956 * linker.c (bfd_wrapped_link_hash_lookup): Set wrapper_symbol for wrapper symbol. include/ PR ld/31956 * bfdlink.h (bfd_link_hash_entry): Add wrapper_symbol. ld/ PR ld/31956 * plugin.c (get_symbols): Set wrap_status to wrapper if wrapper_symbol is set. * testsuite/ld-plugin/lto.exp: Run PR ld/31956 tests. * testsuite/ld-plugin/pr31956a.c: New file. * testsuite/ld-plugin/pr31956b.c: Likewise. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Fixed for 2.43.
Sorry, one more question: richi's comment at https://sourceware.org/bugzilla/show_bug.cgi?id=24406#c10 made me wonder. If I modify https://sourceware.org/bugzilla/show_bug.cgi?id=31956#c6 to have noipa or noinline for buffer_write_file, the status is the same (just the hashes change): ``` --- a/tls_crypt_testdriver.res +++ b/tls_crypt_testdriver.res @@ -1,11 +1,11 @@ 3 tls_crypt_testdriver-tls_crypt_testdriver-test_tls_crypt.o 4 194 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF __wrap_buffer_write_file 196 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF_IRONLY test_tls_crypt_v2_write_server_key_file 200 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF main 220 [-648e684371714953-]{+cc01d97532c69e30+} RESOLVED_IR write_pem_key_file tls_crypt_testdriver-tls_crypt_testdriver-buffer.o 1 202 [-cdf5ef9747252c01-]{+a5f22943180ec17d+} PREVAILING_DEF_IRONLY buffer_write_file tls_crypt_testdriver-tls_crypt_testdriver-crypto.o 2 204 [-a3abc8ef456eac42-]{+e3adb2154884903b+} PREVAILING_DEF_IRONLY write_pem_key_file 207 [-a3abc8ef456eac42-]{+e3adb2154884903b+} RESOLVED_IR buffer_write_file ``` should it be PREVAILING_DEF instead for buffer_write_file, not PREVAILING_DEF_IRONLY if I've forced a real definition? (This makes no difference for the real-world testcase because I doubt OpenVPN would want to make mocked/wrapped functions noinline, but I'm interested in how the plugin interaction works nonetheless.)
(In reply to Sam James from comment #14) > Sorry, one more question: richi's comment at > https://sourceware.org/bugzilla/show_bug.cgi?id=24406#c10 made me wonder. > > If I modify https://sourceware.org/bugzilla/show_bug.cgi?id=31956#c6 to have > noipa or noinline for buffer_write_file, the status is the same (just the > hashes change): > ``` > --- a/tls_crypt_testdriver.res > +++ b/tls_crypt_testdriver.res > @@ -1,11 +1,11 @@ > 3 > tls_crypt_testdriver-tls_crypt_testdriver-test_tls_crypt.o 4 > 194 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF > __wrap_buffer_write_file > 196 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF_IRONLY > test_tls_crypt_v2_write_server_key_file > 200 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF main > 220 [-648e684371714953-]{+cc01d97532c69e30+} RESOLVED_IR write_pem_key_file > tls_crypt_testdriver-tls_crypt_testdriver-buffer.o 1 > 202 [-cdf5ef9747252c01-]{+a5f22943180ec17d+} PREVAILING_DEF_IRONLY > buffer_write_file > tls_crypt_testdriver-tls_crypt_testdriver-crypto.o 2 > 204 [-a3abc8ef456eac42-]{+e3adb2154884903b+} PREVAILING_DEF_IRONLY > write_pem_key_file > 207 [-a3abc8ef456eac42-]{+e3adb2154884903b+} RESOLVED_IR buffer_write_file > ``` > > should it be PREVAILING_DEF instead for buffer_write_file, not > PREVAILING_DEF_IRONLY if I've forced a real definition? No, since there are non-IR references. If there are non-IR references, PREVAILING_DEF will be generated.
(In reply to H.J. Lu from comment #15) > (In reply to Sam James from comment #14) > > Sorry, one more question: richi's comment at > > https://sourceware.org/bugzilla/show_bug.cgi?id=24406#c10 made me wonder. > > > > If I modify https://sourceware.org/bugzilla/show_bug.cgi?id=31956#c6 to have > > noipa or noinline for buffer_write_file, the status is the same (just the > > hashes change): > > ``` > > --- a/tls_crypt_testdriver.res > > +++ b/tls_crypt_testdriver.res > > @@ -1,11 +1,11 @@ > > 3 > > tls_crypt_testdriver-tls_crypt_testdriver-test_tls_crypt.o 4 > > 194 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF > > __wrap_buffer_write_file > > 196 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF_IRONLY > > test_tls_crypt_v2_write_server_key_file > > 200 [-648e684371714953-]{+cc01d97532c69e30+} PREVAILING_DEF main > > 220 [-648e684371714953-]{+cc01d97532c69e30+} RESOLVED_IR write_pem_key_file > > tls_crypt_testdriver-tls_crypt_testdriver-buffer.o 1 > > 202 [-cdf5ef9747252c01-]{+a5f22943180ec17d+} PREVAILING_DEF_IRONLY > > buffer_write_file > > tls_crypt_testdriver-tls_crypt_testdriver-crypto.o 2 > > 204 [-a3abc8ef456eac42-]{+e3adb2154884903b+} PREVAILING_DEF_IRONLY > > write_pem_key_file > > 207 [-a3abc8ef456eac42-]{+e3adb2154884903b+} RESOLVED_IR buffer_write_file > > ``` > > > > should it be PREVAILING_DEF instead for buffer_write_file, not > > PREVAILING_DEF_IRONLY if I've forced a real definition? > > No, since there are non-IR references. If there are non-IR references, > PREVAILING_DEF will be generated. But the linker itself, via --wrap will create a reference to the symbol. With PREVAILING_DEF_IRONLY the compiler is free to elide the symbol, so IMO that's invalid even if the linkers reference will later be "internal".
(In reply to Richard Biener from comment #16) > But the linker itself, via --wrap will create a reference to the symbol. > With PREVAILING_DEF_IRONLY the compiler is free to elide the symbol, > so IMO that's invalid even if the linkers reference will later be "internal" This is what I got with binutils 2.43: 194 af2188219cd4cf4 PREVAILING_DEF __wrap_cook 198 af2188219cd4cf4 PREVAILING_DEF main 214 af2188219cd4cf4 RESOLVED_IR cook
On Wed, 17 Jul 2024, hjl.tools at gmail dot com wrote: > https://sourceware.org/bugzilla/show_bug.cgi?id=31956 > > --- Comment #17 from H.J. Lu <hjl.tools at gmail dot com> --- > (In reply to Richard Biener from comment #16) > > > But the linker itself, via --wrap will create a reference to the symbol. > > With PREVAILING_DEF_IRONLY the compiler is free to elide the symbol, > > so IMO that's invalid even if the linkers reference will later be "internal" > > This is what I got with binutils 2.43: > > 194 af2188219cd4cf4 PREVAILING_DEF __wrap_cook > 198 af2188219cd4cf4 PREVAILING_DEF main > 214 af2188219cd4cf4 RESOLVED_IR cook That looks OK to me (I'm still on 2.41)
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3221c470f0765886a49a1a3d2ec602e4104a377b commit 3221c470f0765886a49a1a3d2ec602e4104a377b Author: H.J. Lu <hjl.tools@gmail.com> Date: Fri Aug 2 19:52:00 2024 -0700 LTO: Restore the wrapper symbol check for standard function Call unwrap_hash_lookup to restore the wrapper symbol check for standard function since reference to standard function may not show up in LTO symbol table: [hjl@gnu-tgl-3 pr31956-3]$ nm foo.o 00000000 T main U __real_malloc 00000000 T __wrap_malloc [hjl@gnu-tgl-3 pr31956-3]$ lto-dump -list foo.o Type Visibility Size Name function default 0 malloc function default 0 __real_malloc function default 3 main function default 5 __wrap_malloc [hjl@gnu-tgl-3 pr31956-3]$ make gcc -O2 -flto -Wall -c -o foo.o foo.c gcc -Wl,--wrap=malloc -O2 -flto -Wall -o x foo.o /usr/local/bin/ld: /tmp/ccsPW0a9.ltrans0.ltrans.o: in function `main': <artificial>:(.text.startup+0xa): undefined reference to `__wrap_malloc' collect2: error: ld returned 1 exit status make: *** [Makefile:22: x] Error 1 [hjl@gnu-tgl-3 pr31956-3]$ Also add a test to verify that the unused wrapper is removed. PR ld/31956 * plugin.c (get_symbols): Restore the wrapper symbol check for standard function. * testsuite/ld-plugin/lto.exp: Run the malloc test and the unused test. * testsuite/ld-plugin/pr31956c.c: New file. * testsuite/ld-plugin/pr31956d.c: New file. * testsuite/ld-plugin/pr31956d.d: New file. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
The binutils-2_43-branch branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2fbb863136724cfef2fc2f4c2b7f51be36f132c7 commit 2fbb863136724cfef2fc2f4c2b7f51be36f132c7 Author: H.J. Lu <hjl.tools@gmail.com> Date: Fri Aug 2 19:52:00 2024 -0700 LTO: Restore the wrapper symbol check for standard function Call unwrap_hash_lookup to restore the wrapper symbol check for standard function since reference to standard function may not show up in LTO symbol table: [hjl@gnu-tgl-3 pr31956-3]$ nm foo.o 00000000 T main U __real_malloc 00000000 T __wrap_malloc [hjl@gnu-tgl-3 pr31956-3]$ lto-dump -list foo.o Type Visibility Size Name function default 0 malloc function default 0 __real_malloc function default 3 main function default 5 __wrap_malloc [hjl@gnu-tgl-3 pr31956-3]$ make gcc -O2 -flto -Wall -c -o foo.o foo.c gcc -Wl,--wrap=malloc -O2 -flto -Wall -o x foo.o /usr/local/bin/ld: /tmp/ccsPW0a9.ltrans0.ltrans.o: in function `main': <artificial>:(.text.startup+0xa): undefined reference to `__wrap_malloc' collect2: error: ld returned 1 exit status make: *** [Makefile:22: x] Error 1 [hjl@gnu-tgl-3 pr31956-3]$ Also add a test to verify that the unused wrapper is removed. PR ld/31956 * plugin.c (get_symbols): Restore the wrapper symbol check for standard function. * testsuite/ld-plugin/lto.exp: Run the malloc test and the unused test. * testsuite/ld-plugin/pr31956c.c: New file. * testsuite/ld-plugin/pr31956d.c: New file. * testsuite/ld-plugin/pr31956d.d: New file. Signed-off-by: H.J. Lu <hjl.tools@gmail.com> (cherry picked from commit 3221c470f0765886a49a1a3d2ec602e4104a377b)
I fear this is still present, I think due to eb7892c4019bd5d00175c0eb80eb0c5a47a42ca1 and 3221c470f0765886a49a1a3d2ec602e4104a377b my testcase should work. Tested on: Gentoo: gcc 15.2.0 p5, ld.bfd 2.44-r1 Ubuntu 26.04dev: gcc 15.2.0-10ubuntu1 ld.bfd 2.45.50.20251201 +++ lib.c +++ #include <stdio.h> int lib() { printf("library function\n"); return 0; } +++ main.c +++ #include <stdio.h> int __real_lib(); int __wrap_lib() { printf("wrap lib\n"); __real_lib(); return 0; } int lib(); int main() { printf("running func\n"); lib(); return 0; } % gcc -Wl,--wrap=lib lib.c main.c -o prog && ./prog running func wrap lib library function % gcc -Wl,--wrap=lib -flto lib.c main.c -o prog && ./prog running func library function