| Bug #115471 | Linker errors with enabled LTO | ||
|---|---|---|---|
| Submitted: | 1 Jul 5:04 | Modified: | 4 Jul 8:23 |
| Reporter: | Laurynas Biveinis (OCA) | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Compiling | Severity: | S3 (Non-critical) |
| Version: | 8.0.37, 8.4.0 | OS: | MacOS (14.5) |
| Assigned to: | CPU Architecture: | ARM | |
[1 Jul 5:04]
Laurynas Biveinis
[1 Jul 13:34]
Tor Didriksen
Posted by developer: The MySQL plugin architecture requires the server (mysqld) to export all symbols that may be needed by any plugin. On Windows and MacOS, symbol availability is verified at link time, on other platforms at plugin load time. For this particular case, adding TARGET_LINK_OPTIONS(mysqld PRIVATE "-Wl,-u,__Z25plugin_thdvar_safe_updateP3THDP7SYS_VARPPcPKc") is a trivial fix. However, we have other plugins which would require dozens of lines like this for symbols __Z16channel_stop_allilPNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE __Z23channel_get_credentialsPKcRNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES8_ __Z47set_replication_failover_channels_configurationRKNSt3__16vectorINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEENS4_IS6_EEEE etc.etc. which simply is not maintainable. So, sadly, LTO is currently broken on MacOS. SET_TARGET_PROPERTIES(mysqld PROPERTIES INTERPROCEDURAL_OPTIMIZATION TRUE) translates to "-flto=thin". I tried other variants, but they are broken too.
[1 Jul 14:48]
Laurynas Biveinis
I am not an expert in this area, so apologies if I'm overlooking something, but can the export of the needed symbols be ensured in the source by "__attribute__ ((visibility ("default"))" annotations?
[2 Jul 7:43]
Tor Didriksen
Posted by developer:
Everything in the server has visibility default (except for the strings library).
The problem is that the linker removes unused code.
So we must explicitly use the code, or tell the linker to keep it.
This patch will fix the 'example' and 'clone' plugins:
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 280d5358946..20f295a6262 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -8164,6 +8164,14 @@ static int init_server_components() {
dummy_function_to_ensure_we_are_linked_into_the_server();
assert(dummy == 1);
}
+ extern int clone_handle_drop();
+ extern int clone_handle_create(const char *plugin_name);
+ extern int clone_handle_check_drop(void*);
+ auto foo1 = clone_handle_drop;
+ auto foo2 = clone_handle_create;
+ auto foo3 = clone_handle_check_drop;
+ auto bar = plugin_thdvar_safe_update;
+ fprintf(stderr, "%p %p %p %p", foo1, foo2, foo3, bar);
not exactly pretty ...
[2 Jul 11:24]
Laurynas Biveinis
Some googling on how to prevent LTO removing symbols it believes are unused pointed me to "__attribute__((retain, used))". Applying it fixed example, clone, but not GR. The remaining errors are:
Undefined symbols for architecture arm64:
"get_server_id()", referenced from:
initialize_plugin_modules(std::__1::bitset<19ul>) in plugin.cc.o
configure_group_member_manager() in plugin.cc.o
set_auto_increment_handler_values() in plugin.cc.o
"is_gtid_committed(Gtid const&)", referenced from:
Certifier::certify(Gtid_set*, std::__1::list<char const*, std::__1::allocator<char const*>>*, bool, char const*, Gtid_log_event*, bool) in certifier.cc.o
Transaction_consistency_manager::handle_remote_prepare(binary_log::Uuid const*, long long, Gcs_member_identifier const&) in consistency_manager.cc.o
"get_connection_attrib()", referenced from:
Applier_module::initialize_applier_thread() in applier.cc.o
Autorejoin_thread::start_autorejoin(unsigned int, unsigned long long) in autorejoin.cc.o
Certifier_broadcast_thread::initialize() in certifier.cc.o
Delayed_initialization_thread::launch_initialization_thread() in delayed_plugin_initialization.cc.o
Group_action_coordinator::launch_group_action_handler_thread() in group_action_coordinator.cc.o
Transaction_monitor_thread::start() in group_actions_transaction_controller.cc.o
Group_partition_handling::launch_partition_handler_thread() in group_partition_handling.cc.o
...
"get_server_parameters(char**, unsigned int*, char**, unsigned int*, unsigned int*)", referenced from:
configure_group_member_manager() in plugin.cc.o
Advertised_recovery_endpoints::check(char const*, Advertised_recovery_endpoints::enum_log_context) in recovery_endpoints.cc.o
"group_replication_init()", referenced from:
plugin_group_replication_init(void*) in plugin.cc.o
"is_server_data_dropped()", referenced from:
Remote_clone_handler::clone_thread_handle() in remote_clone_handler.cc.o
Remote_clone_handler::evaluate_error_code(int) in remote_clone_handler.cc.o
"get_auto_increment_offset()", referenced from:
Plugin_group_replication_auto_increment::reset_auto_increment_variables(bool) in auto_increment.cc.o
Plugin_group_replication_auto_increment::set_auto_increment_variables(unsigned long, unsigned long) in auto_increment.cc.o
"set_auto_increment_offset(unsigned long)", referenced from:
Plugin_group_replication_auto_increment::reset_auto_increment_variables(bool) in auto_increment.cc.o
Plugin_group_replication_auto_increment::set_auto_increment_variables(unsigned long, unsigned long) in auto_increment.cc.o
"global_thd_manager_add_thd(THD*)", referenced from:
Applier_module::set_applier_thread_context() in applier.cc.o
Autorejoin_thread::autorejoin_thread_handle() in autorejoin.cc.o
Certifier_broadcast_thread::dispatcher() in certifier.cc.o
Delayed_initialization_thread::initialization_thread_handler() in delayed_plugin_initialization.cc.o
Group_action_coordinator::execute_group_action_handler() in group_action_coordinator.cc.o
Transaction_monitor_thread::transaction_thread_handle() in group_actions_transaction_controller.cc.o
Group_partition_handling::partition_thread_handler() in group_partition_handling.cc.o
...
"wait_for_gtid_set_committed(char const*, double, bool)", referenced from:
Transaction_consistency_manager::transaction_begin_sync_before_execution(unsigned int, enum_group_replication_consistency_level, unsigned long) const in consistency_manager.cc.o
"get_auto_increment_increment()", referenced from:
Plugin_group_replication_auto_increment::reset_auto_increment_variables(bool) in auto_increment.cc.o
Plugin_group_replication_auto_increment::set_auto_increment_variables(unsigned long, unsigned long) in auto_increment.cc.o
Plugin_gcs_events_handler::handle_joining_members(Gcs_view const&, bool, bool) const in gcs_event_handlers.cc.o
"set_auto_increment_increment(unsigned long)", referenced from:
Plugin_group_replication_auto_increment::reset_auto_increment_variables(bool) in auto_increment.cc.o
Plugin_group_replication_auto_increment::set_auto_increment_variables(unsigned long, unsigned long) in auto_increment.cc.o
"global_thd_manager_remove_thd(THD*)", referenced from:
Applier_module::clean_applier_thread_context() in applier.cc.o
Autorejoin_thread::autorejoin_thread_handle() in autorejoin.cc.o
Certifier_broadcast_thread::dispatcher() in certifier.cc.o
Delayed_initialization_thread::initialization_thread_handler() in delayed_plugin_initialization.cc.o
Group_action_coordinator::execute_group_action_handler() in group_action_coordinator.cc.o
Transaction_monitor_thread::transaction_thread_handle() in group_actions_transaction_controller.cc.o
Group_partition_handling::partition_thread_handler() in group_partition_handling.cc.o
...
"get_replica_max_allowed_packet()", referenced from:
Certification_handler::log_view_change_event_in_order(Pipeline_event*, Continuation*) in certification_handler.cc.o
"get_server_main_ssl_parameters(st_server_ssl_variables*)", referenced from:
build_gcs_parameters(Gcs_interface_parameters&) in plugin.cc.o
"get_server_admin_ssl_parameters(st_server_ssl_variables*)", referenced from:
build_gcs_parameters(Gcs_interface_parameters&) in plugin.cc.o
"get_server_encoded_gtid_executed(unsigned char**, unsigned long*)", referenced from:
Certifier_broadcast_thread::broadcast_gtid_executed() in certifier.cc.o
"get_max_replica_max_allowed_packet()", referenced from:
check_communication_max_message_size(THD*, SYS_VAR*, void*, st_mysql_value*) in plugin.cc.o
check_communication_max_message_size(THD*, SYS_VAR*, void*, st_mysql_value*) in plugin.cc.o
__GLOBAL__sub_I_plugin.cc in plugin.cc.o
"get_server_startup_prerequirements(Trans_context_info&)", referenced from:
plugin_group_replication_start(char**) in plugin.cc.o
"Transaction_context_log_event::get_event_length()", referenced from:
group_replication_trans_before_commit(Trans_param*) in observer_trans.cc.o
"Gtid_set::equals(Gtid_set const*) const", referenced from:
Certifier::certify(Gtid_set*, std::__1::list<char const*, std::__1::allocator<char const*>>*, bool, char const*, Gtid_log_event*, bool) in certifier.cc.o
Certifier::garbage_collect() in certifier.cc.o
Certifier::set_certification_info(std::__1::map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>>>*) in certifier.cc.o
ld: symbol(s) not found for architecture arm64
[2 Jul 11:34]
Laurynas Biveinis
Bug 115471 partial workaround for 8.0.38
Attachment: bug115741-8.0.38-partial-workaround.patch (application/octet-stream, text), 31.89 KiB.
[3 Jul 8:21]
Tor Didriksen
Posted by developer:
I didn't know about the 'retain' attribute, thanks for the tip.
Moving all those annotations to the .cc files, rather than the header files, it is fairly easy to write patch which covers all plugins, including rewriter and thread-pool
(I did it on current head of trunk.)
Did you benchmark the result, is the server any faster?
I would like to do this
STRING_APPEND(CMAKE_C_FLAGS " -flto=thin")
STRING_APPEND(CMAKE_CXX_FLAGS " -flto=thin")
like we do for other Unix platforms, but:
Linking CXX shared library library_output_directory/libserver_unittest_library.dylib
ld: invalid use of branch fixup in 'thread-local initialization routine for ut::this_thread_hash' to 'thread-local initialization routine for ut::this_thread_hash'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[3 Jul 8:27]
Tor Didriksen
Posted by developer: + STRING_APPEND(CMAKE_C_FLAGS " -flto=thin") + STRING_APPEND(CMAKE_CXX_FLAGS " -flto=thin") oops, internal linker error when linking mysqld ld: Assertion failed: (0 && "lto symbol should not be in layout"), function symbolForAtom, file Layout.cpp, line 1381.
[3 Jul 12:36]
Tor Didriksen
Posted by developer: So I experimented a bit more. Adding __attribute__((retain, used)) in about 150 different source locations, so that this works: SET_TARGET_PROPERTIES(mysqld PROPERTIES INTERPROCEDURAL_OPTIMIZATION TRUE) I see no performance improvement. /sysbench run --num-threads=10 --max-requests=100000 --test=oltp ..... Then I added this + STRING_APPEND(CMAKE_C_FLAGS " -flto=thin") + STRING_APPEND(CMAKE_CXX_FLAGS " -flto=thin") + STRING_APPEND(CMAKE_EXE_LINKER_FLAGS " -Wl,-ld_classic") xcodebuild -version Xcode 15.0.1 apparently has an LTO bug, hence the 'ld_classic' I had to do a few C++ code changes in order to make things build/link. With that I see about 2% performance improvement. So I guess my conclusion is "will not fix".
[4 Jul 8:23]
Laurynas Biveinis
Thank you for looking into this. I never did this for benchmarks, but, since successful LTO requires more conformant code base, to get diagnostics for ODR violations on macOS builds before I try Linux ones.
