Skip to content

Commit

Permalink
chore(profiling): fix sanitizers on Linux (#11424)
Browse files Browse the repository at this point in the history
In order for our tests to work with sanitizers,
* sanitizer lib needs to be propagated from the test fixture to the
dynamic libraries (e.g., `dd_wrapper`) (this was broken due to
underspecifying the link type)
* dynamic linker needs to be able to find the .so; the way we did it
worked OK on macos, but on Linux the libs are actually in a subdir

## Checklist
- [X] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
  • Loading branch information
sanchda authored Nov 16, 2024
1 parent 8e9edd3 commit f18fc0e
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,24 @@ function(add_ddup_config target)
# Some sanitizers (or the analysis--such as symbolization--tooling thereof) work better with frame pointers, so
# we include it here.
target_compile_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS} -fno-omit-frame-pointer)
target_link_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS})
target_link_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS} -shared-libsan)

# We need to do a little bit of work in order to ensure the dynamic *san libraries can be linked Procedure
# adapted from datadog/ddprof :)
execute_process(
COMMAND ${CMAKE_CXX_COMPILER} -print-file-name=
OUTPUT_VARIABLE LIBSAN_LIB_PATH
OUTPUT_STRIP_TRAILING_WHITESPACE COMMAND_ERROR_IS_FATAL ANY)
set_target_properties(${target} PROPERTIES INSTALL_RPATH ${LIBSAN_LIB_PATH} BUILD_RPATH ${LIBSAN_LIB_PATH})
# Locate all directories containing relevant `.so` files
execute_process(
COMMAND bash -c "find $(${CMAKE_CXX_COMPILER} -print-file-name=) -name '*.so' -exec dirname {} \; | uniq"
OUTPUT_VARIABLE LIBSAN_LIB_PATHS
OUTPUT_STRIP_TRAILING_WHITESPACE COMMAND_ERROR_IS_FATAL ANY)

# Print for debugging
message(STATUS "LIBSAN_LIB_PATHS: ${LIBSAN_LIB_PATHS}")

# Split the paths into a semicolon-separated list for CMake
string(REPLACE "\n" ";" LIBSAN_LIB_PATHS_LIST "${LIBSAN_LIB_PATHS}")

# Set RPATH to include all identified paths
set_target_properties(${target} PROPERTIES
BUILD_RPATH "${LIBSAN_LIB_PATHS_LIST}"
INSTALL_RPATH "${LIBSAN_LIB_PATHS_LIST}")
endif()

# If DO_FANALYZER is specified and we're using gcc, then we can use -fanalyzer
Expand Down

0 comments on commit f18fc0e

Please sign in to comment.