Dyninst 13#12
Draft
dgaliffiAMD wants to merge 514 commits into
Draft
Conversation
Update implementation for AMDGPU GFX908 based on ISA-SPEC 02/22/23 1. Fix use of wrong immediate length in sign_extend (bug-fix) 2. Add memory counter definitions (dsmem) 3. In functions IS_ENC_* use hex representation instead of int 4. Use isArrayIndexValid function to do boundary check for ENC_*_insn_table 5. Fix compile warning for shadowing num_elements in the decoder 6. Make case label in amdgpu_gfx90a_decoder_impl.C sorted
Update implementation for AMDGPU GFX90A based on ISA-SPEC 02/22/23 1. Fix use of wrong immediate length in sign_extend (bug-fix) 2. Add memory counter definitions 3. In functions IS_ENC_* use hex representation instead of int 4. Use isArrayIndexValid function to do boundary check for ENC_*_insn_table 5. Fix compile warning for shadowing num_elements in the decoder 6. case label in amdgpu_gfx90a_decoder_impl.C made sorted
* Add callback declarations for unknown instructions in InstructionDecoder * Update definition of callback interface This also makes 'unknown_instruction' inconstructible. * make isValid the same as isLegalInsn This makes it impossible to make an Instruction object from a failed decoding process. This only affects x86. * Detect illegal instructions sooner in doIA32Decode * Don't advance the buffer when an x86 decode fails * Use a ternary to avoid linker error on some gcc's Some versions of gcc don't inline std::min _and_ need to resolve maxInstructionLength.
# Changes to building Dyninst - Minimum CMake version is 3.14.0 - STERILE_BUILD is now deprecated - ENABLE_LTO was renamed to DYNINST_ENABLE_LTO - CMAKE_EXPORT_COMPILE_COMMANDS is no longer set - Platform detection is done natively in CMake - full support for Linux on x86, AMD64, ppc64le, and aarch64/ARMv8 - experimental support for 32-bit FreeBSD and Windows on x86 - Custom install targets <target>-install have been removed - Installation subpaths (bin, lib, include, etc.) are no longer user-configurable - Static versions of Dyninst libraries now depend on other static Dyninst libraries - For example, libDynElf.a now depends on libcommon.a, not libcommon.so - Libraries that cannot build with symlight now warn when LIGHTWEIGHT_SYMTAB=ON - User build options passed via CMAKE_<LANG>_FLAGS are correctly preserved and override the builtin options - <PackageName>_ROOT_DIR now implies <PackageName>_NO_SYSTEM_PATHS and sets <PackageName>_ROOT - This forces CMake to find the package at the given location or in CMAKE_PREFIX_PATH - When using CMake >= 3.16, searching CMAKE_PREFIX_PATH can be disabled with CMAKE_FIND_USE_CMAKE_PATH=OFF - For example, -DElfUtils_ROOT_DIR=/some/path sets ElfUtils_NO_SYSTEM_PATHS=ON and ElfUtils_ROOT=/some/path ## RPATH handling - CMP0060 is active and so libraries are linked by their full paths even in implicit directories (e.g., /usr/lib/foo.so instead of -lfoo) - Populate RPATHs for binaries in the build tree: set(CMAKE_SKIP_BUILD_RPATH FALSE) - Do not use the install path as the RPATH: set(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE) - $ORIGIN is used instead - Add paths to any directories outside the project that are in the linker search path or contain linked library files: set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) ## Third-party libraries (tpl) - The ability to build tpls from source has been removed - All libraries must have a CMakeConfig.cmake - Minimum versions - Boost: 1.71.0 - TBB/oneapi-tbb: 2019.9 - elfutils: 0.186 - An imported interface named Dyninst::<library> is created for each library - includes are marked SYSTEM so they don't produce warnings - exported in DyninstConfig.cmake as per https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html - pkgconfig is used to find elfutils and valgrind - if pkgconfig fails, a manual search is done - This can be disabled with ElfUtils_NO_SYSTEM_PATHS=ON # Changes to consuming Dyninst as a CMake project - All targets are in the Dyninst namespace; e.g., symtabAPI is now Dyninst::symtabAPI - Each third-party library has an associated imported interface named Dyninst::<library>, e.g., Dyninst::Boost - Users are encouraged to use these, if the same library is need in their applications - DYNINST_LIBRARIES has been removed - DYNINST_INCLUDE_DIR is now deprecated and will be removed in a future version - Use the provided targets, instead - DYNINST_INTERNAL_DEFINES is now deprecated and will be removed in a future version - DYNINST_PLATFORM is now deprecated and will be removed in a future version - find_package(Dyninst ... COMPONENTS ...) now works correctly - Version constraints for find_package(Dyninst X.Y.Z) now work correctly - Dyninst only guarantees ABI compatibility between major releases, so only the same major versions are compatible Users who are not ready to fully migrate to the new Dyninst CMake package may use the following to preserve backward compatibility: cmake if(TARGET Dyninst::common) foreach(t common symtabapi ...) add_library(${t} INTERFACE IMPORTED) target_link_libraries(${t} INTERFACE Dyninst::${t}) endforeach() endif() * Update minimum CMake version to 3.13.0 * Rename CMake files to prevent name collisions When consumed as a subproject, the CMake files could be imported into the parent project where the filenames could collide. * Prepend to CMAKE_MODULE_PATH instead of overwriting * Fix capitalization error in FindThread_DB This should silence the warning about Thread_Db versus Thread_DB * Require CMake package for TBB (dyninst#1322) * Make libdl/dbghelp private linkage * Remove TBB flags from toolkits that don't use TBB * Remove FindTBB.cmake All supported TBB versions ship as CMake packages, so this is no longer needed. * Remove from-source build option * Create an imported target for TBB used by Dyninst This is needed to force the include directories to be considered 'system' directories so that compiler warnings from TBB sources are ignored * Rename cmake/ThreadingBuildingBlocks.cmake -> cmake/tpls/DyninstTBB.cmake This is needed to keep the namespace clean for DyninstConfig.cmake * Export TBB as part of the Dyninst CMake package This is required by the CMake guidelines: https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html In particular, "All required dependencies of a package must also be found in the package configuration file" * Format DyninstTBB.cmake * Boost CMake modernization (dyninst#1330) * Remove FindBoost.cmake Use the one provided by CMake so we don't have to maintain this one. * Remove from-source build * Remove user-configurable version * Force use of multithreaded libraries We don't need to include Threads here. Boost will do that. * Disable use of statically-linked runtime * Remove Boost_DEBUG * Always enable searching system paths * Unify path calculations, pass to find_package as hints * Clean up comments * Use include_guard * Allow using Boost's CMake package It is now provided by default since 1.70.0 which is the current minimum acceptable version. * Remove rest of cache variables * Remove MSVC-specific template define We can add this back, if we find there are newer VCs still affected. * Make Dyninst::Boost imported interface target This is needed to make the include directories be "system" directories so that warnings in their headers do not propagate into Dyninst. * Do not add Boost as a dependency for all libs * Make find_package QUIET * Use Boost_* variables instead of calculating includes, libs, etc. * Make a header-only wrapper target * Add to Dyninst package * Update CMakeLists * Set Boost_NO_WARN_NEW_VERSIONS * Bump minimum version to 1.71.0 * Elfutils cmake modernization (dyninst#1333) * Rename FindLibDwarf -> FindLibDW * Update FindLibDW * Rename FindLibElf -> FindLibELF * Update FindLibELF * Create FindElfutils * Update FindLibDebuginfod * Update DyninstElfUtils * Update the CMakeLists to use new targets * Use CMP0074 in updated Find modules This enables use of <Package>_ROOT variables when find_package is invoked. * Provide default dummy interface target for ElfUtils::ElfUtils Needed for non-Unix platforms. * Fix rebase bug in CMakeLists.txt * Export DyninstElfUtils * Forward QUIET flag to pkg_check_modules * Forward version to pkg_check_modules * Use lib from pkg-config, if found * Clean up internal variables * Simplify cache variable handling * Use full linkage name for libs returned by pkg-config * Separate out dependent libraries in FindLibDW Some platforms include libelf as a dependency, but IMPORTED_LOCATION accepts only a single entry. Store the rest in IMPORTED_LINK_DEPENDENT_LIBRARIES. * Fix quoting bug in FindLibDW * Fix lib check in FindLibDW * Manually set PC_<XXX>_INCLUDE_DIRS when FindPkgConfig misses it FindPkgConfig uses the output from pkg-config --cflags-only-I <lib> to set PC_<XXX>_INCLUDE_DIRS. Because libelf is usually in a system directory, pkg-config will return nothing for this. FindPkgConfig stores the actual includedir variable from the PC file, so we can fetch it from there. * Libiberty cmake modernization (dyninst#1334) * LibIberty cmake modernization * Use INCLUDE_DIRS directly * Threaddb cmake modernization (dyninst#1338) * Update FindThread_DB * Update thread_db * Update docs URL * Use OpenMP target (dyninst#1339) This also provides a dummy target so we don't have to do any additional checking when USE_OpenMP=OFF. We only use OpenMP_CXX, so I didn't create a target for the other languages (C,Fortran). * Valgrind cmake modernization (dyninst#1340) * Update valgrind * Add version check in Find module * Remove Valgrind_LIBRARIES They are versioned by architecture, so are hard to nail down with find_library. We also don't need them (at least not yet). * Make dummy when ADD_VALGRIND_ANNOTATIONS=OFF * Add compile defs * Update CMakeLists.txt * Make the dummy IMPORTED * Fix bug with version handling in DyninstBoost * Use _min_version in DyninstBoost This is so the CI version check will work uniformly * Add existence check for Dyninst::Boost before creating target * Clean up find_package flag handling in Find modules * Make variable exports uniform across Find modules * Coalesce calls to set_target_properties in Find modules * Fix bug in DyninstElfutils when calling find_package LibDebuginfod * Make just one exported target in FindElfutils * Add target existence check in DyninstTBB * Add SYSTEM property to Elfutils includes * Remove export of DYNINST_LIBRARIES * Preserve user's module path when looking for Dyninst modules * Manually set legacy DYNINST_INCLUDE_DIR This is now deprecated. * Use CMAKE_CURRENT_LIST_DIR intead of DYNINST_CMAKE_DIR * Install Find modules for third-party libraries * Remove unused DyninstConfigVersion.cmake * Remove unneeded comments in DyninstOptions * Move all options to DyninstOptions * Remove modification of CMAKE_CONFIGURATION_TYPES We just support the usual configs. * Move Dyninst version strings to base CMakeLists.txt * Move internal includes into base CMakeListst.txt This ensures that all user options and internal settings are in place before third-party dependencies are configured. * Clean up 3rd party includes in base CMakeLists.txt * Remove unused version strings * Remove unneeded 'add_dependencies(common boost)' in base CMakeListst.txt This is now handled directly in CMake recipe for common. * Remove unused testsuite include in base CMakeLists.txt * Update the project declaration * Move setting of CMAKE_BUILD_TYPE to base CMakeLists.txt This keeps all CMake-level variables in one place. * Don't set CMAKE_EXPORT_COMPILE_COMMANDS The user should set this. * Move BUILD_SHARED_LIBS into base CMakeLists.txt * Remove unused INSTALL_DOC_DIR * Make cmake_minimum_required a FATAL_ERROR * Move rpath and shared lib settings into DyninstLibrary * Move installation items into DyninstInstall.cmake * Automatically generate DyninstConfigVersion.cmake * Use configure_package_config_file to generate DyninstConfig.cmake This will provide more utilities to make a more robust Config.cmake * Use INSTALL_INCLUDE_DIR to set DYNINST_INCLUDE_DIR * Use PACKAGE_INIT in Config.cmake.in Also use it for DYNINST_INCLUDE_DIR * Don't explicitly set Dyninst_FOUND The caller's find_package will do this. * Remove unused DyninstSystemPaths.cmake * Simplify visibility settings and move to DyninstLibrary * Move SYMREADER calculation to base CMakeLists.txt It is needed in several places before DyninstLibrary is included. * Merge platform calculations into a DyninstPlatform.cmake * Remove DyninstVisibility include * Moved configure_file back to base CMakeListst The ordering matters until common gets an explicit list of header files (currently uses a glob). * Remove DyninstConfigVersion.cmake.in This isn't needed anymore since the version file is generated automatically. * Remove custom target install cmake_install.cmake isn't intended to be used that way. * Remove install logic from dyninst_library That is now handled in DyninstInstall. * cmake-format: set tab size to 2 * cmake-format: don't format comments * Calculate platform from CMake-provided mechanisms These are equivalent to the existing bash, but more adaptable. * Map stringy names into CMake variables * Replace Windows platform check with DYNINST_OS_Windows * Replace Linux platform check with DYNINST_OS_Linux * Replace FreeBSD platform check with DYNINST_OS_FreeBSD * Replace x86 platform check with DYNINST_OS_x86_64 * Replace ppc platform check with DYNINST_ARCH_ppc64le * Replace aarch64 platform check with DYNINST_ARCH_aarch64 * Replace mangled platform checks with explicit DYNINST_{OS,ARCH} * Rename PLATFORM to DYNINST_PLATFORM and export it as legacy This is only used in the test suite. * Don't pass -m64 explicitly on ppc64le We only support 64-bit ppc, so this is redundant and non-portable. * Remove commented-out define for aarch64 * Simplify CapArchDef With the new variables, only one pass over the OS names is needed. * Allow FreeBSD to build on i386 I previously thought it was only allowed on x86_64, but there is an old platform called i386-unknown-freebsd7.2. * Add Windows to DYNINST_PLATFORM * Replace usage of WIN32 with DYNINST_OS_Windows * Replace usage of UNIX with DYNINST_OS_UNIX * Merge Linux+FreeBSD checks into UNIX check * symtabAPI - replace i386 check * Use 'option' instead of 'set(... CACHE ...)' * Deprecate STERILE_BUILD * DyninstOptions - use 'OFF' instead of 'NO' * Get rid of dyninst_link_private_library We require CMake >= 3.13.0 so this is no longer needed. * Rename SOVERSION to DYNINST_SOVERSION SOVERSION is a keyword. * Replace LIBVERSION with DYNINST_LIBVERSION For clarity and consistency. * Replace DYNINST_ROOT with PROJECT_SOURCE_DIR * Move library settings into DyninstLibrarySettings.cmake * DyninstLibrarySettings - don't make INSTALL_*_DIR absolute In 'install', they are relative to CMAKE_INSTALL_PREFIX by default. * DyninstLibrarySettings - don't make INSTALL_*_DIR cache variables There's no need to let the user modify these locations. * DyninstLibrarySettings - reuse INSTALL vars This just makes sure changes are propagated. * DyninstLibrarySettings - remove INSTALL_BIN_DIR It's not used. * DyninstLibrary - rename INSTALL_*_DIR -> DYNINST_INSTALL_*DIR This is more in line with the naming convention of GNUInstallDirs. We could use GNUInstallDirs directly, but there's no need to let the user configure the install directory layout. * DyninstLibrarySettings - Use 'Dyninst' instead of PROJECT_NAME This is the only placed PROJECT_NAME is used, so make it consistent. * DyninstLibrarySettings - update RPATH handling * DyninstLibrary - don't manually create cmake_install.cmake These are automatically created and installed by 'install'. * DyninstLibrary - Merge calls to set_target_properties * DyninstLibrary - Cleanup superfluous variable usage * DyninstLibrary - add Windows defines to targets instead of directories * DyninstLibrary - add LIGHTWEIGHT_SYMTAB, SW_ANALYSIS_STEPPER to targets * DyninstLibrary - add DYNINST_DIAGNOSTIC_NO_SUPPRESSIONS to targets * DyninstLibrary - export all targets to DyninstTargets.cmake * DyninstLibrary - don't treat each library as a separate component 'COMPONENT' in 'install' is used to group targets into "bags" of useful features that can be independently installed using cmake --install . --component .... That doesn't work for Dyninst. Note: This is orthogonal to the concept of a COMPONENT in find_package. * DyninstLibrary - remove CLEAN_DIRECT_OUTPUT We no longer offer per-library build targets. * DyninstLibrary - remove WITHOUT_SYMTAB_API and WITHOUT_SYMLITE These are never used. * DyninstLibrary - refactor global defines in 'dyninst_library' * DyninstLibrary - Move calculation of SYMREADER * DyninstLibrary - rename 'target' to '_target' 'target' is a keyword * DyninstLibrary - add named arguments * DyninstLibrary - rename SRC_LIST with _target_SOURCE_FILES The latter is created from the named argument 'SOURCE_FILES'. * DyninstLibrary - Replace ACTUAL_TARGETS with _all_targets * DyninstLibrary - make headers part of library declaration This tracks file changes to signal rebuilds. * DyninstLibrary - add links, properties, includes to all targets This synchronizes the SHARED and STATIC libraries. * DyninstLibrary - link private and public deps separately * DyninstLibrary - Set include directories on interfaces This replaces using PUBLIC_HEADERS which is really intended for use with MacOS Framework targets. * DyninstLibrary - remove LIBRARY_OUTPUT_DIRECTORY The default is CMAKE_CURRENT_BINARY_DIR, so no need to specify it. * DyninstLibrary - Apply DEFINES passed to dyninst_library * DyninstLibrary - make SHARED library explicit * DyninstLibrary - don't deref Boolean variable * DyninstLibrary - update message * DyninstLibrary - Install headers, preserving the directory structure * DyninstLibrary - "return" the list of generated target names * DyninstLibrary - add directory-level include guard * common - remove spurious files Added in 2015. * common - move generation of dyninstversion.h into CMakeLists.txt * common - add global include guard in CMakeLists * common - explicitly list header files in CMakeLists This makes it easier to see what the actual public files are. * common - refactor source list calculation * common - move include of DyninstLibrary into subdirectory * common - use new dyninst_library * common - set includes and Windows stuff on all targets * common - write 'dyninstverison.h' into source dir * elf - make a dummy target for non-Unix OSes * elf - use new dyninst_library * dwarf - use new dyninst_library The changes here are the same as for elf/CMakeLists * symlite - make symlite/h/SymLite-elf.h publicly consumable It refers to files in the build tree that aren't available in the install tree. Although this file is in the install tree, it was never consumable by users. Updating this is of questionable utility, but it at least gives us the opportunity to write tests against the public interface. * symlite - use new dyninst_library * instructionAPI - use new dyninst_library * symtabAPI - use new dyninst_library * proccontrol - use new dyninst_library * parseAPI - use full path to common/h/util.h. This prevents confusion with parseAPI/h/util.h. * parseAPI - use new dyninst_library This one is different from the rest because parseAPI and dataflowAPI have circular dependencies on each other, so it's not currently possible to split them up. * stackwalk - use new dyninst_library Previously, FORCE_FRAME_POINTER was applied to _all_ of Dyninst. The commit comments said it was supposed to be just for the stackwalk sources, so that is the new behavior. * stackwalk - incorporate check of SW_ANALYSIS_STEPPER It only affects stackwalk, so no need to put it on all of the targets. * stackwalk - put SW_ANALYSIS_STEPPER check in post-processing This tidies up the code a bit. * patchAPI - use new dyninst_library * dyninstAPI - use absolute include path for 'debug.h' * dyninstAPI - add missing include for Dyninst::Address This was transitively included, but breaks when reording includes. * dyninstAPI - add missing include for std::set * dyninstAPI - use new dyninst_library * dynC_API - use new dyninst_library * parseThat - use new dyninst_library * dyninstAPI_RT - use new dyninst_library 1. SRC_ASSEMBLY was never used. The assembly files have always been explicitly listed. 2. We are still linking libdl.so on the static versions of the runtime. This hasn't been an issue in the past, so we'll leave it. 3. The creation of the lists of source files has redundant checks in it, but this version is much clearer on what is actually happening. 4. CHECK_C_COMPILER_FLAG_EXTENDED was removed since CMake now supports a much larger set of compilers and Dyninst has dropped support for HP, Sun, and XL. 5. The config is much less noisy. There was no reason to determine if 32-bit support would work when the user didn't ask for it. Now, the config will fail if the user asks for 32-bit libraries and the compiler can't create them. * Remove top-level project includes All of the necessary includes are now handled by each toolkit via explicit import/export of targets and include directories. * Remove top-level check for SYMREADER The respective CMakeLists now handle this. * Add better handling of Threads/pthread 1. Move Threads include into a tpls module 2. Link against Threads::Threads instead of 'pthread' 3. Add explicit dependencies in dyninstAPI and dyninstAPI_RT (they were implicit before). * DyninstConfig - Correctly detect presence of COMPONENTS on import This previously didn't work at all because DYNINST_${COMP}_FOUND should have been spelled Dyninst_${COMP}_FOUND. This is simpler and allows the use of check_required_components directly. * DyninstConfig - Remove unneeded and (now) incorrect comments * Make an OpenMP tpl module This will make it easier to export OpenMP in DyninstConfig. It also makes CMakeLists.txt have just straight-line code. * Include all third-party libraries in DyninstConfig This is explicitly required by the CMake documentation: "All required dependencies of a package must also be found in the package configuration file." https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html This also ensures that the custom Dyninst:: targets are always available to consumers. * Ensure static libraries depend on static Dyninst libraries When creating a static Dyninst library, ensure that it depends on the corresponding static Dyninst library. For example, libDynElf.a should depend on libcommon.a, not libcommon.so. * DyninstWarnings - Ignore no-pragma warnings for non-Werror builds * DyninstCapArchDef - rename UNIFIED_DEFINES DYNINST_PLATFORM_CAPABILITIES is more descriptive. * DyninstCapArchDef - add include guard * DyninstLibrary - add DYNINST_PLATFORM_CAPABILITIES to all targets * DyninstLibrary - add documentation * DyninstLibrary - update comment about install dir structure * DyninstLibrary - install runtime in 'bin' * Include 'src' and 'h' subdirectories in BUILD_INTERFACE Every toolkit has a similar directory structure, so we can do this in just one location instead of in each CMakeLists. * Unswitch loops for platform-specific target updates in CMakeLists This is really a code beautification. * Remove DYNINST_LIBVERSION It's never different from DYNINST_VERSION and has no special purpose. * Remove system path detection in RPATH calculations We use CMAKE_BUILD_WITH_INSTALL_RPATH to ensure the install path is not in RPATH/RUNPATH, but then this code would add it back if the install directory isn't a system path. * DyninstLibrary - simplify regex for header install * Rename cmake/version.h.in to cmake/dyninstversion.h.in This makes the generated file match the generator file name. * DyninstLibrary - install from _target_PUBLIC_HEADER_FILES _public_headers was only incidentally present from the file including this one. * parseAPI - make headers PRIVATE in dyninst_library We manually install them because of how dataflowAPI is consumed. * Issues warning when toolkit cannot be built with LIGHTWEIGHT_SYMTAB * CMake modernization - update compiler flags (dyninst#1374) * Fix spelling of LibDW_INCLUDE_DIRS when marking it as advanced * Mark STERILE_BUILD as advanced * Mark Boost_DIR as advanced * Mark TBB_DIR as advanced * Mark Thread_DB_{INCLUDE_DIRS,LIBRARIES} and as advanced * Update README * Add gfx908 public headers These somehow didn't make it through the rebase * Make flags for RelWithDebInfo and Release match * Disable cmake-format for extra flags in DyninstWarnings * Propagate ElfUtils_ROOT_DIR to FindLib{ELF,DW,Debuginfod} * Use 'set' instead of 'option' for stringy options * Add more cmake-format exceptions in DyninstWarnings * Add <Package>_NO_SYSTEM_PATHS to elf, dw, debuginfod This is used to exclude system directories from the search process. * Have ElfUtils_ROOT_DIR override search paths If the user provides a value, then no system paths (aside from the ones in CMAKE_PREFIX_PATH) are searched. This lets the user 'force' a location for elfutils. * Remove explicit version fail check in elfutils find-modules * Move ElfUtils_ROOT_DIR check into DyninstElfutils * Add LibIberty_NO_SYSTEM_PATHS * Add Valgrind_NO_SYSTEM_PATHS * Remove explicit version check from FindValgrind * Boost - use new meaning of Boost_ROOT_DIR * Fix typo in DyninstElfUtils * Fix bug when setting path flags * TBB - use new meaning of TBB_ROOT_DIR * Add support for common/h/unaligned_memory_access.h * Set default DYNINST_LINKER to empty This will use the default linker. lld is the LLVM linker. * Make requesting an unknown component an error * Always build libdyninstAPI_RT.a This is always needed by the test suite and keeps the old behavior of having it built by default. * Add DYNINST_FORCE_RUNPATH option Setting this flag forces the linker to use RUNPATH instead of RPATH. This is most useful for working with older RedHat distros. * Reduce number of public link dependencies This reduces the number of transitive links that have to be done by binaries linking against Dyninst. * Make elfutils dependency public for symtab It's needed in the Module.h public header. * Require TBB >=2019.9 When building from source, versions before 2019.9 incorrectly set the version in TBBConfigVersion.cmake. For example, 2018.6 sets the version to 2018.0 because it uses the TBB_{MAJOR,MINOR}_VERSION from tbb_stddef.h instead of doing the calculation based on the engineering version. This also unifies the versions required when compiling with gcc and clang. TBB 2018.6 was released in Oct 2018 and 2019.9 was released in Oct 2019, so this just bumps the requirement by just a year even though there are at least 9 releases in between. * Update Boost version in docker/dependencies.versions * Update filenames in dependency-version CI check * Update variable names in dependency-version CI check * Make elfutils dependency public for dynElf * bump CMake minimum version to 3.14.0 The 3.13.* family requires every 'install' to specify a "LIBRARY DESTINATION". We don't need or want that in the custome parseAPI install (line ~130) for exporting the public headers. * Add cmake to dependency-version CI check * Rename cdna2 -> gfx90a in new layout * Always create list of static-only sources in dyninstAPI_RT * Add common/h to BUILD_INTERFACE in 32-bit libdyninstAPI_RT * Fix building with symLite The headers from symtabAPI are still required, even when building with symLite.
clang complains: 'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const' to avoid a change in behavior [-Werror,-Wconstexpr-not-const]
- when compiling with clang 15 and 16 in addition to clang 14, allow a 40000 stack frame size
* Move elfutils logic into build_elfutils.sh * Whitespace * Copy in dependencies.versions This is no longer done in the base container because it didn't make sense there as that image should never change.
Dyninst now requires TBB >= 2019.9.
* Weekly build across all compiler versions and build types * Use DYNINST_WARNINGS_AS_ERRORS=ON instead of -Werror
Github only allows one CPU core per job, but any number of threads. Testing shows that N=2 threads reduces build time by 2-2.5x, N=3 by 2.2x, and N=4 increases build time.
This makes it consistent with std::condition_variable. Found using cppcheck: common/src/dthread.h:114:4: warning: Class 'CondVar' does not have a copy constructor which is recommended since it has dynamic memory/resource allocation(s). [noCopyConstructor] mutex = new mutex_t; ^ common/src/dthread.h:114:4: warning: Class 'CondVar' does not have a operator= which is recommended since it has dynamic memory/resource allocation(s). [noOperatorEq] mutex = new mutex_t;
…eak (dyninst#1427) * AddressTranslateSysV::adjustForAddrSpaceWrap: Fix C file descriptor leak Found using cppcheck: common/src/addrtranslate-sysv.C:1046:58: error: Resource leak: fd [resourceLeak] if (read(fd, &e_hdr, sizeof(e_hdr)) != sizeof(e_hdr)) return base; ^ common/src/addrtranslate-sysv.C:1048:28: error: Resource leak: fd [resourceLeak] if (e_hdr.e_phoff == 0) return base;
* AddressTranslateWin
Found using cppcheck:
common/src/addrtranslate-win.C:49:17: style: Virtual function 'init' is called from constructor 'AddressTranslateWin(PID pid,PROC_HANDLE phandle)' at line 168. Dynamic binding is not used. [virtualCallInConstructor]
virtual bool init();
* NodeIteratorPredicateObj
Found using cppcheck:
common/src/NodeIterator.h:293:18: style: Virtual function 'inc' is
called from constructor
'NodeIteratorPredicateObj(Graph::NodePredicate::Ptr
p,NodeIterator&b,NodeIterator&e)' at line 331. Dynamic binding is not
used. [virtualCallInConstructor]
* NodeIteratorPredicateFunc
Found using cppcheck:
common/src/NodeIterator.h:352:18: style: Virtual function 'inc' is
called from constructor
'NodeIteratorPredicateFunc(Graph::NodePredicateFunc
p,void*u,NodeIterator&b,NodeIterator&e)' at line 394. Dynamic binding is
not used. [virtualCallInConstructor]
* InstructionDecoder_x86
Found using cppcheck:
instructionAPI/src/InstructionDecoder-x86.h:74:49: style: Virtual function 'setMode' is called from constructor 'InstructionDecoder_x86(Architecture a)' at line 144. Dynamic binding is not used. [virtualCallInConstructor]
INSTRUCTION_EXPORT virtual void setMode(bool is64);
^
instructionAPI/src/InstructionDecoder-x86.C:144:28: note: Calling setMode
if(a == Arch_x86_64) setMode(true);
^
instructionAPI/src/InstructionDecoder-x86.h:74:49: note: setMode is a virtual function
INSTRUCTION_EXPORT virtual void setMode(bool is64);
* CFGFactor::destroy_block
Found using cppcheck:
parseAPI/h/CFGFactory.h:117:18: style: Virtual function 'free_block' is called from destructor '~CFGFactory()' at line 92. Dynamic binding is not used. [virtualCallInConstructor]
virtual void free_block(Block * b);
^
parseAPI/src/CFGFactory.C:92:5: note: Calling destroy_block
destroy_block(b);
^
parseAPI/src/CFGFactory.C:182:5: note: Calling free_block
free_block(b);
^
parseAPI/h/CFGFactory.h:117:18: note: free_block is a virtual function
virtual void free_block(Block * b);
* CFGFactor::free_func
Found using cppcheck:
parseAPI/h/CFGFactory.h:116:18: style: Virtual function 'free_func' is called from destructor '~CFGFactory()' at line 95. Dynamic binding is not used. [virtualCallInConstructor]
virtual void free_func(Function * f);
^
parseAPI/src/CFGFactory.C:95:5: note: Calling destroy_func
destroy_func(f);
^
parseAPI/src/CFGFactory.C:172:4: note: Calling free_func
free_func(f);
^
parseAPI/h/CFGFactory.h:116:18: note: free_func is a virtual function
virtual void free_func(Function * f);
* CFGFactor::free_edge
* ~freebsd_process
Found using cppcheck:
proccontrol/src/freebsd.C:1046:17: warning: Member variable 'freebsd_thread::is_exited' is not initialized in the constructor. [uninitMemberVar]
freebsd_thread::freebsd_thread(int_process *p, Dyninst::THR_ID t, Dyninst::LWP l)
^
proccontrol/src/freebsd.h:122:17: style: Virtual function 'getEventQueue' is called from destructor '~freebsd_process()' at line 795. Dynamic binding is not used. [virtualCallInConstructor]
virtual int getEventQueue();
^
proccontrol/src/freebsd.C:795:22: note: Calling getEventQueue
int eventQueue = getEventQueue();
^
proccontrol/src/freebsd.h:122:17: note: getEventQueue is a virtual function
virtual int getEventQueue();
* Mismatched printf format arguments These were found using cppcheck's invalidPrintfArgType_*. * Fix uninitialized member variables
* Protect against self-assignment in copy constructors
These were found by cppcheck:
common/src/Node.C:198:29: warning: 'operator=' should check for assignment to self to avoid problems with dynamic memory. [operatorEqToSelf]
NodeIterator &NodeIterator::operator=(const NodeIterator &rhs) {
^
common/src/Edge.C:108:29: warning: 'operator=' should check for assignment to self to avoid problems with dynamic memory. [operatorEqToSelf]
EdgeIterator &EdgeIterator::operator=(const EdgeIterator &rhs) {
These were found using cppcheck's nullPointerRedundantCheck.
These were detected by cppcheck's uninitMemberVar*.
These were found using cppcheck's uninitvar and eraseDereference. For the usages of iterator-like classes in process.C, they aren't strictly uninitialized variable usages since the classes in question are standard layout types. It would be better to replace the usages there with list construction to avoid the static check altogether.
* Shifting signed 32-bit value by 31 bits is undefined behavior This was found using cppcheck's shiftTooManyBitsSigned. * Signed to unsigned conversion in calculation These were found using cppcheck's signConversion. * Fix signed overflow This was found using cppcheck's integerOverflowCond. * Shifting negative value This was found using cppcheck's shiftNegativeLHS. * BPatch_addressSpace::deleteSnippet: fix enumeral conversion in conditional
* Local var leak in Symtab::addSymbol This was found using cppcheck's memleak. * memCache::doOperation Not technically a leak, but cppcheck can't see through the 'push_back'. * PCProcess::hasPassedMain Found using cppcheck's danglingTemporaryLifetime. * parse_func::calcParentFunc Found using cppcheck's danglingTemporaryLifetime. * int_iRPC::setBinarySize Found using cppcheck's publicAllocationError.
The following instruction decoded with the wrong length if the modrm operand specified a memory access as all the operand was incorrectly specified to be a register only operand: - vcvtpd2udq - vcvtss2usi - vcvttpd2qq - vcvtudq2pd - vcvtudq2ps - vpblendd - vpermpd
- update dyninst header files to directly include the standard header file defining symbols from the standard C++ library that are used by the dyninst header file; in some instances, the code relied on symbols being defined via an unrelated include file indirectly including the necessary header file leading to fragile code - minor other cleanups: remove unnecessary header files, remove definitions of names that are defined in a standard header file
There are several reasons for this. 1. It doesn't copy every member of the class 2. IBSTree doesn't have a copy constructor 3. mod_lookup_ and func_lookup_ are not copied, but are recreated in other member functions (e.g., mod_lookup()). This completely breaks the semantics of a copy ctor. 4. Resets _ref_cnt to 1
The definition was commented out in 3709ead in 2009.
It is never used.
…t#1450) Because there is a user-defined destructor, the compiler will not generate the special member functions (e.g., copy assignment operator). However, we explicitly delete them here to signal that this class is not copyable or movable. The destructor is also moved to the top of the class to be next to the other special member functions.
* Use default member initializers instead of initializer lists There are members missing from the lists. This also simplifies the constructors considerably. * Remove unhelpful message in default ctor * Delegate to default ctor in Symtab(MappedFile*) This fixes the bug of inconsistent base initialization and ensures the constructors are consistent. * Use delegating ctor for Symtab::Symtab(unsigned char*...) This fixes the following bugs 1. Invoke init_debug_symtabAPI before calling create_printf. 2. Because this constructor can return early due to errors, it's imperative that the object is completely constructed by some means _before_ such a return happens. Otherwise, references to this object and its destruction would have class invariants inconsistent with objects created by the other constructors. * Use delegating ctor for Symtab::Symtab(std::string...) Because this constructor can return early due to errors, it's imperative that the object is completely constructed by some means _before_ such a return happens. Otherwise, references to this object and its destruction would have class invariants inconsistent with objects created by the other constructors.
…ninst#1452) This fix does not address the thread safety issues in Symtab::parseFunctionRanges. That function is still thread unsafe and will require separate modifications.
fix nullptr dereference if Module* is nullptr; print "null" instead
LEA is a NOP if and only if the two operands refer to the same register after simplifying binary expressions with identity operands.
Co-authored-by: Ilya Shlyapin <ilya.shlyapin@huawei.com>
…inst#1684) * Refactor IA_IAPI::isSyscall,isInterrupt into arch-specific files This also requires those members be defined for all classes derived from IA_IAPI.
* Treat x86 software interrupts as system calls * Correct detection of Linux vsyscall for 32-bit code The existing code did not work because the Operand formatter uses AT&T syntax and doesn't convert hex to decimal, so the '== "16"' check always failed. The convoluted logic in the visitor is needed because Dyninst does not generate AST for segment registers. Yet, it _does_ record if they are read/written. The AST for the 'gs' register was made a file scope static because its initialization would cause some glibc's to throw a __gnu_cxx::recursive_init_error when using multiple threads. * aarch64 - add svc Supervisor Call * ppc - add system call * Add int1, into to IA_x86::isInterrupt
Currently, all symbols are hidden by default and then explicitly made visible. This is the behavior we want for publishing Dyninst libraries, but unit testing the internal functionality requires access to these symbols. This option allows that.
This was broken by e7db16f in 2010.
- update COPYRIGHT files's notice to include 2024 - update copyright header in source files to reference COPYRIGHT
…yninst#1691) 165f19c and 091929d changed the parsing semantics of system calls such that software interrupts (e.g., `int` on x86) are always interpreted as system calls. The `hasCFT()` check on line 1824 now covers all of the system call cases because it considers those instructions to have a control flow target (CFT). Its non-use was verified against /usr/libx32/libc.so.6, /usr/lib/i386-linux-gnu/libc.so.6, /usr/lib/x86_64-linux-gnu/libc.so.6, and /usr/lib32/libc.so.6 on Ubuntu 22.04.
* Move isSyscall to instructionAPI * Use InstructionAPI::isSystemCall in LivenessAnalyzer::calcRWSets * Use InstructionAPI::isSystemCall in PCSensitiveTransformer::process * Use InstructionAPI::isSystemCall in Parse::parse_frame_one_iteration * Use InstructionAPI::isSystemCall in IA_IAPI::getNewEdges * Remove IA_IAPI::isSyscall It is replaced by InstructionAPI::isSystemCall.
* Move isInterrupt to InstructionAPI * Use InstructionAPI::isInterrupt in IA_IAPI::isInterruptOrSyscall * Use InstructionAPI::isInterrupt in LivenessAnalyzer::calcRWSets * Remove IA_IAPI::isInterrupt It is replaced by InstructionAPI::isInterrupt
- update dyninst version - update CHANGELOG.md - produce manuals with updated release dates and versions - fix .tex files to eliminate latex errors: - invalid char in symtabAPI/doc/API/Types/Type.tex - invalid char in parseAPI/doc/API/Function.tex - invalid char in patchAPI/doc/section/4_api_public.tex - invlide line break in parseAPI/doc/API/CodeObject.tex - restore parseAPI example code needed by docs removed in 6c2e31c Co-authored-by: James Kupsch <kupsch@vmbp15.local>
nullptr check in handleCondDirExits for dyninst relocation instrumenter
This can be used to ensure that all threads in a process are stopped before the breakpoint is executed.
…inst#2131) * Use a synchronized breakpoint for 'main' If a thread is spawned before reaching main (e.g., in a .ctor entry), proccontrol does not observe events from it and so assumes it is safe to remove the breakpoint at 'main'. It is unclear at this time why no events are observed. Particularly becuase there is a check that _should_ catch this case: BPatch::processCreate V BPatch_process::BPatch_process V PCProcess::createProcess V Process::createProcess V int_process::createProcess V PCProcess::bootstrapProcess() V assert( pcProc_->allThreadsStopped() ); co-authored by Kian Cossettini @ AMD * Ignore events for destroyed threads launched before main If a thread is launched and destroyed before main is reached, there could be events in the queue that belong to that dead thread. We should ignore them. This seems at odds with proccontrol's failure to detect that there are threads alive before main is reached, but there is a user-reported crash that is fixed by these changes. Co-authored-by: Kian Cossettini <Kian.Cossettini@amd.com>
## Motivation <!-- Explain the purpose of this PR and the goals it aims to achieve. --> - Fix to allow building Dyninst usint both bundled dependencies, from the parent project, and the installed system dependencies. - Required for ROCm/rocm-systems#4206. ## Technical Details <!-- Explain the changes along with any relevant GitHub links. --> - Revert commit e0d5940, which caused us unable to build with the system's dependencies. - If `Dyninst::Boost`, `Dyninst::TBB`, `Dyninst::ElfUtils`, or `Dyninst::LibIberty` exist, skip find_package() - This allows the parent project, like `rocprofiler-systems`, to provide bundled dependencies via Dyninst::* targets - Essential for bundled dependency support (ROCPROFSYS_BUILD_*=YES) ## JIRA ID - AIPROFSYST-354 ## Test Plan <!-- Explain any relevant testing done to verify this PR. --> Workflows tested in ROCm/rocm-systems#4206 ## Test Result <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Claude (Claude-Sonnet-4.5) <noreply@anthropic.com>
Fixes my issue: dyninst#2203. Cherry-picks dyninst@aab9cbd. Part of ROCM-21219 Co-authored-by: kupsch <kupsch@cs.wisc.edu>
Never c/c++ header versions provided by the gcc 15
requires that stdint.h and cstdint needs to be included excplicitly
to avoid build errors related to types like uint32_t.
Fixes following type of errors with gcc 15:
/therock/rocm-systems/projects/rocprofiler-systems/external/dyninst/common/src/sha1.C:112:5:
error: ‘uint32_t’ does not name a type
10.6 112 | uint32_t state[5];
and
‘INT32_MAX’ was not declared in this scope
Signed-off-by: Mika Laitio <mika.laitio@amd.com>
## Motivation <!-- Explain the purpose of this PR and the goals it aims to achieve. --> Cherry pick 635cacd from https://github.com/dyninst/dyninst This adds the `isSharedLib()` to `BPatch_object`, allowing a quicker way of filtering out shared libraries. This way we can avoid going through the SymTab API or `BPatch_module` ## Technical Details <!-- Explain the changes along with any relevant GitHub links. --> See dyninst#2215 ## Test Plan <!-- Explain any relevant testing done to verify this PR. --> ## Test Result <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
## Motivation Remove Boost dependency from Dyninst and use std and dyncompat as replacement. The removal of Boost dependency needs to be done for the reasons as described in the now closed PR on dyninst master branch: dyninst#2172 ## Technical Details Boost was dropped as an external dependency; Dyninst now builds against an internal dyncompat layer and C++17 standard library equivalents. ## Test Plan Tested with rocm-systems PR: (ROCm/rocm-systems#5406). ## Test Result Rocprofiler-systems (https://github.com/ROCm/rocm-systems/tree/develop/projects/rocprofiler-systems) was tested on supported platforms. A small test with Lulesh/Kokkos path was performed and no performance regressions were observed. Details are updated in PR: ROCm/rocm-systems#5406 ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Kian Cossettini <Kian.Cossettini@amd.com>
## Motivation <!-- Explain the purpose of this PR and the goals it aims to achieve. --> Add CODEOWNERS file to `dyninst_13` branch. CODEOWNER files are required for all public repos and ours was found missing from an org-wide audit ## Technical Details <!-- Explain the changes along with any relevant GitHub links. --> Add default reviewer `ROCm/rocprof-sys` ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
…#19) ## Motivation **NOTE**: This is a modified version of my PR on dyninst (dyninst#2235) Original issue: dyninst#1290 <!-- Explain the purpose of this PR and the goals it aims to achieve. --> Add support for ELF's RELR dynamic relocation. ## Technical Details Generally, I tried to follow how the existing `RT_REL` and `RT_RELA` were implemented. Of course, `RELR` is a different relocation format that is a compressed relative-relocation table encoded with address and bitmap entries, so it needs separate decode logic. Overall, `elf.h` defines for `RELR` were added and their cases were added in the relevant sections. Also: In `Object-elf` files: - `decodeRelrEntries(...)`: Decodes the entries following https://dram.page/p/relative-relocs-explained/#:~:text=The%20idea%20originates,handled%20needs%20relocating. - `get_relocationRelr_entries(...)`: Finds `.relr.dyn`, determine whether `uintptr_t` is 4 or 8 bytes from the ELF class, validate `DT_RELRENT` + `DT_RELRSZ` against the section data, then call `decodeRelrEntries(...)`. In `emitElf` files: - `.relr.dyn` was not added to `updateLinkInfoSecs` as `sh_info` and `sh_link` are unused. - Within `emitElf<ElfTypes>::driver`: - For every `RELR` entry, adjust the value stored in the memory slot needing load-base adjustment. - In the section that checks for pointer-table sections `(.init_array`, `.fini_array`, etc...), prevent re-applying the offset to an address that `RELR` offset adjustment logic already handled. - Handle emitting `RT_RELR` in `createLoadableSections` - `createRelrRelocationSection(...)`: Rebuilds the new `.relr.dyn` section from the decoded `RELR` relocation locations shifted by `library_adjust`. - Within `createSymbolVersions(...)`, preserve original `.gnu.version_r` version-need entries that were not recreated through symbol version references. This keeps symbolless data such as `GLIBC_ABI_DT_RELR` when it was present in the input binary. **NOTE**: `createRelrRelocationSection(...)` does not recompress bitmap entries. It emits each decoded location as a direct `RELR` entry (which is still valid `RELR` encoding... see https://gabi.xinuos.com/elf/06-reloc.html#:~:text=unit%20is%20relocatable.-,Note,that%20a%20simple%20list%20of%20(even)%20addresses%20is%20a%20valid%20encoding.,-%E2%86%90%205.). **More details can be found in dyninst#2235, including discussions with contributors**. <!-- Explain the changes along with any relevant GitHub links. --> ## Test Plan <!-- Explain any relevant testing done to verify this PR. --> See ROCm/rocm-systems#5736 ## Test Result <!-- Briefly summarize test outcomes. --> When I forced binaries on `rocm-systems` to be compiled with `RELR` relocation, CI workflows passed (excluding Ubuntu Jammy, but that's because GLIBC version is too old). When adding RHEL 10 workflows, the original test failure that started this entire fiasco (`coreutils` binary rewrite) is no longer observed. **Note**: The implemented code only executes when `RELR` relocation is observed (EXCLUDING the preservation of the original `.gnu_version_r` version-need entries that were not created through symbol version references). ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
## Motivation
`CMAKE_C_FLAGS` and `CMAKE_CXX_FLAGS` are space-separated strings. When
assigned directly to a CMake list variable the entire string becomes a
single
list element, so a value like `-march=native -O2` reaches the compiler
as one
opaque argument instead of two separate flags. This silently breaks
compilation
when any flag requires a following argument (e.g. `-isystem /path`) or
when
the compiler rejects the concatenated form.
This surfaces in practice with Clang, which is stricter about multi-flag
strings than GCC.
## Technical Details
Use `separate_arguments(... UNIX_COMMAND ...)` to tokenize
`CMAKE_C_FLAGS` and
`CMAKE_CXX_FLAGS` into proper CMake list elements before appending them
to the
`DYNINST_C_FLAGS_<BUILD>` / `DYNINST_CXX_FLAGS_<BUILD>` variables:
```cmake
separate_arguments(_dyninst_c_flags UNIX_COMMAND "${CMAKE_C_FLAGS}")
separate_arguments(_dyninst_cxx_flags UNIX_COMMAND "${CMAKE_CXX_FLAGS}")
```
`UNIX_COMMAND` mode matches POSIX shell word-splitting, which is the
correct
semantic for flag strings passed on a compiler command line.
Changed file: `cmake/DyninstOptimization.cmake`
- Add missing "includes" to various headers.
## Test Plan
- [x] Build Dyninst with a multi-flag `CMAKE_C_FLAGS` /
`CMAKE_CXX_FLAGS`
(e.g. `-march=native -O2`) and confirm all flags are passed
individually.
- [x] Build with Clang as the C/C++ compiler and verify no flag-parsing
errors.
- [x] Confirm existing CMake build types (Release, Debug,
RelWithDebInfo) still
produce correct flag sets.
## Test Result
Builds successfully with Clang when `CMAKE_C_FLAGS` / `CMAKE_CXX_FLAGS`
contain space-separated flags.
## Submission Checklist
- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
## Motivation <!-- Explain the purpose of this PR and the goals it aims to achieve. --> Cherry pick 5d2c62a from https://github.com/dyninst/dyninst This modifies the boundary check in the `find_code_from_data` function from inclusive to a half-open boundary. Without this, it can choose the incorrect `PT_LOAD` segment when `.text` starts exactly at the end address of a previous load segment. ## Technical Details <!-- Explain the changes along with any relevant GitHub links. --> Change the following comparisons from `>=` to `>` - https://github.com/dyninst/dyninst/blob/master/symtabAPI/src/Object-elf.C#L2318 - https://github.com/dyninst/dyninst/blob/master/symtabAPI/src/Object-elf.C#L2320 - https://github.com/dyninst/dyninst/blob/master/symtabAPI/src/Object-elf.C#L2329 ## Test Plan <!-- Explain any relevant testing done to verify this PR. --> PR was tested using the original reproducer that warranted this PR: AIPROFSYST-179. PR was also testing using head of dyninst on a custom walker. ## Test Result <!-- Briefly summarize test outcomes. --> `rocprof-sys-instrument` in binary-rewrite mode no longer crashes. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Update default branch to dyninst_13
Technical Details
Test Plan
Test Result
Submission Checklist