From 16b3352ae7b00e1d5b47d720a6ee7f9a0f790805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Chojnowski?= Date: Tue, 14 Jan 2025 12:57:58 +0100 Subject: [PATCH] build: fix -ffile-prefix-map cmake doesn't set a `-ffile-prefix-map` for source files. Among other things, this results in absolute paths in Scylla logs: ``` Jan 11 09:59:11.462214 longevity-tls-50gb-3d-master-db-node-2dcd4a4a-5 scylla[16339]: scylla: /jenkins/workspace/scylla-master/next/scylla/utils/refcounted.hh:23: utils::refcounted::~refcounted(): Assertion `_count == 0' failed. ``` And it results in absolute paths in gdb, which makes it a hassle to get gdb to display source code during debugging. (A build-specific `substitute-path` has to be configured for that). There is a `-file-prefix-map` rule for `CMAKE_BINARY_DIR`, but it's wrong. Patch dbb056f4f7fd0512ae18ed72d7612d5aa5eab817, which added it, was misguided. What we want is to strip the leading components of paths up to the repository directory, both in __FILE__ macros and in debug info. For example, we want to convert /home/michal/scylla/replica/table.cc to replica/table.cc or ./replica/table.cc, both in Scylla logs and in gdb. What the current rule does is it maps `/home/michal/scylla/build` to `.`, which is wrong: it doesn't do anything about the paths outside of `build`, which are the ones we actually care about. This patch fixes the problem. Closes scylladb/scylladb#22311 --- cmake/mode.common.cmake | 32 ++++++++++++++++++++++++++++++++ configure.py | 26 ++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/cmake/mode.common.cmake b/cmake/mode.common.cmake index 0099fd43c9..fde554d6e9 100644 --- a/cmake/mode.common.cmake +++ b/cmake/mode.common.cmake @@ -83,7 +83,39 @@ function(get_padded_dynamic_linker_option output length) set(${output} "${dynamic_linker_option}=${padded_dynamic_linker}" PARENT_SCOPE) endfunction() +# We want to strip the absolute build paths from the binary, +# so that logs and debuggers show e.g. ./main.cc, +# not /var/lib/jenkins/workdir/scylla/main.cc, or something. +# +# The base way to do that is -ffile-prefix-map=${CMAKE_SOURCE_DIR}/= +# But by itself, it results in *both* DW_AT_name and DW_AT_comp_dir being +# subject to the substitution. +# For example, if table::query() is located +# in /home/user/scylla/replica/table.cc, +# and the compiler working directory is /home/user/scylla/build, +# then after the ffile-prefix-map substitution it will +# have DW_AT_comp_dir equal to ./build +# and DW_AT_name equal to ./replica/table.cc +# +# If DW_AT_name is a relative path, gdb looks for the source files in $DW_AT_comp_dir/$DW_AT_name. +# This results in e.g. gdb looking for seastar::thread_context::main +# in ./build/./replica/table.cc +# instead of replica/table.cc as we would like. +# To unscrew this, we have to add a rule which will +# convert the /absolute/path/to/build to `.`, +# which will result in gdb looking in ././replica/table.cc, which is fine. +# +# The build rule which converts `/absolute/path/to/build/` (note trailing slash) +# to `build/` exists just so that any DW_AT_name under build (e.g. in generated sources) +# is excluded from the first rule. +# +# Note that the order of these options is important. +# Each is strictly more specific than the previous one. +# If they were the other way around, only the most general rule would be used. +add_compile_options("-ffile-prefix-map=${CMAKE_SOURCE_DIR}/=") add_compile_options("-ffile-prefix-map=${CMAKE_BINARY_DIR}=.") +cmake_path(GET CMAKE_BINARY_DIR FILENAME build_dir_name) +add_compile_options("-ffile-prefix-map=${CMAKE_BINARY_DIR}/=${build_dir_name}") default_target_arch(target_arch) if(target_arch) diff --git a/configure.py b/configure.py index 4610030970..cd5651c43b 100755 --- a/configure.py +++ b/configure.py @@ -1891,6 +1891,29 @@ def configure_seastar(build_dir, mode, mode_config): for flag in COVERAGE_INST_FLAGS: seastar_cxx_ld_flags = seastar_cxx_ld_flags.replace(' ' + flag, '') seastar_cxx_ld_flags = seastar_cxx_ld_flags.replace(flag, '') + # There is a global `-ffile-prefix-map={curdir}=.` above. + # By itself, it results in *both* DW_AT_name and DW_AT_comp_dir being + # subject to the substitution. + # For example, if seastar::thread_context::main is located + # in /home/user/scylla/seastar/src/core/thread.cc, + # and the compiler working directory is /home/user/scylla/seastar/build/seastar, + # then after the ffile-prefix-map substitution it will + # have DW_AT_comp_dir equal to ./build/seastar + # and DW_AT_name equal to ./seastar/src/core/thread.cc + # + # If DW_AT_name is a relative path, gdb looks for the source files in $DW_AT_comp_dir/$DW_AT_name. + # This results in e.g. gdb looking for seastar::thread_context::main + # in ./build/seastar/./seastar/src/core/thread.cc, + # instead of seastar/src/core/thread.cc as we would like. + # To unscrew this, we have to add a rule which will + # convert the /absolute/path/to/build/seastar to `.`, + # which will result in gdb looking in ././seastar/src/core/thread.cc, which is fine. + # + # The second build rule, which converts `/absolute/path/to/build/seastar/` (note trailing slash) + # to seastar/ exists just so any possible DW_AT_name under build (e.g. if there are some generated + # sources) is excluded from the first rule. + seastar_build_dir = os.path.join(build_dir, mode, 'seastar') + extra_file_prefix_map = f' -ffile-prefix-map={seastar_build_dir}=. -ffile-prefix-map={seastar_build_dir}/=seastar/' seastar_cmake_args = [ '-DCMAKE_BUILD_TYPE={}'.format(mode_config['cmake_build_type']), '-DCMAKE_C_COMPILER={}'.format(args.cc), @@ -1898,7 +1921,7 @@ def configure_seastar(build_dir, mode, mode_config): '-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON', '-DCMAKE_CXX_STANDARD=23', '-DCMAKE_CXX_EXTENSIONS=ON', - '-DSeastar_CXX_FLAGS=SHELL:{}'.format(mode_config['lib_cflags']), + '-DSeastar_CXX_FLAGS=SHELL:{}'.format(mode_config['lib_cflags'] + extra_file_prefix_map), '-DSeastar_LD_FLAGS={}'.format(semicolon_separated(mode_config['lib_ldflags'], seastar_cxx_ld_flags)), '-DSeastar_API_LEVEL=7', '-DSeastar_DEPRECATED_OSTREAM_FORMATTERS=OFF', @@ -1927,7 +1950,6 @@ def configure_seastar(build_dir, mode, mode_config): seastar_cmake_args += ['-DBUILD_SHARED_LIBS=ON'] cmake_args = seastar_cmake_args[:] - seastar_build_dir = os.path.join(build_dir, mode, 'seastar') seastar_cmd = ['cmake', '-G', 'Ninja', real_relpath(args.seastar_path, seastar_build_dir)] + cmake_args cmake_dir = seastar_build_dir if dpdk: