From 439de454bcb49cb4dbee30938d30a2f5cd6f2f47 Mon Sep 17 00:00:00 2001 From: Alan King Date: Thu, 12 Dec 2024 13:23:24 -0500 Subject: [PATCH 1/3] [ 286] Build hook: Add --debug_build option Based on the --debug_build option in the build hook in the main iRODS repository. --- ...rtium_continuous_integration_build_hook.py | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/irods_consortium_continuous_integration_build_hook.py b/irods_consortium_continuous_integration_build_hook.py index 3f4a1f2..96ef0db 100644 --- a/irods_consortium_continuous_integration_build_hook.py +++ b/irods_consortium_continuous_integration_build_hook.py @@ -1,14 +1,11 @@ -from __future__ import print_function - +import argparse import glob import multiprocessing -import optparse import os import tempfile import irods_python_ci_utilities - def add_cmake_to_front_of_path(): cmake_path = '/opt/irods-externals/cmake3.21.4-0/bin' os.environ['PATH'] = os.pathsep.join([cmake_path, os.environ['PATH']]) @@ -61,25 +58,30 @@ def copy_output_packages(build_directory, output_root_directory): irods_python_ci_utilities.append_os_specific_directory(output_root_directory), lambda s:s.endswith(irods_python_ci_utilities.get_package_suffix())) -def main(build_directory, output_root_directory, irods_packages_root_directory, externals_directory): +def main(build_directory, output_root_directory, irods_packages_root_directory, externals_directory, debug_build=False): install_building_dependencies(externals_directory) if irods_packages_root_directory: irods_python_ci_utilities.install_irods_dev_and_runtime_packages(irods_packages_root_directory) build_directory = os.path.abspath(build_directory or tempfile.mkdtemp(prefix='irods_storage_tiering_plugin_build_directory')) - irods_python_ci_utilities.subprocess_get_output(['cmake', os.path.dirname(os.path.realpath(__file__))], check_rc=True, cwd=build_directory) + build_type = "Debug" if debug_build else "Release" + cmake_command = ['cmake', f"-DCMAKE_BUILD_TYPE={build_type}", os.path.dirname(os.path.realpath(__file__))] + print(cmake_command) + irods_python_ci_utilities.subprocess_get_output(cmake_command, check_rc=True, cwd=build_directory) irods_python_ci_utilities.subprocess_get_output(['make', '-j', str(multiprocessing.cpu_count()), 'package'], check_rc=True, cwd=build_directory) if output_root_directory: copy_output_packages(build_directory, output_root_directory) if __name__ == '__main__': - parser = optparse.OptionParser() - parser.add_option('--build_directory') - parser.add_option('--output_root_directory') - parser.add_option('--irods_packages_root_directory') - parser.add_option('--externals_packages_directory') - options, _ = parser.parse_args() + parser = argparse.ArgumentParser(description='Build unified storage tiering plugin.') + parser.add_argument('--build_directory') + parser.add_argument('--output_root_directory') + parser.add_argument('--irods_packages_root_directory') + parser.add_argument('--externals_packages_directory') + parser.add_argument('--debug_build', action='store_true') + args = parser.parse_args() - main(options.build_directory, - options.output_root_directory, - options.irods_packages_root_directory, - options.externals_packages_directory) + main(args.build_directory, + args.output_root_directory, + args.irods_packages_root_directory, + args.externals_packages_directory, + args.debug_build) From 59c2f9483af576b0c4147b859a255c9690bb019b Mon Sep 17 00:00:00 2001 From: Alan King Date: Thu, 19 Dec 2024 09:31:55 -0500 Subject: [PATCH 2/3] [ 287] Add support for Address Sanitizer (ASAN) Requires irods-dev, irods-runtime, and irods-server packages built with ASAN enabled. Also adds an option to the build hook to enable ASAN from the development environment. --- CMakeLists.txt | 30 ++++++++++++++++++- ...rtium_continuous_integration_build_hook.py | 12 +++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a91327..9197f7d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ set(IRODS_PACKAGE_REVISION "0") include(IrodsCXXCompiler) set(CMAKE_CXX_STANDARD ${IRODS_CXX_STANDARD}) -set(CMAKE_MODULE_LINKER_FLAGS_INIT "-Wl,--enable-new-dtags -Wl,--as-needed -Wl,-z,defs") +set(CMAKE_MODULE_LINKER_FLAGS_INIT "-Wl,--enable-new-dtags -Wl,--as-needed") set(CMAKE_MODULE_LINKER_FLAGS_RELEASE_INIT "-Wl,--gc-sections -Wl,-z,combreloc") include(IrodsRunpathDefaults) @@ -38,6 +38,34 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") add_compile_options($<$:-fpermissive>) endif() +option(IRODS_ENABLE_ADDRESS_SANITIZER "Enables detection of memory leaks and other features provided by Address Sanitizer." OFF) +if (IRODS_ENABLE_ADDRESS_SANITIZER) + # Make sure the correct llvm-symbolizer binary is available to Address Sanitizer. This binary + # allows debug symbols to be reported appropriately. There are two ways to do this: + # + # export PATH=/opt/irods-externals/clang13.0.1-0/bin:$PATH + # + # - or - + # + # export ASAN_SYMBOLIZER_PATH=/opt/irods-externals/clang13.0.1-0/bin/llvm-symbolizer + # + # detect_container_overflow is disabled to guard against false positives which occur when + # parts of the binary are compiled with ASAN and other parts are not. + add_compile_definitions(IRODS_ADDRESS_SANITIZER_DEFAULT_OPTIONS="log_path=/tmp/irods_storage_tiering_asan_output:detect_container_overflow=0") + add_compile_options( + -fsanitize=address + -fno-omit-frame-pointer + -fno-optimize-sibling-calls + -O1) + add_link_options( + -fsanitize=address + -fno-omit-frame-pointer + -fno-optimize-sibling-calls + -O1) +else() + set(CMAKE_MODULE_LINKER_FLAGS_INIT "${CMAKE_MODULE_LINKER_FLAGS_INIT} -Wl,-z,defs") +endif() + if (NOT DEFINED THREADS_PREFER_PTHREAD_FLAG) set(THREADS_PREFER_PTHREAD_FLAG TRUE) endif() diff --git a/irods_consortium_continuous_integration_build_hook.py b/irods_consortium_continuous_integration_build_hook.py index 96ef0db..b94bf7d 100644 --- a/irods_consortium_continuous_integration_build_hook.py +++ b/irods_consortium_continuous_integration_build_hook.py @@ -58,14 +58,16 @@ def copy_output_packages(build_directory, output_root_directory): irods_python_ci_utilities.append_os_specific_directory(output_root_directory), lambda s:s.endswith(irods_python_ci_utilities.get_package_suffix())) -def main(build_directory, output_root_directory, irods_packages_root_directory, externals_directory, debug_build=False): +def main(build_directory, output_root_directory, irods_packages_root_directory, externals_directory, debug_build=False, enable_asan=False): install_building_dependencies(externals_directory) if irods_packages_root_directory: irods_python_ci_utilities.install_irods_dev_and_runtime_packages(irods_packages_root_directory) build_directory = os.path.abspath(build_directory or tempfile.mkdtemp(prefix='irods_storage_tiering_plugin_build_directory')) build_type = "Debug" if debug_build else "Release" - cmake_command = ['cmake', f"-DCMAKE_BUILD_TYPE={build_type}", os.path.dirname(os.path.realpath(__file__))] - print(cmake_command) + cmake_options = [f"-DCMAKE_BUILD_TYPE={build_type}"] + if enable_asan: + cmake_options.append("-DIRODS_ENABLE_ADDRESS_SANITIZER=YES") + cmake_command = ['cmake', os.path.dirname(os.path.realpath(__file__))] + cmake_options irods_python_ci_utilities.subprocess_get_output(cmake_command, check_rc=True, cwd=build_directory) irods_python_ci_utilities.subprocess_get_output(['make', '-j', str(multiprocessing.cpu_count()), 'package'], check_rc=True, cwd=build_directory) if output_root_directory: @@ -78,10 +80,12 @@ def main(build_directory, output_root_directory, irods_packages_root_directory, parser.add_argument('--irods_packages_root_directory') parser.add_argument('--externals_packages_directory') parser.add_argument('--debug_build', action='store_true') + parser.add_argument("--enable_address_sanitizer", dest="enable_asan", action="store_true") args = parser.parse_args() main(args.build_directory, args.output_root_directory, args.irods_packages_root_directory, args.externals_packages_directory, - args.debug_build) + args.debug_build, + args.enable_asan) From 187cb564ffe71bdaf304747a26c44a8a4d8a240f Mon Sep 17 00:00:00 2001 From: Alan King Date: Wed, 18 Dec 2024 14:52:23 -0500 Subject: [PATCH 3/3] [ 288] Fix memleaks discovered by ASAN --- src/main.cpp | 4 ++++ src/storage_tiering.cpp | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 01b4289..e29ef65 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -21,6 +21,7 @@ #include "irods/private/storage_tiering/exec_as_user.hpp" #include +#include #include #undef LIST @@ -118,6 +119,7 @@ namespace { } dataObjInp_t data_obj_inp{}; + const auto free_cond_input = irods::at_scope_exit{[&data_obj_inp] { clearKeyVal(&data_obj_inp.condInput); }}; rstrcpy(data_obj_inp.objPath, _object_path.c_str(), MAX_NAME_LEN); data_obj_inp.createMode = getDefFileMode(); addKeyVal(&data_obj_inp.condInput, RESC_NAME_KW, _source_resource.c_str()); @@ -149,6 +151,7 @@ namespace { } dataObjInp_t obj_inp{}; + const auto free_cond_input = irods::at_scope_exit{[&obj_inp] { clearKeyVal(&obj_inp.condInput); }}; rstrcpy( obj_inp.objPath, _object_path.c_str(), @@ -192,6 +195,7 @@ namespace { const_cast(_attribute.c_str()), const_cast(ts.c_str()), ""}; + const auto free_cond_input = irods::at_scope_exit{[&avuOp] { clearKeyVal(&avuOp.condInput); }}; if (_comm->clientUser.authInfo.authFlag >= LOCAL_PRIV_USER_AUTH) { addKeyVal(&avuOp.condInput, ADMIN_KW, ""); diff --git a/src/storage_tiering.cpp b/src/storage_tiering.cpp index 2133fa4..accded9 100644 --- a/src/storage_tiering.cpp +++ b/src/storage_tiering.cpp @@ -757,8 +757,25 @@ namespace irods { }; execMyRuleInp_t exec_inp{}; - rstrcpy(exec_inp.myRule, rule_obj.dump().c_str(), META_STR_LEN); msParamArray_t* out_arr{}; + // Capture out_arr pointer by reference because it is still nullptr at this point. + const auto free_inputs_and_outputs = irods::at_scope_exit{[&exec_inp, &out_arr] { + clearKeyVal(&exec_inp.condInput); + + if (exec_inp.inpParamArray) { + // The second parameter with a value of 1 instructs the function to free the "inOutStruct". + clearMsParamArray(exec_inp.inpParamArray, 1); + std::free(exec_inp.inpParamArray); + } + + if (out_arr) { + // The second parameter with a value of 1 instructs the function to free the "inOutStruct". + clearMsParamArray(out_arr, 1); + std::free(out_arr); + } + }}; + + rstrcpy(exec_inp.myRule, rule_obj.dump().c_str(), META_STR_LEN); addKeyVal( &exec_inp.condInput , irods::KW_CFG_INSTANCE_NAME