diff --git a/.github/workflows/build_python_3.yml b/.github/workflows/build_python_3.yml index 18c6bd60b45..6b718bb4502 100644 --- a/.github/workflows/build_python_3.yml +++ b/.github/workflows/build_python_3.yml @@ -94,9 +94,6 @@ jobs: CIBW_BEFORE_ALL_MACOS: rustup target add aarch64-apple-darwin CIBW_BEFORE_ALL_LINUX: | if [[ "$(uname -m)-$(uname -i)-$(uname -o | tr '[:upper:]' '[:lower:]')-$(ldd --version 2>&1 | head -n 1 | awk '{print $1}')" != "i686-unknown-linux-musl" ]]; then - if command -v yum &> /dev/null; then - yum install -y libatomic.i686 - fi curl -sSf https://sh.rustup.rs | sh -s -- -y; fi CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24 CMAKE_ARGS="-DNATIVE_TESTING=OFF" SETUPTOOLS_SCM_PRETEND_VERSION_FOR_DDTRACE=${{ needs.compute_version.outputs.library_version }} diff --git a/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake b/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake index 1a7d66493c9..90e1fbd3704 100644 --- a/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake +++ b/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake @@ -1,9 +1,9 @@ include(CheckIPOSupported) function(add_ddup_config target) - # Profiling native extensions are built with C++17, even though underlying repo adheres to the manylinux 2014 + # Profiling native extensions are built with C++20, even though underlying repo adheres to the manylinux 2014 # standard. This isn't currently a problem, but if it becomes one, we may have to structure the library differently. - target_compile_features(${target} PUBLIC cxx_std_17) + target_compile_features(${target} PUBLIC cxx_std_20) # Common compile options target_compile_options( diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt index d48435dfa51..a2ad965873b 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt @@ -1,4 +1,12 @@ cmake_minimum_required(VERSION 3.19) + +# Disable testing by default (can be overridden with -DBUILD_TESTING=ON) Don't use FORCE to allow command-line overrides +if(NOT DEFINED BUILD_TESTING) + set(BUILD_TESTING + OFF + CACHE BOOL "Build the testing tree") +endif() + project( dd_wrapper VERSION 0.1.1 diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt index e5fcaef782d..ade703ca4ad 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt @@ -40,14 +40,23 @@ function(dd_wrapper_add_test name) add_ddup_config(${name}) target_link_options(${name} PRIVATE -Wl,--no-as-needed) + # Test executable needs to find libdd_wrapper in parent directory Append to INSTALL_RPATH instead of replacing to + # preserve sanitizer library paths set by add_ddup_config + get_target_property(EXISTING_INSTALL_RPATH ${name} INSTALL_RPATH) + if(EXISTING_INSTALL_RPATH) + set_target_properties(${name} PROPERTIES INSTALL_RPATH "${EXISTING_INSTALL_RPATH};$ORIGIN/.." + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE) + endif() + gtest_discover_tests( - ${name} + ${name} DISCOVERY_MODE PRE_TEST # Delay test discovery until test execution to avoid running sanitizer-built + # executables during build PROPERTIES # We start new threads after fork(), and we want to continue running the tests after that instead of # dying. ENVIRONMENT "TSAN_OPTIONS=die_after_fork=0:suppressions=${CMAKE_CURRENT_SOURCE_DIR}/TSan.supp") - set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/..") - if(LIB_INSTALL_DIR) install(TARGETS ${name} RUNTIME DESTINATION ${LIB_INSTALL_DIR}/../test) endif() diff --git a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt index dbe4395cc16..c6780223053 100644 --- a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt @@ -68,11 +68,23 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES PREFIX "") set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "") # RPATH is needed for sofile discovery at runtime, since Python packages are not installed in the system path. This is -# typical. +# typical. Use BUILD_WITH_INSTALL_RPATH to avoid rpath manipulation during install phase which can cause conflicts For +# standalone/test builds, also include LIB_INSTALL_DIR so tests can find libdd_wrapper if(APPLE) - set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..") + if(BUILD_TESTING AND LIB_INSTALL_DIR) + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..;${LIB_INSTALL_DIR}" + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/.." BUILD_WITH_INSTALL_RPATH + TRUE) + endif() elseif(UNIX) - set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..") + if(BUILD_TESTING AND LIB_INSTALL_DIR) + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..;${LIB_INSTALL_DIR}" + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE) + endif() endif() target_include_directories( ${EXTENSION_NAME} diff --git a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt index 89d900134bc..7f8f7e4f837 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt @@ -1,5 +1,12 @@ cmake_minimum_required(VERSION 3.19) +# Disable testing by default (can be overridden with -DBUILD_TESTING=ON) Don't use FORCE to allow command-line overrides +if(NOT DEFINED BUILD_TESTING) + set(BUILD_TESTING + OFF + CACHE BOOL "Build the testing tree") +endif() + # The exact name of this extension determines aspects of the installation and build paths, which need to be kept in sync # with setup.py. Accordingly, take the name passed in by the caller, defaulting to "stack_v2" if needed. set(EXTENSION_NAME @@ -109,11 +116,23 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES PREFIX "") set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "") # RPATH is needed for sofile discovery at runtime, since Python packages are not installed in the system path. This is -# typical. +# typical. Use BUILD_WITH_INSTALL_RPATH to avoid rpath manipulation during install phase which can cause conflicts For +# standalone/test builds, also include LIB_INSTALL_DIR so tests can find libdd_wrapper if(APPLE) - set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..") + if(BUILD_TESTING AND LIB_INSTALL_DIR) + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..;${LIB_INSTALL_DIR}" + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/.." BUILD_WITH_INSTALL_RPATH + TRUE) + endif() elseif(UNIX) - set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..") + if(BUILD_TESTING AND LIB_INSTALL_DIR) + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..;${LIB_INSTALL_DIR}" + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE) + endif() endif() target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper Threads::Threads) diff --git a/ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt index 6cf4b2fe7a4..ea2bc39ef9d 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt @@ -40,11 +40,20 @@ function(dd_wrapper_add_test name) target_link_libraries(${name} PRIVATE ${Python3_LIBRARIES}) endif() - set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/../stack_v2") - add_ddup_config(${name}) - gtest_discover_tests(${name}) + # Test executable needs to find _stack_v2 in parent directory Append to INSTALL_RPATH instead of replacing to + # preserve sanitizer library paths set by add_ddup_config + get_target_property(EXISTING_INSTALL_RPATH ${name} INSTALL_RPATH) + if(EXISTING_INSTALL_RPATH) + set_target_properties(${name} PROPERTIES INSTALL_RPATH "${EXISTING_INSTALL_RPATH};$ORIGIN/.." + BUILD_WITH_INSTALL_RPATH TRUE) + else() + set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE) + endif() + + gtest_discover_tests(${name} DISCOVERY_MODE PRE_TEST) # Delay test discovery until test execution to avoid running + # sanitizer-built executables during build # This is supplemental artifact so make sure to install it in the right place if(INPLACE_LIB_INSTALL_DIR) diff --git a/ddtrace/profiling/collector/_memalloc.c b/ddtrace/profiling/collector/_memalloc.cpp similarity index 100% rename from ddtrace/profiling/collector/_memalloc.c rename to ddtrace/profiling/collector/_memalloc.cpp diff --git a/ddtrace/profiling/collector/_memalloc_heap.c b/ddtrace/profiling/collector/_memalloc_heap.cpp similarity index 99% rename from ddtrace/profiling/collector/_memalloc_heap.c rename to ddtrace/profiling/collector/_memalloc_heap.cpp index 5616fe0c0ab..67b71d5003e 100644 --- a/ddtrace/profiling/collector/_memalloc_heap.c +++ b/ddtrace/profiling/collector/_memalloc_heap.cpp @@ -156,7 +156,7 @@ heap_tracker_thaw_no_cpython(heap_tracker_t* heap_tracker, size_t* n_to_free) *n_to_free = heap_tracker->freezer.frees.count; if (*n_to_free > 0) { /* TODO: can we put traceback_t* directly in freezer.frees so we don't need new storage? */ - to_free = malloc(*n_to_free * sizeof(traceback_t*)); + to_free = static_cast(malloc(*n_to_free * sizeof(traceback_t*))); for (size_t i = 0; i < *n_to_free; i++) { traceback_t* tb = memalloc_heap_map_remove(heap_tracker->allocs_m, heap_tracker->freezer.frees.tab[i]); to_free[i] = tb; diff --git a/ddtrace/profiling/collector/_memalloc_heap_map.c b/ddtrace/profiling/collector/_memalloc_heap_map.cpp similarity index 96% rename from ddtrace/profiling/collector/_memalloc_heap_map.c rename to ddtrace/profiling/collector/_memalloc_heap_map.cpp index 18cb0d19beb..6f4c3443ed9 100644 --- a/ddtrace/profiling/collector/_memalloc_heap_map.c +++ b/ddtrace/profiling/collector/_memalloc_heap_map.cpp @@ -90,7 +90,7 @@ typedef struct memalloc_heap_map_iter_t memalloc_heap_map_t* memalloc_heap_map_new() { - memalloc_heap_map_t* m = calloc(sizeof(memalloc_heap_map_t), 1); + memalloc_heap_map_t* m = static_cast(calloc(sizeof(memalloc_heap_map_t), 1)); m->map = HeapSamples_new(0); return m; } @@ -104,7 +104,7 @@ memalloc_heap_map_size(memalloc_heap_map_t* m) traceback_t* memalloc_heap_map_insert(memalloc_heap_map_t* m, void* key, traceback_t* value) { - HeapSamples_Entry k = { key = key, value = value }; + HeapSamples_Entry k = { .key = key, .val = value }; HeapSamples_Insert res = HeapSamples_insert(&m->map, &k); traceback_t* prev = NULL; if (!res.inserted) { @@ -187,7 +187,7 @@ memalloc_heap_map_delete(memalloc_heap_map_t* m) memalloc_heap_map_iter_t* memalloc_heap_map_iter_new(memalloc_heap_map_t* m) { - memalloc_heap_map_iter_t* it = malloc(sizeof(memalloc_heap_map_iter_t)); + memalloc_heap_map_iter_t* it = static_cast(malloc(sizeof(memalloc_heap_map_iter_t))); if (it) { it->iter = HeapSamples_citer(&m->map); } diff --git a/ddtrace/profiling/collector/_memalloc_reentrant.c b/ddtrace/profiling/collector/_memalloc_reentrant.cpp similarity index 100% rename from ddtrace/profiling/collector/_memalloc_reentrant.c rename to ddtrace/profiling/collector/_memalloc_reentrant.cpp diff --git a/ddtrace/profiling/collector/_memalloc_reentrant.h b/ddtrace/profiling/collector/_memalloc_reentrant.h index d7e67823df9..2e803486754 100644 --- a/ddtrace/profiling/collector/_memalloc_reentrant.h +++ b/ddtrace/profiling/collector/_memalloc_reentrant.h @@ -7,7 +7,11 @@ #define _POSIX_C_SOURCE 200809L #include #include +#ifdef __cplusplus +#include +#else #include +#endif #include #include #endif diff --git a/ddtrace/profiling/collector/_memalloc_tb.c b/ddtrace/profiling/collector/_memalloc_tb.cpp similarity index 98% rename from ddtrace/profiling/collector/_memalloc_tb.c rename to ddtrace/profiling/collector/_memalloc_tb.cpp index a4518d2d632..b4990c3e9d0 100644 --- a/ddtrace/profiling/collector/_memalloc_tb.c +++ b/ddtrace/profiling/collector/_memalloc_tb.cpp @@ -63,7 +63,7 @@ memalloc_tb_buffer_pool_get(memalloc_tb_buffer_pool* pool, uint16_t max_nframe) pool->pool[pool->count - 1] = NULL; pool->count--; } else { - t = malloc(TRACEBACK_SIZE(max_nframe)); + t = static_cast(malloc(TRACEBACK_SIZE(max_nframe))); } return t; } @@ -238,7 +238,7 @@ memalloc_frame_to_traceback(PyFrameObject* pyframe, uint16_t max_nframe) } size_t traceback_size = TRACEBACK_SIZE(traceback_buffer->nframe); - traceback_t* traceback = PyMem_RawMalloc(traceback_size); + traceback_t* traceback = static_cast(PyMem_RawMalloc(traceback_size)); if (traceback) memcpy(traceback, traceback_buffer, traceback_size); diff --git a/ddtrace/profiling/collector/_utils.h b/ddtrace/profiling/collector/_utils.h index 86d17837fb5..26b26806e87 100644 --- a/ddtrace/profiling/collector/_utils.h +++ b/ddtrace/profiling/collector/_utils.h @@ -14,13 +14,25 @@ random_range(uint64_t max) #define DO_NOTHING(...) +#ifdef __cplusplus +#define p_new(type, count) static_cast(PyMem_RawMalloc(sizeof(type) * (count))) +#else #define p_new(type, count) PyMem_RawMalloc(sizeof(type) * (count)) +#endif + #define p_delete(mem_p) PyMem_RawFree(mem_p); // Allocate at least 16 and 50% more than requested to avoid allocating items one by one. #define p_alloc_nr(x) (((x) + 16) * 3 / 2) + +#ifdef __cplusplus +#define P_REALLOC_CAST(p, expr) static_cast(expr) +#else +#define P_REALLOC_CAST(p, expr) (expr) +#endif + #define p_realloc(p, count) \ do { \ - (p) = PyMem_RawRealloc((p), sizeof(*p) * (count)); \ + (p) = P_REALLOC_CAST(p, PyMem_RawRealloc((p), sizeof(*p) * (count))); \ } while (0) #define p_grow(p, goalnb, allocnb) \ diff --git a/setup.py b/setup.py index d1467ad1a8a..88b679aaa03 100644 --- a/setup.py +++ b/setup.py @@ -700,6 +700,26 @@ def build_extension(self, ext): return raise else: + # For the memalloc extension, dynamically add libdd_wrapper to extra_objects + if ext.name == "ddtrace.profiling.collector._memalloc" and CURRENT_OS in ("Linux", "Darwin"): + dd_wrapper_suffix: t.Optional[str] = sysconfig.get_config_var("EXT_SUFFIX") + + wrapper_dir: Path + if IS_EDITABLE or getattr(self, "inplace", False): + # Editable build: use source directory + wrapper_dir = Path(__file__).parent / "ddtrace" / "internal" / "datadog" / "profiling" + else: + # Non-editable build: use build directory + wrapper_dir = ( + Path(__file__).parent / Path(self.build_lib) / "ddtrace" / "internal" / "datadog" / "profiling" + ) + + wrapper_path: Path = wrapper_dir / f"libdd_wrapper{dd_wrapper_suffix}" + if wrapper_path.exists(): + wrapper_path_str: str = str(wrapper_path) + if wrapper_path_str not in ext.extra_objects: + ext.extra_objects.append(wrapper_path_str) + super().build_extension(ext) if COMPILE_MODE.lower() in ("release", "minsizerel"): @@ -792,6 +812,8 @@ def build_extension_cmake(self, ext: "CMakeExtension") -> None: if BUILD_PROFILING_NATIVE_TESTS: cmake_args += ["-DBUILD_TESTING=ON"] + else: + cmake_args += ["-DBUILD_TESTING=OFF"] # If this is an inplace build, propagate this fact to CMake in case it's helpful # In particular, this is needed for build products which are not otherwise managed @@ -1039,38 +1061,49 @@ def get_exts_for(name): if not IS_PYSTON: ext_modules: t.List[t.Union[Extension, Cython.Distutils.Extension, RustExtension]] = [ - Extension( - "ddtrace.profiling.collector._memalloc", - sources=[ - "ddtrace/profiling/collector/_memalloc.c", - "ddtrace/profiling/collector/_memalloc_tb.c", - "ddtrace/profiling/collector/_memalloc_heap.c", - "ddtrace/profiling/collector/_memalloc_reentrant.c", - "ddtrace/profiling/collector/_memalloc_heap_map.c", - ], - extra_compile_args=( - debug_compile_args - # If NDEBUG is set, assert statements are compiled out. Make - # sure we explicitly set this for normal builds, and explicitly - # _unset_ it for debug builds in case the CFLAGS from sysconfig - # include -DNDEBUG - + (["-DNDEBUG"] if not debug_compile_args else ["-UNDEBUG"]) - + ["-D_POSIX_C_SOURCE=200809L", "-std=c11"] - + fast_build_args - if CURRENT_OS != "Windows" - else ["/std:c11", "/experimental:c11atomics"] - ), - ), Extension( "ddtrace.internal._threads", sources=["ddtrace/internal/_threads.cpp"], extra_compile_args=( - ["-std=c++17", "-Wall", "-Wextra"] + fast_build_args + ["-std=c++20", "-Wall", "-Wextra"] + fast_build_args if CURRENT_OS != "Windows" else ["/std:c++20", "/MT"] ), ), ] + + # _memalloc uses cwisstable which is not supported on Windows + # Profiler extensions are only needed on Linux and macOS + if CURRENT_OS != "Windows": + ext_modules.append( + Extension( + "ddtrace.profiling.collector._memalloc", + sources=[ + "ddtrace/profiling/collector/_memalloc.cpp", + "ddtrace/profiling/collector/_memalloc_tb.cpp", + "ddtrace/profiling/collector/_memalloc_heap.cpp", + "ddtrace/profiling/collector/_memalloc_reentrant.cpp", + "ddtrace/profiling/collector/_memalloc_heap_map.cpp", + ], + include_dirs=[ + "ddtrace/internal/datadog/profiling/dd_wrapper/include", + ], + extra_link_args=( + ["-Wl,-rpath,$ORIGIN/../../internal/datadog/profiling", "-latomic"] + if CURRENT_OS == "Linux" + else ["-Wl,-rpath,@loader_path/../../internal/datadog/profiling"] + if CURRENT_OS == "Darwin" + else [] + ), + language="c++", + extra_compile_args=( + debug_compile_args + + (["-DNDEBUG"] if not debug_compile_args else ["-UNDEBUG"]) + + ["-D_POSIX_C_SOURCE=200809L", "-std=c++20"] + + fast_build_args + ), + ), + ) if platform.system() not in ("Windows", ""): ext_modules.append( Extension( @@ -1134,7 +1167,8 @@ def get_exts_for(name): "ddtrace.appsec._ddwaf": ["libddwaf/*/lib/libddwaf.*"], "ddtrace.appsec._iast._taint_tracking": ["CMakeLists.txt"], "ddtrace.internal.datadog.profiling": ( - ["libdd_wrapper*.*"] + ["ddtrace/internal/datadog/profiling/test/*"] if BUILD_PROFILING_NATIVE_TESTS else [] + ["libdd_wrapper*.*"] + + (["ddtrace/internal/datadog/profiling/test/*"] if BUILD_PROFILING_NATIVE_TESTS else []) ), }, zip_safe=False,