From 721d5d04d1df599bfd96d3abb72953460261e9a2 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Mon, 10 Aug 2026 14:47:26 -0400 Subject: [PATCH 1/9] Phase 1 Test --- .github/workflows/build.yaml | 218 ++++++++++++++++----- .github/workflows/cmake-formatting.yaml | 55 ++++-- .github/workflows/compiler-multibuild.yaml | 130 ------------ .github/workflows/consumers.yaml | 170 ---------------- .github/workflows/dependency-version.yaml | 54 ----- .github/workflows/dev-containers.yaml | 42 ---- .github/workflows/libabigail.yaml | 95 --------- .github/workflows/pr-tests.yaml | 122 +++--------- .github/workflows/spack-build.yaml | 19 -- .github/workflows/system-libs.yaml | 45 ----- .github/workflows/unit-tests.yaml | 53 ----- .gitignore | 2 + scripts/build-tpls.sh | 185 +++++++++++++++++ scripts/tpl-versions.env | 17 ++ 14 files changed, 432 insertions(+), 775 deletions(-) delete mode 100644 .github/workflows/compiler-multibuild.yaml delete mode 100644 .github/workflows/consumers.yaml delete mode 100644 .github/workflows/dependency-version.yaml delete mode 100644 .github/workflows/dev-containers.yaml delete mode 100644 .github/workflows/libabigail.yaml delete mode 100644 .github/workflows/spack-build.yaml delete mode 100644 .github/workflows/system-libs.yaml delete mode 100644 .github/workflows/unit-tests.yaml create mode 100644 scripts/build-tpls.sh create mode 100644 scripts/tpl-versions.env diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 5c5403471b..7a66d5b483 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -1,75 +1,201 @@ name: Build Dyninst +# Reusable builder. Builds Dyninst's third-party libraries from source (see +# scripts/build-tpls.sh for why), then builds, installs and smoke-tests Dyninst. +# +# The TPLs are built in a separate job because actions/cache skips its save step +# when a job fails: building them inline would mean a Dyninst build failure +# discards the TPL prefix and the next run pays for it again. + on: workflow_call: inputs: name: + description: Display name for this configuration required: true type: string - os: + image: + description: Container image to build in required: true type: string - extra-libs: + cc: required: false type: string - extra-cmake-flags: + default: gcc + cxx: required: false type: string - c-compiler: - required: true - type: string - cxx-compiler: - required: true + default: g++ + # RELWITHDEBINFO is RELEASE plus -g3 (see cmake/DyninstOptimization.cmake), + # so it covers the code generation of a RELEASE build, which is what + # rocprofiler-systems builds with, while leaving failures debuggable. + build-types: + description: JSON array of CMAKE_BUILD_TYPE values + required: false type: string - is-clang: + default: '["RELWITHDEBINFO"]' + container-options: required: false - type: boolean - default: false + type: string + default: '--shm-size=512m' + +permissions: + contents: read + +env: + TPL_PREFIX: ${{ github.workspace }}/.tpls + INSTALL_PREFIX: ${{ github.workspace }}/install jobs: + tpls: + name: ${{ inputs.name }} / third-party libs + runs-on: ubuntu-latest + container: + image: ${{ inputs.image }} + options: ${{ inputs.container-options }} + steps: + - uses: actions/checkout@v6 + + # The prefix bakes absolute paths (elfutils is built with an absolute + # RPATH), so the cache is only valid for an identical workspace path and + # image. Both are part of the key. + - name: Cache third-party libs + id: tpl-cache + uses: actions/cache@v6 + with: + path: ${{ env.TPL_PREFIX }} + key: tpls-${{ inputs.image }}-${{ hashFiles('scripts/tpl-versions.env', 'scripts/build-tpls.sh') }} + + - name: Build third-party libs + if: steps.tpl-cache.outputs.cache-hit != 'true' + run: bash scripts/build-tpls.sh --prefix "${TPL_PREFIX}" --jobs "$(nproc)" + build: - permissions: - packages: read + name: ${{ inputs.name }} (${{ matrix.build-type }}) + needs: tpls + runs-on: ubuntu-latest + container: + image: ${{ inputs.image }} + options: ${{ inputs.container-options }} + strategy: fail-fast: false matrix: - build-type: ['DEBUG', 'RELWITHDEBINFO', 'RELEASE'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ inputs.os }}:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: ${{ inputs.name }} (${{ matrix.build-type }}) + build-type: ${{ fromJSON(inputs.build-types) }} + + env: + CCACHE_DIR: ${{ github.workspace }}/.ccache + steps: - # Clang doesn't allow for multiple libomp installations - - name: Clean libomp install - if: ${{ inputs.is-clang }} - run: apt remove --purge -y "libomp*" - - - name: Install C compiler (${{ inputs.c-compiler }}) - run: | - apt update -qq - apt install -qq --no-install-recommends -y ${{ inputs.c-compiler }} + - uses: actions/checkout@v6 + + - name: Restore third-party libs + uses: actions/cache/restore@v6 + with: + path: ${{ env.TPL_PREFIX }} + key: tpls-${{ inputs.image }}-${{ hashFiles('scripts/tpl-versions.env', 'scripts/build-tpls.sh') }} + fail-on-cache-miss: true - # There is no apt package for clang++ - - name: Install ${{ inputs.cxx-compiler }} - if: ${{ !inputs.is-clang }} - run: apt install -qq --no-install-recommends -y ${{ inputs.cxx-compiler }} + - name: Restore ccache + uses: actions/cache@v6 + with: + path: ${{ env.CCACHE_DIR }} + key: ccache-${{ inputs.image }}-${{ matrix.build-type }}-${{ github.sha }} + restore-keys: | + ccache-${{ inputs.image }}-${{ matrix.build-type }}- - - name: Install extra libs (${{ inputs.extra-libs }}) - if: ${{ inputs.extra-libs != '' }} - run: apt install -qq --no-install-recommends -y ${{ inputs.extra-libs }} + - name: Configure ccache + run: | + mkdir -p "${CCACHE_DIR}" + ccache --max-size=1G + ccache --set-config=sloppiness=time_macros,include_file_mtime,include_file_ctime,pch_defines + ccache -z - - name: Configure Dyninst (${{ matrix.build-type }}) - shell: bash + - name: Configure run: | - cmake /dyninst/src \ - -DCMAKE_BUILD_TYPE="${{ matrix.build-type }}" \ - -DCMAKE_C_COMPILER="${{ inputs.c-compiler }}" \ - -DCMAKE_CXX_COMPILER="${{ inputs.cxx-compiler }}" \ - -DDYNINST_WARNINGS_AS_ERRORS=ON ${{ inputs.extra-cmake-flags }} + git config --global --add safe.directory "${GITHUB_WORKSPACE}" + cmake --version + ${{ inputs.cxx }} --version + cmake -S . -B build \ + -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} \ + -DCMAKE_INSTALL_PREFIX="${INSTALL_PREFIX}" \ + -DCMAKE_C_COMPILER=${{ inputs.cc }} \ + -DCMAKE_CXX_COMPILER=${{ inputs.cxx }} \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ + -DDYNINST_WARNINGS_AS_ERRORS=ON \ + -DTBB_ROOT_DIR="${TPL_PREFIX}/tbb" \ + -DElfUtils_ROOT_DIR="${TPL_PREFIX}/elfutils" \ + -DLibIberty_ROOT_DIR="${TPL_PREFIX}/binutils" - - name: Build Dyninst + # A silent fallback to the distro's TBB/elfutils/libiberty would make this + # job green while testing the wrong dependency versions. LibIberty in + # particular does not reliably exclude system paths, so assert explicitly. + - name: Verify third-party libs resolved to the built prefix run: | - cmake --build . --parallel 2 + fail=0 + check() { + val=$(grep -E "^$1:" build/CMakeCache.txt | head -1 | cut -d= -f2-) + if [ -z "${val}" ]; then + echo "MISSING $1 is not set in CMakeCache.txt" + fail=1 + elif [ "${val#"${TPL_PREFIX}"}" != "${val}" ]; then + echo "ok $1 = ${val}" + else + echo "WRONG $1 = ${val}" + echo " expected a path under ${TPL_PREFIX}" + fail=1 + fi + } + check Elfutils_LIBRARIES + check LibIberty_LIBRARIES + check TBB_DIR + exit "${fail}" + + - name: Build + run: cmake --build build --parallel "$(nproc)" + + - name: Install + run: cmake --install build + + # Proves the install can parse, instrument and rewrite a real binary, and + # that the rewritten binary still runs. A link-only check would not. + - name: Smoke test (parseThat binary rewrite) + run: | + set -x + export DYNINSTAPI_RT_LIB="${INSTALL_PREFIX}/lib/libdyninstAPI_RT.so" + export LD_LIBRARY_PATH="${INSTALL_PREFIX}/lib:${TPL_PREFIX}/elfutils/lib:${TPL_PREFIX}/tbb/lib:${LD_LIBRARY_PATH:-}" + test -f "${DYNINSTAPI_RT_LIB}" + + mkdir -p /tmp/smoke && cd /tmp/smoke + cat > hello.c <<'EOF' + #include + int work(int x) { return x * 2; } + int main(void) { printf("dyninst-smoke-ok %d\n", work(21)); return 0; } + EOF + ${{ inputs.cc }} -g -O0 -o hello hello.c + ./hello | tee baseline.txt + grep -q 'dyninst-smoke-ok 42' baseline.txt + + # -i 1 instruments function entries, so this exercises instrumentation + # rather than only parse-and-write-back. + "${INSTALL_PREFIX}/bin/parseThat" -i 1 --binary-edit=./hello.rewritten ./hello + + test -x ./hello.rewritten + ./hello.rewritten | tee rewritten.txt + grep -q 'dyninst-smoke-ok 42' rewritten.txt + + - name: ccache stats + if: always() + run: ccache -s + + - name: Upload CMake logs + if: failure() + uses: actions/upload-artifact@v7 + with: + name: cmake-logs-${{ inputs.name }}-${{ matrix.build-type }} + path: | + build/CMakeCache.txt + build/CMakeFiles/CMakeConfigureLog.yaml + build/CMakeFiles/*.log + if-no-files-found: ignore diff --git a/.github/workflows/cmake-formatting.yaml b/.github/workflows/cmake-formatting.yaml index 7507ed1277..3d5c3b1909 100644 --- a/.github/workflows/cmake-formatting.yaml +++ b/.github/workflows/cmake-formatting.yaml @@ -1,33 +1,46 @@ - name: CMake Formatting on: pull_request: - branches: [ master ] + branches: [dyninst_13] paths: - - '**.cmake' - - '**CMakeLists.txt' + - '**.cmake' + - '**CMakeLists.txt' + - '.cmake-format.yaml' + - '.github/workflows/cmake-formatting.yaml' workflow_dispatch: +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: cmake-formatting: + name: cmake-format runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Install dependencies - run: | - sudo apt-get update - sudo apt-get install -y python3-pip - python3 -m pip install cmake-format - - name: cmake-format - run: | - set +e - cmake-format -i $(find . -type f | egrep 'CMakeLists.txt|\.cmake$') - if [ $(git diff | wc -l) -gt 0 ]; then - echo -e "\nError! CMake files not formatted." - echo -e "\nRun the following to fix:" - for f in $(git diff --name-only); do echo -e " cmake-format -i $f"; done - echo - exit 1 - fi + - uses: actions/checkout@v6 + + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + + # Pinned: an unpinned cmake-format can start formatting differently and turn + # every PR red without any change to this repository. + - name: Install cmake-format + run: python3 -m pip install cmake-format==0.6.13 + + - name: Check formatting + run: | + cmake-format -i $(find . -type f | grep -E 'CMakeLists.txt|\.cmake$') + if [ -n "$(git diff --name-only)" ]; then + echo "Error! CMake files are not formatted." + echo "Run the following to fix:" + for f in $(git diff --name-only); do echo " cmake-format -i $f"; done + git diff + exit 1 + fi diff --git a/.github/workflows/compiler-multibuild.yaml b/.github/workflows/compiler-multibuild.yaml deleted file mode 100644 index 9f2fa173d4..0000000000 --- a/.github/workflows/compiler-multibuild.yaml +++ /dev/null @@ -1,130 +0,0 @@ -# 1. Build with multiple versions of gcc and clang using the ubuntu-provided compiler -# 2. Using the latest version of each compiler, build against all supported C++ standards - -name: Compiler multibuild - -on: - schedule: - - cron: '0 3 * * 1' # 3AM on Monday - workflow_dispatch: - -jobs: - gcc-ubuntu-20_04: - strategy: - fail-fast: false - matrix: - version: [7, 8, 9, 10] - uses: ./.github/workflows/build.yaml - with: - name: gcc-${{ matrix.version }} - os: "ubuntu-20.04" - c-compiler: "gcc-${{ matrix.version }}" - cxx-compiler: "g++-${{ matrix.version }}" - - gcc-ubuntu-22_04: - strategy: - fail-fast: false - matrix: - version: [11, 12] - uses: ./.github/workflows/build.yaml - with: - name: gcc-${{ matrix.version }} - os: "ubuntu-22.04" - c-compiler: "gcc-${{ matrix.version }}" - cxx-compiler: "g++-${{ matrix.version }}" - - gcc-ubuntu-23_10: - strategy: - fail-fast: false - matrix: - version: [13] - uses: ./.github/workflows/build.yaml - with: - name: gcc-${{ matrix.version }} - os: "ubuntu-23.10" - c-compiler: "gcc-${{ matrix.version }}" - cxx-compiler: "g++-${{ matrix.version }}" - - - clang-ubuntu-20_04: - strategy: - fail-fast: false - matrix: - version: [7, 8, 9, 10, 11, 12] - uses: ./.github/workflows/build.yaml - with: - name: clang-${{ matrix.version }} - os: "ubuntu-20.04" - c-compiler: "clang-${{ matrix.version }}" - cxx-compiler: "clang++-${{ matrix.version }}" - is-clang: true - extra-libs: "libomp-${{ matrix.version }}-dev" - - clang-ubuntu-22_04: - strategy: - fail-fast: false - matrix: - version: [13, 14, 15] - uses: ./.github/workflows/build.yaml - with: - name: clang-${{ matrix.version }} - os: "ubuntu-22.04" - c-compiler: "clang-${{ matrix.version }}" - cxx-compiler: "clang++-${{ matrix.version }}" - is-clang: true - extra-libs: "libomp-${{ matrix.version }}-dev" - - clang-ubuntu-23_10: - strategy: - fail-fast: false - matrix: - version: [16, 17] - uses: ./.github/workflows/build.yaml - with: - name: clang-${{ matrix.version }} - os: "ubuntu-23.10" - c-compiler: "clang-${{ matrix.version }}" - cxx-compiler: "clang++-${{ matrix.version }}" - is-clang: true - extra-libs: "libomp-${{ matrix.version }}-dev" - - gcc-cxx-standards-11-17: - strategy: - fail-fast: false - matrix: - std: [11, 14, 17] - uses: ./.github/workflows/build.yaml - with: - name: gcc-cxx-${{ matrix.std }} - os: "ubuntu-22.04" - c-compiler: "gcc-12" - cxx-compiler: "g++-12" - extra-cmake-flags: "-DDYNINST_CXX_LANGUAGE_STANDARD=${{ matrix.std }}" - - gcc-cxx-standards-20-23: - strategy: - fail-fast: false - matrix: - std: [20, 23] - uses: ./.github/workflows/build.yaml - with: - name: gcc-cxx-${{ matrix.std }} - os: "ubuntu-23.10" - c-compiler: "gcc-13" - cxx-compiler: "g++-13" - extra-cmake-flags: "-DDYNINST_CXX_LANGUAGE_STANDARD=${{ matrix.std }}" - - clang-cxx-standards: - strategy: - fail-fast: false - matrix: - std: [11, 14, 17] # clang has a bug with 20+ and operator== reflexiveness - uses: ./.github/workflows/build.yaml - with: - name: cxx-${{ matrix.std }} - os: "ubuntu-22.04" - c-compiler: "clang-15" - cxx-compiler: "clang++-15" - is-clang: true - extra-cmake-flags: "-DDYNINST_CXX_LANGUAGE_STANDARD=${{ matrix.std }}" - extra-libs: "libomp-15-dev" diff --git a/.github/workflows/consumers.yaml b/.github/workflows/consumers.yaml deleted file mode 100644 index 3a21c3d023..0000000000 --- a/.github/workflows/consumers.yaml +++ /dev/null @@ -1,170 +0,0 @@ -# Build the latest versions of applications that consume Dyninst - -name: Build Consumers - -on: - schedule: - - cron: '0 3 * * 1' # Monday at 3AM - workflow_dispatch: - -jobs: - spack-build: - strategy: - fail-fast: false - matrix: - consumer: [ - "hpctoolkit@develop", - "must+stackwalker~backward~tsan" -# extrae+dyninst - not yet tested, may not support dyninst >10.0.0 -# omnitrace - Needs updated cmake -# timemory - Needs updated cmake - ] - runs-on: ubuntu-latest - steps: - - name: ${{ matrix.consumer }} - run: | - sudo apt update -qq - sudo apt install -y -qq --no-install-recommends build-essential gcc g++ gfortran m4 cmake autoconf python3 git unzip openmpi-bin libopenmpi-dev - git clone --depth=1 --branch=develop https://github.com/spack/spack - spack/bin/spack compiler find - spack/bin/spack external find --not-buildable cmake python git m4 openmpi gmake - spack/bin/spack install ${{ matrix.consumer }} ^dyninst@master - - systemtap: - permissions: - packages: read - strategy: - fail-fast: true - matrix: - os: ['ubuntu-23.10'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: systemtap ${{ matrix.os }} - steps: - - name: Install dependencies - run: | - apt update - apt install -y git python3 libjson-c-dev m4 autoconf - - name: Fetch systemtap - run: | - git clone --depth=1 https://sourceware.org/git/systemtap.git - - name: Make symlinks - run: | - ln -s /dyninst/install/include /usr/include/dyninst - ln -s /dyninst/install/lib /usr/lib64/dyninst - - name: Build systemtap - run: | - cd systemtap - autoreconf - mkdir build - cd build - ../configure --with-dyninst --without-python3-probes - make -j2 - - llnl-stat: - permissions: - packages: read - strategy: - fail-fast: true - matrix: - os: ['ubuntu-20.04'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: llnl-stat ${{ matrix.os }} - steps: - - name: Install dependencies - run: | - apt update - apt install -y nano git bison flex python3 build-essential dh-autoreconf wget libgcrypt20-dev libboost-program-options-dev libboost-regex-dev libboost-wave-dev libpython3-dev python3-distutils - - name: Install GraphLib - run: | - git clone --depth=1 https://github.com/LLNL/graphlib - cd graphlib - mkdir build - cd build - cmake .. - cmake --build . --parallel 2 - cmake --install . # /usr - - name: Install GraphViz - run: | - cd / - git clone --depth=1 https://gitlab.com/graphviz/graphviz.git - cd graphviz - ./autogen.sh - mkdir build - cd build - ../configure --without-qt --without-gts --without-doc --without-expat --without-ghostscript --without-gtkplus --without-libgd --without-pangocairo --without-popler --without-quartz --without-x - make -j2 - make install - - name: Install launchmon - run: | - cd / - git clone --depth=1 https://github.com/llnl/launchmon.git - cd launchmon - ./bootstrap - mkdir build - cd build - ../configure - make -j2 - make install - - name: Install MRNet - run: | - cd / - git clone --depth=1 https://github.com/dyninst/mrnet.git - cd mrnet - mkdir build - cd build - CC=gcc CXX=g++ ../configure --enable-shared - make -j2 - make install - - name: Install STAT - run: | - cd / - git clone --depth=1 https://github.com/llnl/stat.git - cd stat - ./bootstrap - mkdir build - cd build - ../configure --disable-gui --disable-examples --with-stackwalker=/dyninst/install --with-mrnet=/usr/local - make -j2 - tau: - permissions: - packages: read - strategy: - fail-fast: true - matrix: - os: ['ubuntu-20.04'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: TAU ${{ matrix.os }} - steps: - - name: Install dependencies - run: | - # TAU assumes Dyninst needs libdwarf instead of libdw. This has no real - # effect on Dyninst as we RPATH our deps. It just makes the manually-constructed - # link line in the TAU build work. - apt update - apt install -y git libdwarf1 - - name: Fix libdwarf - run: | - ln -s /usr/lib/x86_64-linux-gnu/libdwarf.so.1.0.0 /usr/lib/x86_64-linux-gnu/libdwarf.so - - name: Fetch TAU - run: | - git clone --depth=1 https://github.com/UO-OACISS/tau2 - - name: Build TAU - run: | - cd tau2 - ./configure -dyninst=/dyninst/install - make -j2 diff --git a/.github/workflows/dependency-version.yaml b/.github/workflows/dependency-version.yaml deleted file mode 100644 index bb2e3d4bb3..0000000000 --- a/.github/workflows/dependency-version.yaml +++ /dev/null @@ -1,54 +0,0 @@ -# Ensure the minimum dependency versions found in the various CMake -# files match the expected values. This ensures we synchronize versions -# across containers and workflows. - -name: Check dependency versions - -on: - pull_request: - branches: [ master ] - paths: - - '**.cmake' - - '**CMakeLists.txt' - - 'docker/dependencies.versions' - workflow_dispatch: - -jobs: - check-version: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Version check - run: | - res=0 - current=$(awk 'match($0,/set\(_min_version (.+)\)/,a){print a[1]}' cmake/tpls/DyninstBoost.cmake) - expected=$(awk 'match($0,/boost:(.+)/,a){print a[1]}' docker/dependencies.versions) - if test "$current" != "$expected"; then - echo "Boost mismatch: Found $current, expected $expected" >/dev/stderr - res=1 - fi - - current=$(awk 'match($0,/set\(_min_version (.+)\)/,a){print a[1]}' cmake/tpls/DyninstTBB.cmake) - expected=$(awk 'match($0,/tbb:(.+)/,a){print a[1]}' docker/dependencies.versions) - if test "$current" != "$expected"; then - echo "TBB mismatch: Found $current, expected $expected" >/dev/stderr - res=1 - fi - - current=$(awk 'match($0,/set\(_min_version (.+)\)/,a){print a[1]}' cmake/tpls/DyninstElfUtils.cmake) - expected=$(awk 'match($0,/elfutils:(.+)/,a){print a[1]}' docker/dependencies.versions) - if test "$current" != "$expected"; then - echo "Elfutils mismatch: Found $current, expected $expected" >/dev/stderr - res=1 - fi - - current=$(awk 'match($0,/cmake_minimum_required\(VERSION (.+) FATAL_ERROR\)/,a){print a[1]}' CMakeLists.txt) - expected=$(awk 'match($0,/cmake:(.+)/,a){print a[1]}' docker/dependencies.versions) - if test "$current" != "$expected"; then - echo "CMake mismatch: Found $current, expected $expected" >/dev/stderr - res=1 - fi - - exit $res diff --git a/.github/workflows/dev-containers.yaml b/.github/workflows/dev-containers.yaml deleted file mode 100644 index d85fe5458e..0000000000 --- a/.github/workflows/dev-containers.yaml +++ /dev/null @@ -1,42 +0,0 @@ -name: Build and Deploy Development Containers - -on: - push: - branches: - - master - workflow_dispatch: - -jobs: - build: - permissions: - packages: write - strategy: - fail-fast: false - matrix: - os: ['ubuntu-20.04', 'ubuntu-22.04', 'ubuntu-23.04', 'ubuntu-23.10', 'ubuntu-24.04', 'fedora-37', 'fedora-38', 'fedora-39'] - runs-on: ubuntu-latest - name: Update dev containers - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: GHCR Login - uses: docker/login-action@v1 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Pull base image - run: docker pull ghcr.io/dyninst/amd64/${{ matrix.os }}-base:latest - - - name: Build Dyninst Dev Container - run: | - cd docker/ - docker build --build-arg base=ghcr.io/dyninst/amd64/${{ matrix.os }}-base:latest \ - --build-arg build_jobs=2 \ - -f Dockerfile \ - -t ghcr.io/dyninst/amd64/${{ matrix.os }}:latest ../ - - - name: Deploy - run: docker push ghcr.io/dyninst/amd64/${{ matrix.os }}:latest diff --git a/.github/workflows/libabigail.yaml b/.github/workflows/libabigail.yaml deleted file mode 100644 index 36ab118611..0000000000 --- a/.github/workflows/libabigail.yaml +++ /dev/null @@ -1,95 +0,0 @@ -name: Libabigail ABI Checks -on: - pull_request: [] - -jobs: - get-release: - container: ghcr.io/dyninst/dyninst-ubuntu-20.04:v12.1.0 - runs-on: ubuntu-latest - steps: - - name: Upload Libs - uses: actions/upload-artifact@v2-preview - with: - name: release-libs - path: /opt/dyninst-env/install/dyninst/lib - - get-latest: - container: ghcr.io/dyninst/dyninst-ubuntu-20.04:latest - runs-on: ubuntu-latest - steps: - - name: Upload Libs - uses: actions/upload-artifact@v2-preview - with: - name: latest-libs - path: /opt/dyninst-env/install/dyninst/lib - - get-pr: - container: ghcr.io/dyninst/dyninst-ubuntu-20.04:latest - runs-on: ubuntu-latest - steps: - - name: Build Pull Request - uses: actions/checkout@v3 - - name: Build - run: | - rm -rf /code - cp -R $PWD /code - ls /code - cd /opt/dyninst-env - /bin/bash build.sh - - - name: Upload results - uses: actions/upload-artifact@v2-preview - with: - name: pr-libs - path: /opt/dyninst-env/install/dyninst/lib - - abi: - runs-on: ubuntu-latest - needs: [get-latest, get-release, get-pr] - strategy: - fail-fast: false - matrix: - - # Testing every paired library for release vs pr and main vs. pr - libs: ["libcommon.so", - "libdynC_API.so", - "libdynDwarf.so", - "libdynElf.so", - "libdyninstAPI_RT.so", - "libdyninstAPI.so", - "libinstructionAPI.so", - "libparseAPI.so", - "libpatchAPI.so", - "libpcontrol.so", - "libstackwalk.so", - "libsymLite.so", - "libsymtabAPI.so"] - - # Artifact pairs (named) for comparison) - artifacts: [["pr-libs", "latest-libs"], - ["pr-libs", "release-libs"]] - - steps: - - name: Download Previous Version - uses: actions/download-artifact@v2 - with: - name: ${{ matrix.artifacts[1] }} - path: previous/ - - - name: Download Pull Request Version - uses: actions/download-artifact@v2 - with: - name: ${{ matrix.artifacts[0] }} - path: current/ - - - name: Show Files - run: | - ls current/ - ls previous/ - - - name: Run Libabigail - uses: buildsi/libabigail-action@main - env: - lib: ${{ matrix.libs }} - with: - abidiff: previous/${{ env.lib }} current/${{ env.lib }} diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml index ea8751f309..d69fc79da6 100644 --- a/.github/workflows/pr-tests.yaml +++ b/.github/workflows/pr-tests.yaml @@ -1,108 +1,30 @@ -# On each pull request, we build Dyninst, the test suite, the examples from -# dyninst/examples, and the external test from dyninst/external-tests -# -# The builds are carried out for each supported OS using the base containers -# at https://github.com/orgs/dyninst/packages +name: PR Tests -name: Pull Request Tests +# Goal 1: Dyninst builds and can instrument and run a program on its own. +# +# Currently a single distro while the from-source third-party library approach +# proves itself; the remaining images in the rocprofiler-systems CI matrix are +# added once this is green. on: pull_request: - branches: - - master + branches: [dyninst_13] workflow_dispatch: -jobs: - gcc-build: - permissions: - packages: read - strategy: - fail-fast: false - matrix: - os: ['ubuntu-20.04', 'ubuntu-22.04', 'ubuntu-23.04', 'ubuntu-23.10', 'ubuntu-24.04', 'fedora-37', 'fedora-38', 'fedora-39'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}-base:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: gcc on ${{ matrix.os }} - steps: - - name: Checkout Dyninst - uses: actions/checkout@v3 - with: - path: dyninst/src - - - name: Build Dyninst - run: | - ln -s $PWD/dyninst /dyninst - export DYNINST_C_FLAGS="-Werror" DYNINST_CXX_FLAGS="-Werror" - export DYNINST_C_COMPILER="gcc" DYNINST_CXX_COMPILER="g++" - bash /dyninst/src/docker/build.sh /dyninst/src 2 - - - name: Checkout Test Suite - uses: actions/checkout@v3 - with: - repository: dyninst/testsuite - path: testsuite - - - name: Build testsuite - run: | - cd testsuite; mkdir build; cd build - cmake .. -DDyninst_DIR=/dyninst/install/lib/cmake/Dyninst - cmake --build . --parallel 2 +permissions: + contents: read - - name: Checkout Examples - uses: actions/checkout@v3 - with: - repository: dyninst/examples - path: examples +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true - - name: Build examples - run: | - cd examples; mkdir build; cd build - cmake .. -DDyninst_DIR=/dyninst/install/lib/cmake/Dyninst - cmake --build . --parallel 2 - - - name: Checkout External Tests - uses: actions/checkout@v3 - with: - repository: dyninst/external-tests - path: external-tests - - - name: Build external tests - run: | - cd external-tests; mkdir build; cd build - cmake .. -DDyninst_DIR=/dyninst/install/lib/cmake/Dyninst - cmake --build . --parallel 2 - - name: Run tests - run: | - cd external-tests/build - ctest . - - clang-build: - permissions: - packages: read - strategy: - fail-fast: false - matrix: - os: ['ubuntu-20.04', 'ubuntu-22.04', 'ubuntu-23.04', 'ubuntu-23.10', 'ubuntu-24.04'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}-base:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: clang on ${{ matrix.os }} - steps: - - name: Checkout Dyninst - uses: actions/checkout@v3 - with: - path: dyninst/src - - - name: Build Dyninst - run: | - ln -s $PWD/dyninst /dyninst - export DYNINST_C_FLAGS="-Werror" DYNINST_CXX_FLAGS="-Werror" - export DYNINST_C_COMPILER="clang" DYNINST_CXX_COMPILER="clang++" - bash /dyninst/src/docker/build.sh /dyninst/src 2 +jobs: + ubuntu-24-04: + name: ubuntu-24.04 + uses: ./.github/workflows/build.yaml + with: + name: ubuntu-24.04 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-ubuntu-24.04 + cc: gcc + cxx: g++ + build-types: '["RELWITHDEBINFO"]' diff --git a/.github/workflows/spack-build.yaml b/.github/workflows/spack-build.yaml deleted file mode 100644 index 9a4b143926..0000000000 --- a/.github/workflows/spack-build.yaml +++ /dev/null @@ -1,19 +0,0 @@ -name: Spack Build - -on: - schedule: - - cron: '0 3 * * 0' # Every Sunday at 3AM - workflow_dispatch: - -jobs: - build: - runs-on: ubuntu-latest - steps: - - name: spack - run: | - sudo apt update -qq - sudo apt install -y -qq --no-install-recommends build-essential gcc g++ m4 cmake autoconf python3 git - git clone --depth=1 --branch=develop https://github.com/spack/spack - spack/bin/spack compiler find - spack/bin/spack external find --not-buildable cmake - spack/bin/spack install dyninst@master diff --git a/.github/workflows/system-libs.yaml b/.github/workflows/system-libs.yaml deleted file mode 100644 index 55cc360bef..0000000000 --- a/.github/workflows/system-libs.yaml +++ /dev/null @@ -1,45 +0,0 @@ -# Do a simple parse of all system libraries - -name: Parse sys libs - -on: - schedule: - - cron: '0 1 * * 1' # 1AM on Monday - workflow_dispatch: - -jobs: - parse: - permissions: - packages: read - strategy: - fail-fast: false - matrix: - os: ['ubuntu-20.04', 'ubuntu-22.04', 'ubuntu-23.04', 'ubuntu-23.10', 'ubuntu-24.04', 'fedora-37', 'fedora-38', 'fedora-39'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: ${{ matrix.os }} - steps: - - name: Build parser - run: | - git clone --depth=1 https://github.com/dyninst/external-tests - cd external-tests - mkdir build - cd build - cmake .. -DDyninst_DIR=/dyninst/install/lib/cmake/Dyninst - cmake --build . - cp parseAPI/simpleParser / - - - name: Run parser - run: | - cd / - export LD_LIBRARY_PATH=/dyninst/install/lib:$LD_LIBRARY_PATH - for dir in /usr/lib /usr/lib64; do \ - for file in $(find $dir -type f -name "*.so.*"); do \ - echo $file; \ - ./simpleParser $file; \ - done \ - done diff --git a/.github/workflows/unit-tests.yaml b/.github/workflows/unit-tests.yaml deleted file mode 100644 index dbc7a227f6..0000000000 --- a/.github/workflows/unit-tests.yaml +++ /dev/null @@ -1,53 +0,0 @@ -name: Unit tests - -on: - pull_request: - branches: - - master - workflow_dispatch: - -jobs: - unit-tests: - permissions: - packages: read - strategy: - fail-fast: false - matrix: - os: ['ubuntu-20.04', 'ubuntu-22.04', 'ubuntu-23.04', 'ubuntu-23.10', 'ubuntu-24.04', 'fedora-37', 'fedora-38', 'fedora-39'] - runs-on: ubuntu-latest - container: - image: ghcr.io/dyninst/amd64/${{ matrix.os }}-base:latest - credentials: - username: ${{ github.actor }} - password: ${{ secrets.github_token }} - name: unit tests - steps: - - name: Checkout Dyninst - uses: actions/checkout@v3 - with: - path: dyninst/src - - - name: Build Dyninst - run: | - ln -s $PWD/dyninst /dyninst - export DYNINST_C_FLAGS="-Werror" DYNINST_CXX_FLAGS="-Werror" - export DYNINST_C_COMPILER="gcc" DYNINST_CXX_COMPILER="g++" - export EXTRA_CMAKE_FLAGS="-DDYNINST_EXPORT_ALL=1" - bash /dyninst/src/docker/build.sh /dyninst/src 2 - - - name: Checkout Unit Tests - uses: actions/checkout@v3 - with: - repository: dyninst/unit-tests - path: unit-tests - - - name: Build unit tests - run: | - cd unit-tests; mkdir build; cd build - cmake .. -DDyninst_DIR=/dyninst/install/lib/cmake/Dyninst -DDYNINST_SOURCE_TREE=/dyninst/src - cmake --build . --parallel 2 - - - name: Run unit tests - run: | - cd unit-tests/build - ctest . diff --git a/.gitignore b/.gitignore index 465fd49466..7600603c8a 100644 --- a/.gitignore +++ b/.gitignore @@ -121,6 +121,8 @@ doxyfiles/* .vscode/* build*/ cmake-build-*/ +# Staging prefix produced by scripts/build-tpls.sh +.tpls/ .project .cproject .settings diff --git a/scripts/build-tpls.sh b/scripts/build-tpls.sh new file mode 100644 index 0000000000..4e89f18da5 --- /dev/null +++ b/scripts/build-tpls.sh @@ -0,0 +1,185 @@ +#!/usr/bin/env bash +# +# Build Dyninst's third-party libraries (oneTBB, elfutils, libiberty) from source. +# +# Dyninst's CMake requires all three to be present and has no download fallback: +# cmake/tpls/Dyninst{TBB,ElfUtils,LibIberty}.cmake each call find_package(REQUIRED). +# Distro packages lag well behind the versions Dyninst is validated against as part +# of rocprofiler-systems, so CI builds them here instead. See scripts/tpl-versions.env. +# +# Usage: +# build-tpls.sh --prefix DIR [--jobs N] [--skip-prereqs] +# +# Produces one root per library, mirroring the rocprofiler-systems layout so that +# binutils' generic headers (dwarf2.h, demangle.h, ...) cannot shadow elfutils': +# +# $PREFIX/tbb pass to cmake as -DTBB_ROOT_DIR +# $PREFIX/elfutils pass to cmake as -DElfUtils_ROOT_DIR +# $PREFIX/binutils pass to cmake as -DLibIberty_ROOT_DIR +# +# The prefix is self-describing: a stamp file records the versions it was built +# from, so a restored CI cache built from different versions is rebuilt rather +# than silently reused. + +set -euo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=tpl-versions.env +source "${script_dir}/tpl-versions.env" + +prefix="" +jobs="$(nproc 2>/dev/null || echo 2)" +skip_prereqs=0 + +usage() { + sed -n '3,25p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --prefix) prefix="$2"; shift 2 ;; + --jobs|-j) jobs="$2"; shift 2 ;; + --skip-prereqs) skip_prereqs=1; shift ;; + -h|--help) usage; exit 0 ;; + *) echo "error: unknown argument '$1'" >&2; usage >&2; exit 2 ;; + esac +done + +if [[ -z "${prefix}" ]]; then + echo "error: --prefix is required" >&2 + exit 2 +fi + +mkdir -p "${prefix}" +prefix="$(cd "${prefix}" && pwd)" + +tbb_root="${prefix}/tbb" +elfutils_root="${prefix}/elfutils" +binutils_root="${prefix}/binutils" + +stamp="${prefix}/.tpl-versions" +want_stamp="onetbb=${ONETBB_VERSION} elfutils=${ELFUTILS_VERSION} binutils=${BINUTILS_VERSION}" + +if [[ -f "${stamp}" ]] && [[ "$(cat "${stamp}")" == "${want_stamp}" ]]; then + echo "Third-party libraries already present at ${prefix} (${want_stamp}); nothing to do." + exit 0 +fi + +echo "Building third-party libraries into ${prefix}" +echo " ${want_stamp}" +echo " jobs: ${jobs}" + +install_prereqs() { + if ! command -v apt-get >/dev/null 2>&1; then + echo "error: only apt-based images are supported today." >&2 + echo " Re-run with --skip-prereqs after installing the equivalents of:" >&2 + echo " bzip2 curl git m4 make pkg-config zlib libzstd libbz2 liblzma (all -dev)" >&2 + exit 1 + fi + apt-get update -qq + apt-get install -y -qq --no-install-recommends \ + bzip2 ca-certificates curl git m4 make pkg-config \ + zlib1g-dev libzstd-dev libbz2-dev liblzma-dev +} + +build_tbb() { + echo "::group::Build oneTBB ${ONETBB_VERSION}" + local src="${workdir}/oneTBB" + git clone --depth 1 --branch "v${ONETBB_VERSION}" \ + https://github.com/uxlfoundation/oneTBB.git "${src}" + + # TBB_TEST=OFF skips the (slow) test tree; TBB_STRICT=OFF keeps oneTBB's own + # -Werror from failing the build on whichever compiler the image ships. + cmake -S "${src}" -B "${workdir}/tbb-build" \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_INSTALL_PREFIX="${tbb_root}" \ + -DCMAKE_INSTALL_LIBDIR=lib \ + -DTBB_TEST=OFF \ + -DTBB_STRICT=OFF \ + -DTBB_DISABLE_HWLOC_AUTOMATIC_SEARCH=ON + cmake --build "${workdir}/tbb-build" --parallel "${jobs}" \ + --target tbb tbbmalloc tbbmalloc_proxy + cmake --install "${workdir}/tbb-build" + echo "::endgroup::" +} + +build_elfutils() { + echo "::group::Build elfutils ${ELFUTILS_VERSION}" + local tarball="elfutils-${ELFUTILS_VERSION}.tar.bz2" + local src="${workdir}/elfutils-${ELFUTILS_VERSION}" + + curl -fsSL --retry 3 --retry-delay 5 -o "${workdir}/${tarball}" \ + "https://sourceware.org/elfutils/ftp/${ELFUTILS_VERSION}/${tarball}" \ + || curl -fsSL --retry 3 --retry-delay 5 -o "${workdir}/${tarball}" \ + "https://mirrors.kernel.org/sourceware/elfutils/${ELFUTILS_VERSION}/${tarball}" + tar -xf "${workdir}/${tarball}" -C "${workdir}" + + # Flags mirror rocprofiler-systems' DyninstElfUtils.cmake. -fPIC because + # Dyninst links these into shared libraries. debuginfod is disabled to match + # Dyninst's ENABLE_DEBUGINFOD default of OFF; enabling one without the other + # produces a find_package component mismatch. + ( + cd "${src}" + CFLAGS="-fPIC -O3 -Wno-error=maybe-uninitialized" \ + CXXFLAGS="-fPIC -O3 -Wno-error=maybe-uninitialized" \ + LDFLAGS="-Wl,-rpath,${elfutils_root}/lib -pthread" \ + ./configure \ + --prefix="${elfutils_root}" \ + --libdir="${elfutils_root}/lib" \ + --enable-install-elfh \ + --enable-thread-safety \ + --disable-libdebuginfod \ + --disable-debuginfod \ + --disable-nls + make install "-j${jobs}" + ) + echo "::endgroup::" +} + +build_libiberty() { + echo "::group::Build libiberty from binutils ${BINUTILS_VERSION}" + local tarball="binutils-${BINUTILS_VERSION}.tar.gz" + local src="${workdir}/binutils-${BINUTILS_VERSION}" + + curl -fsSL --retry 3 --retry-delay 5 -o "${workdir}/${tarball}" \ + "https://ftpmirror.gnu.org/gnu/binutils/${tarball}" \ + || curl -fsSL --retry 3 --retry-delay 5 -o "${workdir}/${tarball}" \ + "https://mirrors.kernel.org/sourceware/binutils/releases/${tarball}" + tar -xf "${workdir}/${tarball}" -C "${workdir}" + + mkdir -p "${binutils_root}/lib" "${binutils_root}/include" + + # Build only the libiberty subtree: a full binutils build additionally needs + # bison/flex/texinfo. MAKEINFO=true no-ops the doc rules that would otherwise + # require Texinfo. + ( + cd "${src}" + CFLAGS="-fPIC -O3 -Wno-error" \ + CXXFLAGS="-fPIC -O3 -Wno-error" \ + MAKEINFO=true \ + ./configure --prefix="${binutils_root}" + make MAKEINFO=true "-j${jobs}" all-libiberty + ) + + install -C "${src}/libiberty/libiberty.a" "${binutils_root}/lib/" + install -C -m 644 "${src}"/include/*.h "${binutils_root}/include/" + echo "::endgroup::" +} + +workdir="$(mktemp -d)" +trap 'rm -rf "${workdir}"' EXIT + +if [[ "${skip_prereqs}" -eq 0 ]]; then + install_prereqs +fi + +build_tbb +build_elfutils +build_libiberty + +echo "${want_stamp}" > "${stamp}" + +echo "Third-party libraries installed:" +echo " TBB_ROOT_DIR = ${tbb_root}" +echo " ElfUtils_ROOT_DIR = ${elfutils_root}" +echo " LibIberty_ROOT_DIR = ${binutils_root}" diff --git a/scripts/tpl-versions.env b/scripts/tpl-versions.env new file mode 100644 index 0000000000..1a1d062e93 --- /dev/null +++ b/scripts/tpl-versions.env @@ -0,0 +1,17 @@ +# Versions of Dyninst's third-party libraries built from source by build-tpls.sh. +# +# These deliberately mirror what rocprofiler-systems builds, so that CI validates +# Dyninst against the dependency versions it is actually shipped with rather than +# whatever the distro happens to package. When bumping any of these, check the +# corresponding pin downstream: +# +# oneTBB .gitmodules -> projects/rocprofiler-systems/external/onetbb +# elfutils projects/rocprofiler-systems/cmake/DyninstElfUtils.cmake +# binutils projects/rocprofiler-systems/cmake/DyninstLibIberty.cmake +# +# This file is read by build-tpls.sh and is the cache key for the built prefix, +# so any edit here invalidates the CI cache and forces a rebuild. + +ONETBB_VERSION=2022.3.0 +ELFUTILS_VERSION=0.195 +BINUTILS_VERSION=2.46.0 From 731bb0a62a195d75c6704ffcaff44a8fb651b920 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Tue, 11 Aug 2026 08:40:23 -0400 Subject: [PATCH 2/9] Phase 2 - Test CI --- .github/workflows/build.yaml | 54 ++-- .github/workflows/cmake-formatting.yaml | 46 --- .github/workflows/lint.yaml | 61 ++++ .github/workflows/rocprofiler-systems.yaml | 359 +++++++++++++++++++++ .gitignore | 2 + scripts/build-tpls.sh | 2 +- scripts/smoke/mutatee.c | 28 ++ scripts/smoke/run-smoke.sh | 242 ++++++++++++++ scripts/smoke/runtime_instrument.cpp | 105 ++++++ 9 files changed, 827 insertions(+), 72 deletions(-) delete mode 100644 .github/workflows/cmake-formatting.yaml create mode 100644 .github/workflows/lint.yaml create mode 100644 .github/workflows/rocprofiler-systems.yaml mode change 100644 => 100755 scripts/build-tpls.sh create mode 100644 scripts/smoke/mutatee.c create mode 100755 scripts/smoke/run-smoke.sh create mode 100644 scripts/smoke/runtime_instrument.cpp diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 7a66d5b483..11b0ce914a 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -158,32 +158,26 @@ jobs: - name: Install run: cmake --install build - # Proves the install can parse, instrument and rewrite a real binary, and - # that the rewritten binary still runs. A link-only check would not. - - name: Smoke test (parseThat binary rewrite) + # Rewrites a binary with parseThat and instruments a live process with + # BPatch::processCreate. A link-only check would prove neither, and the + # runtime half is the only one that exercises ProcControlAPI and the + # injection of libdyninstAPI_RT. + # + # --retries applies only to a timeout, never to a failed assertion, so a + # genuine regression still fails on the first attempt. It is here because + # ProcControl startup was seen to hang rather than fail; the step timeout + # is the outer backstop if even the retries wedge. + - name: Smoke test + timeout-minutes: 10 run: | - set -x - export DYNINSTAPI_RT_LIB="${INSTALL_PREFIX}/lib/libdyninstAPI_RT.so" - export LD_LIBRARY_PATH="${INSTALL_PREFIX}/lib:${TPL_PREFIX}/elfutils/lib:${TPL_PREFIX}/tbb/lib:${LD_LIBRARY_PATH:-}" - test -f "${DYNINSTAPI_RT_LIB}" - - mkdir -p /tmp/smoke && cd /tmp/smoke - cat > hello.c <<'EOF' - #include - int work(int x) { return x * 2; } - int main(void) { printf("dyninst-smoke-ok %d\n", work(21)); return 0; } - EOF - ${{ inputs.cc }} -g -O0 -o hello hello.c - ./hello | tee baseline.txt - grep -q 'dyninst-smoke-ok 42' baseline.txt - - # -i 1 instruments function entries, so this exercises instrumentation - # rather than only parse-and-write-back. - "${INSTALL_PREFIX}/bin/parseThat" -i 1 --binary-edit=./hello.rewritten ./hello - - test -x ./hello.rewritten - ./hello.rewritten | tee rewritten.txt - grep -q 'dyninst-smoke-ok 42' rewritten.txt + bash scripts/smoke/run-smoke.sh \ + --prefix "${INSTALL_PREFIX}" \ + --tpl-prefix "${TPL_PREFIX}" \ + --cc ${{ inputs.cc }} \ + --cxx ${{ inputs.cxx }} \ + --workdir "${{ github.workspace }}/.smoke" \ + --timeout 120 \ + --retries 1 - name: ccache stats if: always() @@ -199,3 +193,13 @@ jobs: build/CMakeFiles/CMakeConfigureLog.yaml build/CMakeFiles/*.log if-no-files-found: ignore + + # The captured stdout of each stage is what distinguishes "instrumentation + # did not run" from "the mutatee itself failed". + - name: Upload smoke test output + if: failure() + uses: actions/upload-artifact@v7 + with: + name: smoke-${{ inputs.name }}-${{ matrix.build-type }} + path: .smoke/*.txt + if-no-files-found: ignore diff --git a/.github/workflows/cmake-formatting.yaml b/.github/workflows/cmake-formatting.yaml deleted file mode 100644 index 3d5c3b1909..0000000000 --- a/.github/workflows/cmake-formatting.yaml +++ /dev/null @@ -1,46 +0,0 @@ -name: CMake Formatting - -on: - pull_request: - branches: [dyninst_13] - paths: - - '**.cmake' - - '**CMakeLists.txt' - - '.cmake-format.yaml' - - '.github/workflows/cmake-formatting.yaml' - workflow_dispatch: - -permissions: - contents: read - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - cmake-formatting: - name: cmake-format - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v6 - - - uses: actions/setup-python@v6 - with: - python-version: '3.12' - - # Pinned: an unpinned cmake-format can start formatting differently and turn - # every PR red without any change to this repository. - - name: Install cmake-format - run: python3 -m pip install cmake-format==0.6.13 - - - name: Check formatting - run: | - cmake-format -i $(find . -type f | grep -E 'CMakeLists.txt|\.cmake$') - if [ -n "$(git diff --name-only)" ]; then - echo "Error! CMake files are not formatted." - echo "Run the following to fix:" - for f in $(git diff --name-only); do echo " cmake-format -i $f"; done - git diff - exit 1 - fi diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml new file mode 100644 index 0000000000..c6b4a5b5eb --- /dev/null +++ b/.github/workflows/lint.yaml @@ -0,0 +1,61 @@ +name: Lint + +# Runs the repository's pre-commit hooks: cmake-format, shellcheck and +# actionlint. Contributors who ran `pre-commit install` can never fail this +# check, because it executes the same pinned hooks they do. +# +# Deliberately has no paths filter. The hooks already select their own files, +# and a filter here would only create ways for a change to skip the check. + +on: + pull_request: + branches: [dyninst_13] + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + lint: + name: pre-commit + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + + # Tool versions are pinned in .pre-commit-config.yaml, not here. An + # unpinned formatter or linter can change its output and turn every PR red + # without any change to this repository. + - name: Install pre-commit + run: python3 -m pip install pre-commit==4.6.2 + + - name: Cache pre-commit environments + uses: actions/cache@v6 + with: + path: ~/.cache/pre-commit + key: pre-commit-3.12-${{ hashFiles('.pre-commit-config.yaml') }} + + - name: Run pre-commit + id: check + run: pre-commit run --all-files --show-diff-on-failure --color=always + + - name: How to fix + if: failure() && steps.check.outcome == 'failure' + run: | + echo "pre-commit reported problems. Anything printed as a diff above is" + echo "an automatic fix; shellcheck and actionlint findings need editing." + echo + echo "Reproduce locally with:" + echo " pip install pre-commit" + echo " pre-commit run --all-files" + echo + echo "Or run the hooks automatically on every commit:" + echo " pre-commit install" diff --git a/.github/workflows/rocprofiler-systems.yaml b/.github/workflows/rocprofiler-systems.yaml new file mode 100644 index 0000000000..bad9edff16 --- /dev/null +++ b/.github/workflows/rocprofiler-systems.yaml @@ -0,0 +1,359 @@ +name: rocprofiler-systems + +# Goal 2: rocprofiler-systems builds and its Dyninst-facing ctests pass when +# external/dyninst is this PR's head rather than the pinned submodule. +# +# This reproduces the upstream job in +# rocm-systems/.github/workflows/rocprofiler-systems-build-group.yml rather than +# calling it, because that workflow is not reusable from another repository. +# +# Advisory: the jobs are continue-on-error. rocm-systems develop moves +# independently of this repository, so a red result can mean upstream breakage +# rather than a regression in the pull request. Revisit once it has been stable +# for a while. + +on: + pull_request: + branches: [dyninst_13] + workflow_dispatch: + inputs: + scope: + description: Which upstream CI entries to run + type: choice + options: [single, full] + default: single + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + ROCM_SYSTEMS_REPO: ROCm/rocm-systems + ROCM_SYSTEMS_BRANCH: develop + +jobs: + # Reads the upstream matrix definition instead of copying it, so the set of + # images cannot drift from what rocprofiler-systems actually tests. + matrix: + name: resolve upstream matrix + runs-on: ubuntu-latest + outputs: + entries: ${{ steps.select.outputs.entries }} + sha: ${{ steps.resolve.outputs.sha }} + steps: + # Resolved once and reused by every build job, so a push to develop + # mid-run cannot leave the matrix entries testing different commits. + - name: Resolve rocm-systems branch to a commit + id: resolve + run: | + sha=$(git ls-remote "https://github.com/${ROCM_SYSTEMS_REPO}.git" \ + "refs/heads/${ROCM_SYSTEMS_BRANCH}" | cut -f1) + if [ -z "${sha}" ]; then + echo "Could not resolve ${ROCM_SYSTEMS_REPO}@${ROCM_SYSTEMS_BRANCH}" + exit 1 + fi + echo "sha=${sha}" >> "${GITHUB_OUTPUT}" + + - name: Select matrix entries + id: select + env: + SCOPE: ${{ github.event.inputs.scope || 'single' }} + SHA: ${{ steps.resolve.outputs.sha }} + run: | + curl -fsSL -o matrix.json \ + "https://raw.githubusercontent.com/${ROCM_SYSTEMS_REPO}/${SHA}/projects/rocprofiler-systems/.github/ci-build-matrix.json" + + # kind drives the only real difference between the two upstream jobs: + # whether the Dyninst third-party libraries are built or taken from + # the distro. + if [ "${SCOPE}" = "full" ]; then + jq -c '[ + (.primary[] + | select(.image | test("ci-rocm-7\\.2-")) + | . + {kind: "primary"}), + (.system_deps[] + | select(.image | test("ci-rocm-7\\.2-ubuntu")) + | select(.compiler == "g++") + | . + {kind: "system_deps"}) + ]' matrix.json > entries.json + else + jq -c '[ + .primary[] + | select(.image | endswith("ci-rocm-7.2-ubuntu-24.04")) + | . + {kind: "primary"} + ]' matrix.json > entries.json + fi + + # An upstream rename would otherwise silently produce an empty matrix, + # which GitHub reports as a successful job. + count=$(jq 'length' entries.json) + if [ "${count}" -eq 0 ]; then + echo "No matrix entries matched. Upstream ci-build-matrix.json may have changed." + jq -r '.primary[].image, .system_deps[].image' matrix.json | sort -u + exit 1 + fi + echo "Selected ${count} entry/entries:" + jq -r '.[].name' entries.json + + echo "entries=$(jq -c '{include: .}' entries.json)" >> "${GITHUB_OUTPUT}" + + downstream: + name: ${{ matrix.name }} + needs: matrix + runs-on: ubuntu-latest + continue-on-error: true + container: + image: ${{ matrix.image }} + options: ${{ matrix.container_opts || '--shm-size=512m' }} + + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.matrix.outputs.entries) }} + + defaults: + run: + shell: bash + working-directory: rocm-systems/projects/rocprofiler-systems + + env: + CDASH_NAME: dyninst-${{ github.event.number || github.ref_name }}-${{ matrix.cdash_suffix }} + OMPI_ALLOW_RUN_AS_ROOT: 1 + OMPI_ALLOW_RUN_AS_ROOT_CONFIRM: 1 + ROCPROFSYS_CI: 'ON' + ROCPROFSYS_MAX_THREADS: '64' + ROCPROFSYS_KEEP_TEST_OUTPUT: '0' + ROCPROFSYS_TMPDIR: "%env{PWD}%/testing-tmp" + CCACHE_DIR: ${{ github.workspace }}/.ccache + GIT_DISCOVERY_ACROSS_FILESYSTEM: 1 + CMAKE_C_COMPILER_LAUNCHER: ccache + CMAKE_CXX_COMPILER_LAUNCHER: ccache + + steps: + - name: Check out rocm-systems + uses: actions/checkout@v6 + with: + repository: ${{ env.ROCM_SYSTEMS_REPO }} + ref: ${{ needs.matrix.outputs.sha }} + path: rocm-systems + sparse-checkout: | + projects/rocprofiler-systems/ + .gitmodules + + # The whole point of this workflow. rocprofiler_systems_checkout_git_submodule() + # returns early when the submodule's CMakeLists.txt already exists, so + # populating the path here means CMake never fetches the pinned Dyninst. + - name: Check out this pull request into external/dyninst + uses: actions/checkout@v6 + with: + path: rocm-systems/projects/rocprofiler-systems/external/dyninst + + - name: Record the versions under test + run: | + git config --global --add safe.directory '*' + test -f external/dyninst/CMakeLists.txt + dyninst_sha=$(git -C external/dyninst rev-parse HEAD) + echo "DYNINST_SHA=${dyninst_sha}" >> "${GITHUB_ENV}" + { + echo "| component | commit |" + echo "| --- | --- |" + echo "| rocm-systems ${ROCM_SYSTEMS_BRANCH} | \`${{ needs.matrix.outputs.sha }}\` |" + echo "| dyninst (this PR) | \`${dyninst_sha}\` |" + } >> "${GITHUB_STEP_SUMMARY}" + + - name: Configure ROCm environment (Ubuntu / Debian) + if: ${{ !matrix.is_rhel }} + run: | + echo "/opt/rocm/bin" >> "${GITHUB_PATH}" + echo "ROCM_PATH=/opt/rocm" >> "${GITHUB_ENV}" + echo "LD_LIBRARY_PATH=/opt/rocm/lib:${LD_LIBRARY_PATH}" >> "${GITHUB_ENV}" + + - name: Configure ROCm environment (RHEL) + if: ${{ matrix.is_rhel }} + run: | + echo "CC=${{ matrix.cc }}" >> "${GITHUB_ENV}" + echo "CXX=${{ matrix.compiler }}" >> "${GITHUB_ENV}" + echo "/opt/rocm/bin" >> "${GITHUB_PATH}" + echo "/opt/rocm/llvm/bin" >> "${GITHUB_PATH}" + echo "ROCM_PATH=/opt/rocm" >> "${GITHUB_ENV}" + echo "LD_LIBRARY_PATH=/opt/rocm/lib:${LD_LIBRARY_PATH}" >> "${GITHUB_ENV}" + + # Upstream uses nick-fields/retry here. A loop keeps this repository free + # of third-party actions; the retries matter because the package mirrors + # are the flakiest step in the job. + - name: Install distro packages + env: + APT_COMPILER: ${{ matrix.apt_install_compiler && matrix.compiler || '' }} + APT_SYSTEM_DEPS: ${{ matrix.kind == 'system_deps' && matrix.system_deps_apt || '' }} + run: | + packages="${APT_COMPILER} ${APT_SYSTEM_DEPS}" + if [ -z "${packages// /}" ] && [ "${{ matrix.os_major }}" != "10" ]; then + echo "No extra packages needed for this entry" + exit 0 + fi + retry() { + for attempt in 1 2 3 4 5; do + if "$@"; then return 0; fi + echo "attempt ${attempt} failed, retrying in 30s" + sleep 30 + done + return 1 + } + if [ "${{ matrix.is_rhel }}" = "true" ]; then + # mpich lives in the crb repository on RHEL 10. + if [ "${{ matrix.os_major }}" = "10" ]; then + retry dnf install -y --enablerepo=crb mpich mpich-devel + echo "/usr/lib64/mpich/bin" >> "${GITHUB_PATH}" + fi + elif [ -n "${packages// /}" ]; then + retry apt-get update + # shellcheck disable=SC2086 + retry apt-get install -y ${packages} + apt-get autoclean + fi + + - name: Restore ccache + uses: actions/cache@v6 + with: + path: ${{ github.workspace }}/.ccache + key: rps-ccache-${{ matrix.ccache_key_distro }}-${{ matrix.kind }}-${{ github.sha }} + restore-keys: | + rps-ccache-${{ matrix.ccache_key_distro }}-${{ matrix.kind }}- + + - name: Configure ccache + run: | + mkdir -p "${CCACHE_DIR}" + ccache --max-size=2G + ccache --set-config=sloppiness=time_macros,include_file_mtime,include_file_ctime,pch_defines + ccache -z + + - name: Install Python test dependencies + timeout-minutes: 10 + run: | + for env_dir in /opt/conda/envs/${{ matrix.python_envs_glob }}/; do + if [ -d "${env_dir}" ] && [ -x "${env_dir}bin/python3" ]; then + echo "Installing requirements into ${env_dir}" + "${env_dir}bin/python3" -m pip install -r requirements.txt + fi + done + + # Mirrors the two upstream CMAKE_COMMON_FLAGS blocks. The system-deps + # entries carry no DISABLE_EXAMPLES of their own, so it belongs here. + - name: Select dependency flags + run: | + if [ "${{ matrix.kind }}" = "system_deps" ]; then + flags="-DROCPROFSYS_BUILD_TBB=OFF -DROCPROFSYS_BUILD_ELFUTILS=OFF -DROCPROFSYS_BUILD_LIBIBERTY=OFF" + flags="${flags} -DROCPROFSYS_BUILD_HIDDEN_VISIBILITY=ON -DROCPROFSYS_STRIP_LIBRARIES=OFF" + flags="${flags} -DROCPROFSYS_DISABLE_EXAMPLES=transpose;rccl;openmp-target;openmp-vv;videodecode;jpegdecode;network" + else + flags="-DROCPROFSYS_BUILD_TBB=ON -DROCPROFSYS_BUILD_ELFUTILS=ON -DROCPROFSYS_BUILD_LIBIBERTY=ON" + fi + echo "DEP_FLAGS=${flags}" >> "${GITHUB_ENV}" + + # The trailing -L narrows the suite to the tests that actually exercise + # Dyninst. binary_rewrite and runtime_instrument reach ctest as labels + # because tests/pytest/conftest.py emits the parametrised `mode` of each + # test as a CTest label; -LE matches the upstream exclusion. + - name: Generate CI scripts + timeout-minutes: 5 + run: | + echo "CMake: $(cmake --version | head -n 1)" + echo "Compiler: $(${{ matrix.compiler }} --version | head -n 1)" + # DEP_FLAGS holds several cmake -D arguments and must word-split. + # shellcheck disable=SC2086 + python3 ./scripts/run-ci.py --stage generate \ + --name "${CDASH_NAME}" \ + --site GitHub-dyninst \ + --build-jobs "$(nproc)" \ + -B build \ + -- \ + -DCMAKE_C_COMPILER=${{ matrix.cc }} \ + -DCMAKE_CXX_COMPILER=${{ matrix.compiler }} \ + -DCMAKE_BUILD_TYPE=Release \ + -DROCPROFSYS_BUILD_TESTING=ON \ + -DROCPROFSYS_BUILD_EXAMPLES=ON \ + -DROCPROFSYS_USE_PYTHON=ON \ + -DROCPROFSYS_BUILD_DYNINST=ON \ + -DROCPROFSYS_MAX_THREADS=64 \ + -DROCPROFSYS_PYTHON_PREFIX=/opt/conda/envs \ + ${DEP_FLAGS} \ + ${{ matrix.cmake_flags }} \ + -DROCPROFSYS_BUILD_NUMBER=${{ github.run_attempt }} \ + -- \ + -L "binary_rewrite|runtime_instrument" \ + -LE "network|gpu" + + - name: Configure + timeout-minutes: 20 + run: | + python3 ./scripts/run-ci.py --stage configure \ + --name "${CDASH_NAME}" \ + -B build + + # Without this, a failure of the early-return guard in + # rocprofiler_systems_checkout_git_submodule() would quietly build the + # pinned Dyninst and report a green result for the wrong code. + - name: Verify the pull request is what got configured + run: | + now=$(git -C external/dyninst rev-parse HEAD) + if [ "${now}" != "${DYNINST_SHA}" ]; then + echo "external/dyninst changed during configure: ${DYNINST_SHA} -> ${now}" + echo "CMake replaced the injected checkout, so this job is not testing the pull request." + exit 1 + fi + echo "ok external/dyninst is still ${now}" + + - name: Build + timeout-minutes: 90 + run: | + python3 ./scripts/run-ci.py --stage build \ + --name "${CDASH_NAME}" \ + -B build + + - name: Test + timeout-minutes: 60 + run: | + python3 ./scripts/run-ci.py --stage test \ + --name "${CDASH_NAME}" \ + -B build + + - name: CDash link + if: always() + run: | + python3 ./scripts/run-ci.py --stage cdash-link \ + --name "${CDASH_NAME}" + + - name: ccache stats + if: always() + run: ccache -s + + - name: Upload JUnit test results + if: always() + uses: actions/upload-artifact@v7 + with: + name: junit-${{ matrix.kind }}-${{ strategy.job-index }} + path: rocm-systems/projects/rocprofiler-systems/build/test-results.xml + if-no-files-found: ignore + + - name: Upload ctest logs + if: failure() + continue-on-error: true + uses: actions/upload-artifact@v7 + with: + name: ctest-${{ matrix.kind }}-${{ strategy.job-index }}-log + path: rocm-systems/projects/rocprofiler-systems/build/*.log + if-no-files-found: ignore + + - name: Upload test output + if: failure() + continue-on-error: true + uses: actions/upload-artifact@v7 + with: + name: data-${{ matrix.kind }}-${{ strategy.job-index }}-files + path: | + rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-config/*.cfg + rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*.txt + rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*-instr*.json + if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index 7600603c8a..9fdb9772f1 100644 --- a/.gitignore +++ b/.gitignore @@ -123,6 +123,8 @@ build*/ cmake-build-*/ # Staging prefix produced by scripts/build-tpls.sh .tpls/ +# Scratch directory produced by scripts/smoke/run-smoke.sh +.smoke/ .project .cproject .settings diff --git a/scripts/build-tpls.sh b/scripts/build-tpls.sh old mode 100644 new mode 100755 index 4e89f18da5..4466c3cb56 --- a/scripts/build-tpls.sh +++ b/scripts/build-tpls.sh @@ -24,7 +24,7 @@ set -euo pipefail script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=tpl-versions.env +# shellcheck source-path=SCRIPTDIR source=tpl-versions.env source "${script_dir}/tpl-versions.env" prefix="" diff --git a/scripts/smoke/mutatee.c b/scripts/smoke/mutatee.c new file mode 100644 index 0000000000..c9a48e2913 --- /dev/null +++ b/scripts/smoke/mutatee.c @@ -0,0 +1,28 @@ +/* + * Target program for the CI smoke tests. Shared by the binary-rewrite check and + * the runtime-instrumentation check. + * + * dyninst_marker() is deliberately never called. Its output appears only if a + * mutator inserted a call to it, which makes "did instrumentation actually + * execute" a grep rather than an inference from a tool's exit code. + */ + +#include + +__attribute__((noinline, used)) void dyninst_marker(void) +{ + printf("dyninst-marker-ran\n"); + fflush(stdout); +} + +__attribute__((noinline, used)) int work(int x) +{ + return x * 2; +} + +int main(void) +{ + printf("mutatee-ok %d\n", work(21)); + fflush(stdout); + return 0; +} diff --git a/scripts/smoke/run-smoke.sh b/scripts/smoke/run-smoke.sh new file mode 100755 index 0000000000..b24e499ca2 --- /dev/null +++ b/scripts/smoke/run-smoke.sh @@ -0,0 +1,242 @@ +#!/usr/bin/env bash +# +# Smoke-test an installed Dyninst against a trivial mutatee. +# +# Two checks, covering the two halves of Dyninst that a link-only test misses: +# +# binary rewrite parseThat instruments and writes out a new binary, which +# is then executed +# runtime instrument a mutator built against the install uses +# BPatch::processCreate to instrument a live process +# +# The runtime check is the only one that exercises ProcControlAPI and the +# injection of libdyninstAPI_RT, so it fails on a class of breakage that binary +# rewriting cannot see. +# +# Every Dyninst invocation is wrapped in a timeout. ProcControl process startup +# has been observed to hang rather than fail: the mutatee sits in ptrace_stop +# while the mutator's event thread waits on a futex that nothing will post, so +# without a bound the job stalls until the runner kills it and the log ends +# mid-step with no indication of which stage was to blame. +# +# Usage: +# run-smoke.sh --prefix DIR [--tpl-prefix DIR] [--cc CC] [--cxx CXX] +# [--workdir DIR] [--timeout SECONDS] [--retries N] +# +# --prefix Dyninst install prefix (must contain bin/parseThat and +# lib/libdyninstAPI_RT.so) +# --tpl-prefix third-party library prefix from build-tpls.sh, added to +# LD_LIBRARY_PATH so the install can resolve elfutils and TBB +# --timeout per-invocation limit in seconds (default 120) +# --retries extra attempts for the runtime check only, which is the one +# stage observed to hang nondeterministically (default 0) + +set -euo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +prefix="" +tpl_prefix="" +cc="${CC:-gcc}" +cxx="${CXX:-g++}" +workdir="" +timeout_secs=120 +retries=0 + +usage() { + sed -n '3,30p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --prefix) prefix="$2"; shift 2 ;; + --tpl-prefix) tpl_prefix="$2"; shift 2 ;; + --cc) cc="$2"; shift 2 ;; + --cxx) cxx="$2"; shift 2 ;; + --workdir) workdir="$2"; shift 2 ;; + --timeout) timeout_secs="$2"; shift 2 ;; + --retries) retries="$2"; shift 2 ;; + -h|--help) usage; exit 0 ;; + *) echo "error: unknown argument '$1'" >&2; usage >&2; exit 2 ;; + esac +done + +if [[ -z "${prefix}" ]]; then + echo "error: --prefix is required" >&2 + exit 2 +fi +prefix="$(cd "${prefix}" && pwd)" + +if [[ -z "${workdir}" ]]; then + workdir="$(mktemp -d)" + trap 'rm -rf "${workdir}"' EXIT +fi +mkdir -p "${workdir}" + +libdir="${prefix}/lib" +[[ -d "${libdir}" ]] || libdir="${prefix}/lib64" + +export DYNINSTAPI_RT_LIB="${libdir}/libdyninstAPI_RT.so" + +# Two paths, because the runtime check deliberately runs without ${libdir} on +# it. Measured against a Dyninst 13.0.0 install, BPatch::processCreate hung +# 10 times out of 10 with ${libdir} on LD_LIBRARY_PATH and roughly 1 in 10 +# without it, so keeping the mutator's Dyninst libraries on the search path +# turns an occasional hang into a guaranteed one. The mutator is linked with an +# rpath to ${libdir}, so it resolves them without help; the third-party entries +# have to stay because libdyninstAPI.so's own dependencies are found through +# LD_LIBRARY_PATH, not through the mutator's rpath. +tpl_ld_path="${tpl_prefix:+${tpl_prefix}/elfutils/lib:${tpl_prefix}/tbb/lib}" + +runtime_ld_path="${tpl_ld_path}" +if [[ -n "${LD_LIBRARY_PATH:-}" ]]; then + runtime_ld_path="${runtime_ld_path:+${runtime_ld_path}:}${LD_LIBRARY_PATH}" +fi + +if [[ -n "${runtime_ld_path}" ]]; then + runtime_env=(env "LD_LIBRARY_PATH=${runtime_ld_path}") +else + # An empty LD_LIBRARY_PATH is not the same as an unset one: the loader reads + # the empty entry as the current directory. + runtime_env=(env -u LD_LIBRARY_PATH) +fi + +export LD_LIBRARY_PATH="${libdir}${runtime_ld_path:+:${runtime_ld_path}}" + +if [[ ! -f "${DYNINSTAPI_RT_LIB}" ]]; then + echo "error: ${DYNINSTAPI_RT_LIB} not found" >&2 + exit 1 +fi + +step() { printf '\n=== %s ===\n' "$1"; } + +# SIGKILL rather than the default SIGTERM: a mutator wedged inside ProcControl +# is not reliably servicing signals, and a timeout that itself hangs is worse +# than no timeout at all. GNU timeout then reports 137 instead of 124. +is_timeout() { [[ "$1" -eq 124 || "$1" -eq 137 ]]; } + +# Runs a command under the timeout and tees its output, leaving the status in +# run_rc. errexit is lifted around the pipeline so the caller can tell a hang +# apart from a non-zero exit instead of the script dying on the spot. +run_logged() { + local logfile="$1" + shift + set +e + timeout -s KILL "${timeout_secs}" "$@" 2>&1 | tee "${logfile}" + run_rc="${PIPESTATUS[0]}" + set -e +} + +report_failure() { + if is_timeout "$2"; then + echo "error: $1 did not finish within ${timeout_secs}s" >&2 + echo " this is a hang, not a failed assertion; see ${3}" >&2 + else + echo "error: $1 exited $2" >&2 + fi +} + +# --------------------------------------------------------------------------- +# Baseline +# --------------------------------------------------------------------------- + +step "build the mutatee" +# -O0 keeps work() and dyninst_marker() as distinct, findable functions. +"${cc}" -g -O0 -o "${workdir}/mutatee" "${script_dir}/mutatee.c" + +step "run the mutatee uninstrumented" +run_logged "${workdir}/baseline.txt" "${workdir}/mutatee" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "the uninstrumented mutatee" "${run_rc}" "${workdir}/baseline.txt" + exit 1 +fi +grep -q 'mutatee-ok 42' "${workdir}/baseline.txt" +# Nothing has been instrumented yet, so the marker must be silent. If this +# fires, the mutatee itself calls dyninst_marker() and the runtime check below +# would pass without Dyninst doing anything. +if grep -q 'dyninst-marker-ran' "${workdir}/baseline.txt"; then + echo "error: marker ran without instrumentation" >&2 + exit 1 +fi + +# --------------------------------------------------------------------------- +# Binary rewrite +# --------------------------------------------------------------------------- + +step "rewrite the mutatee with parseThat" +# -i 1 instruments function entries, so this covers instrumentation rather than +# only parse-and-write-back. +run_logged "${workdir}/rewrite.txt" \ + "${prefix}/bin/parseThat" -i 1 \ + --binary-edit="${workdir}/mutatee.rewritten" \ + "${workdir}/mutatee" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "parseThat" "${run_rc}" "${workdir}/rewrite.txt" + exit 1 +fi + +step "run the rewritten mutatee" +test -x "${workdir}/mutatee.rewritten" +run_logged "${workdir}/rewritten.txt" "${workdir}/mutatee.rewritten" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "the rewritten mutatee" "${run_rc}" "${workdir}/rewritten.txt" + exit 1 +fi +grep -q 'mutatee-ok 42' "${workdir}/rewritten.txt" + +# --------------------------------------------------------------------------- +# Runtime instrumentation +# --------------------------------------------------------------------------- + +step "build the runtime-instrumentation mutator" +# Without this, a prefix missing its headers falls through to whatever Dyninst +# happens to sit in a default include path, and the failure arrives as a wall of +# template errors from the wrong version. +if [[ ! -f "${prefix}/include/BPatch.h" ]]; then + echo "error: ${prefix}/include/BPatch.h not found; is --prefix an install tree?" >&2 + exit 1 +fi +"${cxx}" -std=c++17 -g -O0 \ + -o "${workdir}/runtime_instrument" \ + "${script_dir}/runtime_instrument.cpp" \ + -I"${prefix}/include" \ + -L"${libdir}" -Wl,-rpath,"${libdir}" \ + -ldyninstAPI + +step "instrument a live process with BPatch::processCreate" +# Logged unconditionally: a Dyninst lib directory reaching this variable is the +# one known cause of a hang here, and a container that sets LD_LIBRARY_PATH +# itself can reintroduce it without this script changing. +echo "runtime LD_LIBRARY_PATH: ${runtime_ld_path:-(unset)}" + +# Only a hang is retried, and only when --retries asks for it. A mutator that +# runs to completion with the wrong output is a regression and must stay red. +attempt=0 +while true; do + run_logged "${workdir}/runtime.txt" \ + "${runtime_env[@]}" "${workdir}/runtime_instrument" "${workdir}/mutatee" + + if [[ "${run_rc}" -eq 0 ]]; then + break + fi + + if is_timeout "${run_rc}" && [[ "${attempt}" -lt "${retries}" ]]; then + attempt=$((attempt + 1)) + echo "warning: runtime instrumentation hung, retrying (${attempt}/${retries})" >&2 + continue + fi + + report_failure "runtime instrumentation" "${run_rc}" "${workdir}/runtime.txt" + if is_timeout "${run_rc}"; then + echo " check whether a Dyninst lib directory reached LD_LIBRARY_PATH" >&2 + echo " (logged above); that reproduces this hang every time" >&2 + fi + exit 1 +done + +# The mutatee never calls dyninst_marker() itself, so this line can only come +# from the snippet inserted at the entry of work(). +grep -q 'dyninst-marker-ran' "${workdir}/runtime.txt" +grep -q 'mutatee-ok 42' "${workdir}/runtime.txt" + +printf '\nall smoke checks passed\n' diff --git a/scripts/smoke/runtime_instrument.cpp b/scripts/smoke/runtime_instrument.cpp new file mode 100644 index 0000000000..23fd5167c9 --- /dev/null +++ b/scripts/smoke/runtime_instrument.cpp @@ -0,0 +1,105 @@ +/* + * Runtime-instrumentation smoke test. + * + * Creates a process with BPatch::processCreate, inserts a call to the mutatee's + * dyninst_marker() at the entry of work(), and runs it to completion. This is + * the half of Dyninst that binary rewriting never touches: ProcControlAPI + * process control, injection of libdyninstAPI_RT into a live process, and code + * patching of a running mutatee. + * + * Mirrors the runtime_instrument mode of the rocprofiler-systems test suite, so + * a break here shows up before the much slower downstream job runs. + * + * Usage: runtime_instrument + */ + +#include "BPatch.h" +#include "BPatch_addressSpace.h" +#include "BPatch_function.h" +#include "BPatch_image.h" +#include "BPatch_point.h" +#include "BPatch_process.h" +#include "BPatch_snippet.h" + +#include +#include + +namespace { + +BPatch_function* find_only(BPatch_image* image, const char* name) +{ + BPatch_Vector funcs; + image->findFunction(name, funcs); + + // Anything other than exactly one match means the mutatee was built + // differently than this test assumes, not that Dyninst is broken. + if(funcs.size() != 1) + { + std::fprintf(stderr, "error: expected exactly one '%s', found %lu\n", name, + static_cast(funcs.size())); + return nullptr; + } + return funcs[0]; +} + +} // namespace + +int main(int argc, char* argv[]) +{ + if(argc != 2) + { + std::fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + + const char* mutatee = argv[1]; + const char* mutatee_argv[] = { mutatee, nullptr }; + + BPatch bpatch; + BPatch_process* proc = bpatch.processCreate(mutatee, mutatee_argv); + if(proc == nullptr) + { + std::fprintf(stderr, "error: processCreate('%s') failed\n", mutatee); + return 1; + } + + BPatch_image* image = proc->getImage(); + if(image == nullptr) + { + std::fprintf(stderr, "error: getImage() returned null\n"); + return 1; + } + + BPatch_function* work = find_only(image, "work"); + BPatch_function* marker = find_only(image, "dyninst_marker"); + if(work == nullptr || marker == nullptr) return 1; + + BPatch_Vector* entry = work->findPoint(BPatch_locEntry); + if(entry == nullptr || entry->empty()) + { + std::fprintf(stderr, "error: no entry point found for work()\n"); + return 1; + } + + BPatch_Vector no_args; + BPatch_funcCallExpr call_marker(*marker, no_args); + + if(proc->insertSnippet(call_marker, *entry) == nullptr) + { + std::fprintf(stderr, "error: insertSnippet() failed\n"); + return 1; + } + + if(!proc->continueExecution()) + { + std::fprintf(stderr, "error: continueExecution() failed\n"); + return 1; + } + + while(!proc->isTerminated()) + bpatch.waitForStatusChange(); + + int exit_code = proc->getExitCode(); + std::fprintf(stderr, "mutatee terminated, exit code %d\n", exit_code); + return exit_code; +} From 4dae85e5843202d21487b538d5f177817d7fc009 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Tue, 11 Aug 2026 11:01:27 -0400 Subject: [PATCH 3/9] Phase 2 - Fixes --- .github/workflows/build.yaml | 6 +- .pre-commit-config.yaml | 51 +++++++++ CMakeLists.txt | 11 ++ scripts/smoke/CMakeLists.txt | 27 +++++ scripts/smoke/binary_rewrite.cpp | 55 ++++++++++ scripts/smoke/run-smoke.sh | 151 +++++++++++++++++++++------ scripts/smoke/runtime_instrument.cpp | 61 ++--------- scripts/smoke/smoke_mutator.h | 71 +++++++++++++ 8 files changed, 346 insertions(+), 87 deletions(-) create mode 100644 .pre-commit-config.yaml create mode 100644 scripts/smoke/CMakeLists.txt create mode 100644 scripts/smoke/binary_rewrite.cpp create mode 100644 scripts/smoke/smoke_mutator.h diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 11b0ce914a..dc51cc8ae0 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -175,7 +175,7 @@ jobs: --tpl-prefix "${TPL_PREFIX}" \ --cc ${{ inputs.cc }} \ --cxx ${{ inputs.cxx }} \ - --workdir "${{ github.workspace }}/.smoke" \ + --workdir "${GITHUB_WORKSPACE}/.smoke" \ --timeout 120 \ --retries 1 @@ -196,10 +196,14 @@ jobs: # The captured stdout of each stage is what distinguishes "instrumentation # did not run" from "the mutatee itself failed". + # + # include-hidden-files is required because .smoke is a dot directory, and + # upload-artifact skips those by default without treating it as an error. - name: Upload smoke test output if: failure() uses: actions/upload-artifact@v7 with: name: smoke-${{ inputs.name }}-${{ matrix.build-type }} + include-hidden-files: true path: .smoke/*.txt if-no-files-found: ignore diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000000..3367e5e341 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,51 @@ +# Formats and lints the files CI checks, so the lint workflow never asks for a +# manual fix. Enable it once per clone with: +# +# pip install pre-commit && pre-commit install +# +# CI runs these same hooks, so the pins below are the single definition of +# "correct" for this repository. +# +# Every hook is declared locally with a pinned pip package rather than pulled +# from an upstream hook repository. cheshirekow/cmake_format publishes no +# .pre-commit-hooks.yaml at all, and doing the same thing for the rest keeps one +# mechanism instead of three. + +repos: + - repo: local + hooks: + # The [YAML] extra is required: cmakelang imports PyYAML lazily and does + # not depend on it, so reading .cmake-format.yaml fails with + # ModuleNotFoundError without it. + - id: cmake-format + name: cmake-format + entry: cmake-format --in-place + language: python + additional_dependencies: ['cmake-format[YAML]==0.6.13'] + files: (^|/)CMakeLists\.txt$|\.cmake$ + + # Scoped to scripts/. The inherited docker/ and instructionAPI/ scripts + # have dozens of pre-existing findings, and rewriting them would churn + # files that still merge from upstream Dyninst for no CI benefit. + # + # --external-sources lets it follow `source`d files such as + # scripts/tpl-versions.env instead of reporting SC1091. + - id: shellcheck + name: shellcheck + entry: shellcheck --external-sources + language: python + additional_dependencies: ['shellcheck-py==0.11.0.1'] + files: ^scripts/ + types: [shell] + + # actionlint shells out to shellcheck to check inline `run:` blocks, which + # is where most of this repository's untested shell lives. It only does so + # when shellcheck is on PATH, hence the second dependency. + - id: actionlint + name: actionlint + entry: actionlint + language: python + additional_dependencies: + - 'actionlint-py==1.7.12.24' + - 'shellcheck-py==0.11.0.1' + files: ^\.github/workflows/.*\.ya?ml$ diff --git a/CMakeLists.txt b/CMakeLists.txt index 5174c479f0..ac632af7da 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,6 +34,17 @@ include_directories(SYSTEM "${PROJECT_SOURCE_DIR}") include(DyninstLibrarySettings) include(DyninstOptions) +# Installed public headers include (concurrent.h and most of the +# BPatch_* headers do), so the directory has to ship alongside them. Without +# this, the include_directories above makes the build work while anything +# compiling against the install tree fails to find the header. +install( + DIRECTORY "${PROJECT_SOURCE_DIR}/dyncompat" + DESTINATION "${DYNINST_INSTALL_INCLUDEDIR}" + FILES_MATCHING + PATTERN "*.h" + PATTERN "*.hpp") + # Set up Dyninst internals include(DyninstPlatform) include(DyninstCapArchDef) diff --git a/scripts/smoke/CMakeLists.txt b/scripts/smoke/CMakeLists.txt new file mode 100644 index 0000000000..b39f61f4da --- /dev/null +++ b/scripts/smoke/CMakeLists.txt @@ -0,0 +1,27 @@ +# Builds the smoke-test mutators against an installed Dyninst. +# +# This goes through find_package rather than a hand-written compiler line +# because the installed public headers include third-party headers of their own +# -- among them -- and the include directories that +# resolve those are carried by the exported targets. Listing them by hand means +# rediscovering each one as a CI failure, and on a machine that happens to have +# the library installed system-wide the omission does not show up at all. +# +# Going through the package config also puts the same consumer path +# rocprofiler-systems uses under test, so an install tree that is unusable from +# outside the build directory fails here rather than downstream. +# +# TBB_ROOT_DIR and ElfUtils_ROOT_DIR are read by the find modules the config +# ships; pass the same values the Dyninst build was given. + +cmake_minimum_required(VERSION 3.14) + +project(dyninst-smoke LANGUAGES CXX) + +find_package(Dyninst REQUIRED COMPONENTS dyninstAPI) + +foreach(_mutator binary_rewrite runtime_instrument) + add_executable(${_mutator} "${CMAKE_CURRENT_SOURCE_DIR}/${_mutator}.cpp") + target_compile_features(${_mutator} PRIVATE cxx_std_17) + target_link_libraries(${_mutator} PRIVATE Dyninst::dyninstAPI) +endforeach() diff --git a/scripts/smoke/binary_rewrite.cpp b/scripts/smoke/binary_rewrite.cpp new file mode 100644 index 0000000000..50d6799a34 --- /dev/null +++ b/scripts/smoke/binary_rewrite.cpp @@ -0,0 +1,55 @@ +/* + * Binary-rewrite smoke test. + * + * Opens the mutatee with BPatch::openBinary, inserts a call to its + * dyninst_marker() at the entry of work(), and writes the result out. The + * script then runs that binary and greps for the marker. + * + * This exists because "the rewritten binary still produces the right answer" is + * not evidence that anything was instrumented: a rewriter that selects zero + * functions and writes back a working copy passes that check. Downstream + * rocprofiler-systems tests fail exactly that way, reporting a successful + * rewrite whose output carries no instrumentation, so the assertion here has to + * be the marker firing rather than the mutatee's own output. + * + * Usage: binary_rewrite + */ + +#include "BPatch.h" +#include "BPatch_binaryEdit.h" + +#include "smoke_mutator.h" + +#include + +int main(int argc, char* argv[]) +{ + if(argc != 3) + { + std::fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + + const char* mutatee = argv[1]; + const char* output = argv[2]; + + BPatch bpatch; + + BPatch_binaryEdit* binary = bpatch.openBinary(mutatee); + if(binary == nullptr) + { + std::fprintf(stderr, "error: openBinary('%s') failed\n", mutatee); + return 1; + } + + if(!smoke::insert_marker(binary)) return 1; + + if(!binary->writeFile(output)) + { + std::fprintf(stderr, "error: writeFile('%s') failed\n", output); + return 1; + } + + std::fprintf(stderr, "wrote instrumented binary to %s\n", output); + return 0; +} diff --git a/scripts/smoke/run-smoke.sh b/scripts/smoke/run-smoke.sh index b24e499ca2..25e311f7f0 100755 --- a/scripts/smoke/run-smoke.sh +++ b/scripts/smoke/run-smoke.sh @@ -4,8 +4,9 @@ # # Two checks, covering the two halves of Dyninst that a link-only test misses: # -# binary rewrite parseThat instruments and writes out a new binary, which -# is then executed +# binary rewrite parseThat writes out a new binary, and a mutator built +# against the install writes out another with a snippet of +# our own in it # runtime instrument a mutator built against the install uses # BPatch::processCreate to instrument a live process # @@ -13,6 +14,11 @@ # injection of libdyninstAPI_RT, so it fails on a class of breakage that binary # rewriting cannot see. # +# Both checks assert that an inserted snippet ran rather than that the tools +# exited zero and the mutatee still works. A rewriter that selects no functions +# at all satisfies the weaker condition, and that is the shape of the binary +# rewriting failures seen downstream in rocprofiler-systems. +# # Every Dyninst invocation is wrapped in a timeout. ProcControl process startup # has been observed to hang rather than fail: the mutatee sits in ptrace_stop # while the mutator's event thread waits on a futex that nothing will post, so @@ -76,22 +82,53 @@ mkdir -p "${workdir}" libdir="${prefix}/lib" [[ -d "${libdir}" ]] || libdir="${prefix}/lib64" +# Honoured by the ppc, aarch64 and FreeBSD backends. The x86_64 Linux backend +# ignores it and resolves the runtime library by search instead, which is what +# DYNINST_REWRITER_PATHS below is for. export DYNINSTAPI_RT_LIB="${libdir}/libdyninstAPI_RT.so" -# Two paths, because the runtime check deliberately runs without ${libdir} on -# it. Measured against a Dyninst 13.0.0 install, BPatch::processCreate hung -# 10 times out of 10 with ${libdir} on LD_LIBRARY_PATH and roughly 1 in 10 -# without it, so keeping the mutator's Dyninst libraries on the search path -# turns an occasional hang into a guaranteed one. The mutator is linked with an -# rpath to ${libdir}, so it resolves them without help; the third-party entries -# have to stay because libdyninstAPI.so's own dependencies are found through -# LD_LIBRARY_PATH, not through the mutator's rpath. +# How PCProcess::setEnvPreload finds the library it LD_PRELOADs into the +# mutatee. On x86_64 Linux it calls BinaryEdit::getResolvedLibraryPath with the +# bare name "libdyninstAPI_RT.so" and takes the first hit from, in order, this +# variable, LD_LIBRARY_PATH, and the compiler's search directories. Naming +# ${libdir} here pins the runtime library to the install under test and lets the +# runtime check keep ${libdir} off LD_LIBRARY_PATH: without it the search either +# finds nothing, and process creation fails during bootstrap with no explanation +# unless DYNINST_DEBUG_STARTUP is set, or finds some other Dyninst that happens +# to be on the path and instruments the mutatee with the wrong runtime. +export DYNINST_REWRITER_PATHS="${libdir}" + tpl_ld_path="${tpl_prefix:+${tpl_prefix}/elfutils/lib:${tpl_prefix}/tbb/lib}" +# Two paths, because the runtime check runs without ${libdir} on it: the mutator +# reaches its own libraries through the RPATH cmake links it with, and leaving +# the directory off means nothing on the loader's search path can stand in for +# the install being tested. The third-party entries have to stay, because +# libdyninstAPI.so's own dependencies resolve through LD_LIBRARY_PATH rather +# than through the mutator's RUNPATH, which is not transitive. +# +# The inherited value is filtered the same way rather than trusted, since a +# developer shell or a container image can point it at a different Dyninst. +strip_dyninst_dirs() { + local result="" entry + local -a entries=() + IFS=':' read -r -a entries <<< "$1" + for entry in "${entries[@]}"; do + [[ -z "${entry}" ]] && continue + if compgen -G "${entry}/libdyninstAPI*.so*" > /dev/null; then + echo "note: dropping Dyninst library directory '${entry}' from the runtime search path" >&2 + continue + fi + result="${result:+${result}:}${entry}" + done + printf '%s' "${result}" +} + runtime_ld_path="${tpl_ld_path}" if [[ -n "${LD_LIBRARY_PATH:-}" ]]; then runtime_ld_path="${runtime_ld_path:+${runtime_ld_path}:}${LD_LIBRARY_PATH}" fi +runtime_ld_path="$(strip_dyninst_dirs "${runtime_ld_path}")" if [[ -n "${runtime_ld_path}" ]]; then runtime_env=(env "LD_LIBRARY_PATH=${runtime_ld_path}") @@ -159,6 +196,50 @@ if grep -q 'dyninst-marker-ran' "${workdir}/baseline.txt"; then exit 1 fi +# --------------------------------------------------------------------------- +# Mutators +# --------------------------------------------------------------------------- + +step "build the mutators" +# Without this, a prefix missing its headers falls through to whatever Dyninst +# happens to sit in a default include path, and the failure arrives as a wall of +# template errors from the wrong version. +if [[ ! -f "${prefix}/include/BPatch.h" ]]; then + echo "error: ${prefix}/include/BPatch.h not found; is --prefix an install tree?" >&2 + exit 1 +fi + +# See scripts/smoke/CMakeLists.txt for why this is a CMake build and not a +# compiler invocation. The mutators end up with a build-tree RPATH covering the +# install's libraries, which is what lets the runtime check below drop them from +# LD_LIBRARY_PATH. +mutator_build="${workdir}/mutators" +cmake_args=( + -S "${script_dir}" + -B "${mutator_build}" + -DCMAKE_BUILD_TYPE=RelWithDebInfo + -DCMAKE_CXX_COMPILER="${cxx}" + -DCMAKE_PREFIX_PATH="${prefix}" +) +if [[ -n "${tpl_prefix}" ]]; then + cmake_args+=( + -DTBB_ROOT_DIR="${tpl_prefix}/tbb" + -DElfUtils_ROOT_DIR="${tpl_prefix}/elfutils" + ) +fi + +run_logged "${workdir}/cmake.txt" cmake "${cmake_args[@]}" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "configuring the mutators" "${run_rc}" "${workdir}/cmake.txt" + exit 1 +fi + +run_logged "${workdir}/cmake-build.txt" cmake --build "${mutator_build}" --parallel +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "building the mutators" "${run_rc}" "${workdir}/cmake-build.txt" + exit 1 +fi + # --------------------------------------------------------------------------- # Binary rewrite # --------------------------------------------------------------------------- @@ -184,29 +265,36 @@ if [[ "${run_rc}" -ne 0 ]]; then fi grep -q 'mutatee-ok 42' "${workdir}/rewritten.txt" -# --------------------------------------------------------------------------- -# Runtime instrumentation -# --------------------------------------------------------------------------- +step "rewrite the mutatee with a snippet of our own" +# parseThat above shows the rewriter produces a working binary, which it also +# does when it instruments nothing at all. This inserts a call we can observe so +# the run below can tell those two outcomes apart. +run_logged "${workdir}/rewrite-snippet.txt" \ + "${mutator_build}/binary_rewrite" "${workdir}/mutatee" "${workdir}/mutatee.snippet" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "binary rewriting" "${run_rc}" "${workdir}/rewrite-snippet.txt" + exit 1 +fi -step "build the runtime-instrumentation mutator" -# Without this, a prefix missing its headers falls through to whatever Dyninst -# happens to sit in a default include path, and the failure arrives as a wall of -# template errors from the wrong version. -if [[ ! -f "${prefix}/include/BPatch.h" ]]; then - echo "error: ${prefix}/include/BPatch.h not found; is --prefix an install tree?" >&2 +step "run the rewritten mutatee carrying the snippet" +test -x "${workdir}/mutatee.snippet" +run_logged "${workdir}/snippet.txt" "${workdir}/mutatee.snippet" +if [[ "${run_rc}" -ne 0 ]]; then + report_failure "the rewritten mutatee" "${run_rc}" "${workdir}/snippet.txt" exit 1 fi -"${cxx}" -std=c++17 -g -O0 \ - -o "${workdir}/runtime_instrument" \ - "${script_dir}/runtime_instrument.cpp" \ - -I"${prefix}/include" \ - -L"${libdir}" -Wl,-rpath,"${libdir}" \ - -ldyninstAPI +# The mutatee never calls dyninst_marker() itself, so this line can only come +# from the snippet written into the entry of work(). +grep -q 'dyninst-marker-ran' "${workdir}/snippet.txt" +grep -q 'mutatee-ok 42' "${workdir}/snippet.txt" + +# --------------------------------------------------------------------------- +# Runtime instrumentation +# --------------------------------------------------------------------------- step "instrument a live process with BPatch::processCreate" -# Logged unconditionally: a Dyninst lib directory reaching this variable is the -# one known cause of a hang here, and a container that sets LD_LIBRARY_PATH -# itself can reintroduce it without this script changing. +# Logged unconditionally, so that a hang here can be read against the search +# path that produced it without having to reconstruct what was filtered. echo "runtime LD_LIBRARY_PATH: ${runtime_ld_path:-(unset)}" # Only a hang is retried, and only when --retries asks for it. A mutator that @@ -214,7 +302,7 @@ echo "runtime LD_LIBRARY_PATH: ${runtime_ld_path:-(unset)}" attempt=0 while true; do run_logged "${workdir}/runtime.txt" \ - "${runtime_env[@]}" "${workdir}/runtime_instrument" "${workdir}/mutatee" + "${runtime_env[@]}" "${mutator_build}/runtime_instrument" "${workdir}/mutatee" if [[ "${run_rc}" -eq 0 ]]; then break @@ -228,8 +316,9 @@ while true; do report_failure "runtime instrumentation" "${run_rc}" "${workdir}/runtime.txt" if is_timeout "${run_rc}"; then - echo " check whether a Dyninst lib directory reached LD_LIBRARY_PATH" >&2 - echo " (logged above); that reproduces this hang every time" >&2 + echo " re-run with DYNINST_DEBUG_STARTUP=1 DYNINST_DEBUG_PROCCONTROL=1" >&2 + echo " to see which runtime library was injected and how far" >&2 + echo " bootstrap got before it stopped" >&2 fi exit 1 done diff --git a/scripts/smoke/runtime_instrument.cpp b/scripts/smoke/runtime_instrument.cpp index 23fd5167c9..75ca332d63 100644 --- a/scripts/smoke/runtime_instrument.cpp +++ b/scripts/smoke/runtime_instrument.cpp @@ -14,35 +14,11 @@ */ #include "BPatch.h" -#include "BPatch_addressSpace.h" -#include "BPatch_function.h" -#include "BPatch_image.h" -#include "BPatch_point.h" #include "BPatch_process.h" -#include "BPatch_snippet.h" -#include -#include - -namespace { - -BPatch_function* find_only(BPatch_image* image, const char* name) -{ - BPatch_Vector funcs; - image->findFunction(name, funcs); +#include "smoke_mutator.h" - // Anything other than exactly one match means the mutatee was built - // differently than this test assumes, not that Dyninst is broken. - if(funcs.size() != 1) - { - std::fprintf(stderr, "error: expected exactly one '%s', found %lu\n", name, - static_cast(funcs.size())); - return nullptr; - } - return funcs[0]; -} - -} // namespace +#include int main(int argc, char* argv[]) { @@ -52,43 +28,18 @@ int main(int argc, char* argv[]) return 2; } - const char* mutatee = argv[1]; + const char* mutatee = argv[1]; const char* mutatee_argv[] = { mutatee, nullptr }; - BPatch bpatch; - BPatch_process* proc = bpatch.processCreate(mutatee, mutatee_argv); + BPatch bpatch; + BPatch_process* proc = bpatch.processCreate(mutatee, mutatee_argv); if(proc == nullptr) { std::fprintf(stderr, "error: processCreate('%s') failed\n", mutatee); return 1; } - BPatch_image* image = proc->getImage(); - if(image == nullptr) - { - std::fprintf(stderr, "error: getImage() returned null\n"); - return 1; - } - - BPatch_function* work = find_only(image, "work"); - BPatch_function* marker = find_only(image, "dyninst_marker"); - if(work == nullptr || marker == nullptr) return 1; - - BPatch_Vector* entry = work->findPoint(BPatch_locEntry); - if(entry == nullptr || entry->empty()) - { - std::fprintf(stderr, "error: no entry point found for work()\n"); - return 1; - } - - BPatch_Vector no_args; - BPatch_funcCallExpr call_marker(*marker, no_args); - - if(proc->insertSnippet(call_marker, *entry) == nullptr) - { - std::fprintf(stderr, "error: insertSnippet() failed\n"); - return 1; - } + if(!smoke::insert_marker(proc)) return 1; if(!proc->continueExecution()) { diff --git a/scripts/smoke/smoke_mutator.h b/scripts/smoke/smoke_mutator.h new file mode 100644 index 0000000000..fe69c3c784 --- /dev/null +++ b/scripts/smoke/smoke_mutator.h @@ -0,0 +1,71 @@ +/* + * Shared by the two smoke-test mutators. + * + * Both checks insert the same snippet into the same mutatee and differ only in + * how the address space is obtained and what is done with it afterwards, so the + * instrumentation itself lives here rather than being written twice. + */ + +#pragma once + +#include "BPatch_addressSpace.h" +#include "BPatch_function.h" +#include "BPatch_image.h" +#include "BPatch_point.h" +#include "BPatch_snippet.h" + +#include + +namespace smoke +{ + +inline BPatch_function* find_only(BPatch_image* image, const char* name) +{ + BPatch_Vector funcs; + image->findFunction(name, funcs); + + // Anything other than exactly one match means the mutatee was built + // differently than this test assumes, not that Dyninst is broken. + if(funcs.size() != 1) + { + std::fprintf(stderr, "error: expected exactly one '%s', found %lu\n", name, + static_cast(funcs.size())); + return nullptr; + } + return funcs[0]; +} + +// Inserts a call to the mutatee's dyninst_marker() at the entry of work(). +inline bool insert_marker(BPatch_addressSpace* aspace) +{ + BPatch_image* image = aspace->getImage(); + if(image == nullptr) + { + std::fprintf(stderr, "error: getImage() returned null\n"); + return false; + } + + BPatch_function* work = find_only(image, "work"); + BPatch_function* marker = find_only(image, "dyninst_marker"); + if(work == nullptr || marker == nullptr) return false; + + BPatch_Vector* entry = work->findPoint(BPatch_locEntry); + if(entry == nullptr || entry->empty()) + { + std::fprintf(stderr, "error: no entry point found for work()\n"); + return false; + } + + BPatch_Vector no_args; + BPatch_funcCallExpr call_marker(*marker, no_args); + + if(aspace->insertSnippet(call_marker, *entry) == nullptr) + { + std::fprintf(stderr, "error: insertSnippet() failed\n"); + return false; + } + + return true; +} + +} // namespace smoke From 6aef7e42379e6ced2779db634897fa0d7ed363e8 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Tue, 11 Aug 2026 13:01:09 -0400 Subject: [PATCH 4/9] Prevent instrumentation in dyninst folder --- .github/workflows/rocprofiler-systems.yaml | 34 ++++++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/.github/workflows/rocprofiler-systems.yaml b/.github/workflows/rocprofiler-systems.yaml index bad9edff16..f5b6c8a84e 100644 --- a/.github/workflows/rocprofiler-systems.yaml +++ b/.github/workflows/rocprofiler-systems.yaml @@ -116,7 +116,8 @@ jobs: defaults: run: shell: bash - working-directory: rocm-systems/projects/rocprofiler-systems + # Not under GITHUB_WORKSPACE; see "Move the checkout off the dyninst path". + working-directory: /rocm-systems/projects/rocprofiler-systems env: CDASH_NAME: dyninst-${{ github.event.number || github.ref_name }}-${{ matrix.cdash_suffix }} @@ -150,6 +151,27 @@ jobs: with: path: rocm-systems/projects/rocprofiler-systems/external/dyninst + # rocprof-sys-instrument discards any module whose path matches the regex + # "dyninst" (is_module_constrained in module_function.cpp), and the module + # name is the full source path. GITHUB_WORKSPACE is /__w/dyninst/dyninst + # because this repository is named dyninst, so a build left there produces + # test binaries with zero functions instrumented: the rewrite succeeds, the + # tests run, and every trace assertion fails for a reason unrelated to + # Dyninst. No command-line flag overrides it, as the module check precedes + # the --min-instructions and --include filters. + - name: Move the checkout off the "dyninst" path + working-directory: ${{ github.workspace }} + run: | + rm -rf /rocm-systems + mv rocm-systems /rocm-systems + # -P because the compiler records the resolved path, not the logical one. + build_root=$(cd /rocm-systems/projects/rocprofiler-systems && pwd -P) + echo "building in ${build_root}" + if echo "${build_root}" | grep -qi dyninst; then + echo "Path still matches 'dyninst'; every test binary would be built uninstrumented." + exit 1 + fi + - name: Record the versions under test run: | git config --global --add safe.directory '*' @@ -334,7 +356,7 @@ jobs: uses: actions/upload-artifact@v7 with: name: junit-${{ matrix.kind }}-${{ strategy.job-index }} - path: rocm-systems/projects/rocprofiler-systems/build/test-results.xml + path: /rocm-systems/projects/rocprofiler-systems/build/test-results.xml if-no-files-found: ignore - name: Upload ctest logs @@ -343,7 +365,7 @@ jobs: uses: actions/upload-artifact@v7 with: name: ctest-${{ matrix.kind }}-${{ strategy.job-index }}-log - path: rocm-systems/projects/rocprofiler-systems/build/*.log + path: /rocm-systems/projects/rocprofiler-systems/build/*.log if-no-files-found: ignore - name: Upload test output @@ -353,7 +375,7 @@ jobs: with: name: data-${{ matrix.kind }}-${{ strategy.job-index }}-files path: | - rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-config/*.cfg - rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*.txt - rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*-instr*.json + /rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-config/*.cfg + /rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*.txt + /rocm-systems/projects/rocprofiler-systems/build/rocprofsys-tests-output/**/*-instr*.json if-no-files-found: ignore From 57593f7a289d2ec594ca25fd74941cdb23bc6ee3 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Tue, 11 Aug 2026 14:35:25 -0400 Subject: [PATCH 5/9] Enable remaining workflows --- .github/workflows/pr-tests.yaml | 38 +++++++++++++----- .github/workflows/rocprofiler-systems.yaml | 6 ++- scripts/build-tpls.sh | 45 +++++++++++++++++++--- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml index d69fc79da6..81ea37fefe 100644 --- a/.github/workflows/pr-tests.yaml +++ b/.github/workflows/pr-tests.yaml @@ -2,9 +2,14 @@ name: PR Tests # Goal 1: Dyninst builds and can instrument and run a program on its own. # -# Currently a single distro while the from-source third-party library approach -# proves itself; the remaining images in the rocprofiler-systems CI matrix are -# added once this is green. +# The images are the ROCm 7.2 primary entries of the rocprofiler-systems CI +# matrix, which are the environments this fork actually has to work in. They are +# listed here rather than fetched from ci-build-matrix.json because a standalone +# Dyninst build needs only the image name, and pinning the list keeps a change +# to the downstream matrix from silently altering what this workflow builds. +# +# Every entry is g++/gcc, which is what the reusable builder defaults to, so +# adding an amdclang++ row means explicitly passing cc/cxx. on: pull_request: @@ -19,12 +24,25 @@ concurrency: cancel-in-progress: true jobs: - ubuntu-24-04: - name: ubuntu-24.04 + build: + name: ${{ matrix.name }} + strategy: + fail-fast: false + matrix: + include: + - name: ubuntu-24.04 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-ubuntu-24.04 + - name: ubuntu-22.04 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-ubuntu-22.04 + - name: debian-12 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-debian-12 + - name: rhel-8.10 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-rhel-8.10 + - name: rhel-9 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-rhel-9 + - name: rhel-10 + image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-rhel-10 uses: ./.github/workflows/build.yaml with: - name: ubuntu-24.04 - image: dgaliffiamd/rocprofiler-systems:ci-rocm-7.2-ubuntu-24.04 - cc: gcc - cxx: g++ - build-types: '["RELWITHDEBINFO"]' + name: ${{ matrix.name }} + image: ${{ matrix.image }} diff --git a/.github/workflows/rocprofiler-systems.yaml b/.github/workflows/rocprofiler-systems.yaml index f5b6c8a84e..696829fbb6 100644 --- a/.github/workflows/rocprofiler-systems.yaml +++ b/.github/workflows/rocprofiler-systems.yaml @@ -21,7 +21,7 @@ on: description: Which upstream CI entries to run type: choice options: [single, full] - default: single + default: full permissions: contents: read @@ -60,7 +60,9 @@ jobs: - name: Select matrix entries id: select env: - SCOPE: ${{ github.event.inputs.scope || 'single' }} + # 'single' remains available through workflow_dispatch for bisecting a + # failure down to one image without paying for the other seven. + SCOPE: ${{ github.event.inputs.scope || 'full' }} SHA: ${{ steps.resolve.outputs.sha }} run: | curl -fsSL -o matrix.json \ diff --git a/scripts/build-tpls.sh b/scripts/build-tpls.sh index 4466c3cb56..20e621f522 100755 --- a/scripts/build-tpls.sh +++ b/scripts/build-tpls.sh @@ -69,17 +69,50 @@ echo "Building third-party libraries into ${prefix}" echo " ${want_stamp}" echo " jobs: ${jobs}" +install_prereqs_apt() { + apt-get update -qq + apt-get install -y -qq --no-install-recommends \ + bzip2 ca-certificates curl git m4 make pkg-config \ + zlib1g-dev libzstd-dev libbz2-dev liblzma-dev +} + +install_prereqs_dnf() { + # libzstd-devel ships in CodeReady Builder, which is disabled by default and + # named powertools on RHEL 8 but crb from RHEL 9 on. Rather than detect the + # name by parsing repolist -- whose output differs between dnf4 and dnf5 -- + # try the plain install first, so images that already enable it, or that + # carry the package in a base repository, are unaffected. + local pkgs=( + bzip2 ca-certificates curl git m4 make pkgconfig + zlib-devel libzstd-devel bzip2-devel xz-devel + ) + + if dnf install -y "${pkgs[@]}"; then + return 0 + fi + + local repo + for repo in crb powertools; do + echo "retrying prerequisite install with --enablerepo=${repo}" + if dnf install -y "--enablerepo=${repo}" "${pkgs[@]}"; then + return 0 + fi + done + + return 1 +} + install_prereqs() { - if ! command -v apt-get >/dev/null 2>&1; then - echo "error: only apt-based images are supported today." >&2 + if command -v apt-get >/dev/null 2>&1; then + install_prereqs_apt + elif command -v dnf >/dev/null 2>&1; then + install_prereqs_dnf + else + echo "error: no supported package manager (apt-get or dnf) found." >&2 echo " Re-run with --skip-prereqs after installing the equivalents of:" >&2 echo " bzip2 curl git m4 make pkg-config zlib libzstd libbz2 liblzma (all -dev)" >&2 exit 1 fi - apt-get update -qq - apt-get install -y -qq --no-install-recommends \ - bzip2 ca-certificates curl git m4 make pkg-config \ - zlib1g-dev libzstd-dev libbz2-dev liblzma-dev } build_tbb() { From fc4db40c0f65d75435e6b38d250707437c9f63eb Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Wed, 12 Aug 2026 08:10:44 -0400 Subject: [PATCH 6/9] Fix GCC 14 build failures RHEL 10 (gcc 14.3.1) is the only image in the CI matrix new enough to reach these three diagnostics, each of which is fatal under DYNINST_WARNINGS_AS_ERRORS. Annotatable.h passed calloc its arguments transposed, tripping gcc 14's -Wcalloc-transposed-args. The allocation size is unchanged. indexed_symbols::erase discarded the result of std::remove, which gcc 14's libstdc++ marks [[nodiscard]]. This was a real defect rather than a diagnostic nuisance: std::remove shifts the survivors down and returns the new logical end, so ignoring it left each vector at its original length with a stale duplicate in the tail, and did not remove the symbol at all when it was the last element. Completed to the erase-remove idiom. gcc 14 compiles the buildTables() lambda to a 74560-byte frame, but DyninstWarnings.cmake populated the non-debug power-opcode-table override only for gcc 7 and clang, so gcc 14 fell through to the 20480 default. Added a gcc 14+ arm at 76800, the worst case already documented for this file. The Annotatable.h and indexed_symbols.hpp changes match upstream master. Co-authored-by: Cursor --- cmake/DyninstWarnings.cmake | 3 +++ common/h/Annotatable.h | 4 ++-- symtabAPI/src/indexed_symbols.hpp | 12 ++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/cmake/DyninstWarnings.cmake b/cmake/DyninstWarnings.cmake index 6e450f642f..51f0e31005 100644 --- a/cmake/DyninstWarnings.cmake +++ b/cmake/DyninstWarnings.cmake @@ -156,6 +156,9 @@ if(HAS_CPP_FLAG_Wframe_larger_than AND NOT DYNINST_DISABLE_DIAGNOSTIC_SUPPRESSIO set(debugMaxFrameSizeOverridePowerOpcodeTable 358400) if(${CMAKE_CXX_COMPILER_VERSION} MATCHES "^[7](\.|$)") set(nonDebugMaxFrameSizeOverridePowerOpcodeTable 38912) + elseif(${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL 14) + # gcc 14 emits a 74560-byte frame for the buildTables() lambda at -O2. + set(nonDebugMaxFrameSizeOverridePowerOpcodeTable 76800) endif() # most gcc's are under the default using -Og, but rhel's requires 30000 set(debugMaxFrameSizeOverrideFinalizeOperands 30000) diff --git a/common/h/Annotatable.h b/common/h/Annotatable.h index b5bc5cea14..ac1a149bf2 100644 --- a/common/h/Annotatable.h +++ b/common/h/Annotatable.h @@ -176,7 +176,7 @@ class COMMON_EXPORT AnnotatableDense if (annotations->data == NULL) { - annotations->data = (anno_list_t *) calloc(sizeof(anno_list_t), (size)); + annotations->data = (anno_list_t *) calloc(size, sizeof(anno_list_t)); annotations->max = size; for (unsigned i=0; idata[i] = NULL; @@ -228,7 +228,7 @@ class COMMON_EXPORT AnnotatableDense annotations = (aInfo *) malloc(sizeof(aInfo)); unsigned size = rhs.annotations->max; annotations->max = size; - annotations->data = (anno_list_t *)calloc(sizeof(anno_list_t), (size)); + annotations->data = (anno_list_t *)calloc(size, sizeof(anno_list_t)); memcpy(annotations->data, rhs.annotations->data, size * sizeof(anno_list_t)); } else { annotations = NULL; diff --git a/symtabAPI/src/indexed_symbols.hpp b/symtabAPI/src/indexed_symbols.hpp index 80b0c85540..f87e13d306 100644 --- a/symtabAPI/src/indexed_symbols.hpp +++ b/symtabAPI/src/indexed_symbols.hpp @@ -71,28 +71,32 @@ struct indexed_symbols { if (!by_offset.find(oa, s->getOffset())) { assert(!"by_offset.find(oa, s->getOffset())"); } - std::remove(oa->second.begin(), oa->second.end(), s); + auto it = std::remove(oa->second.begin(), oa->second.end(), s); + oa->second.erase(it, oa->second.end()); } { by_name_t::accessor ma; if (!by_mangled.find(ma, s->getMangledName())) { assert(!"by_mangled.find(ma, s->getMangledName())"); } - std::remove(ma->second.begin(), ma->second.end(), s); + auto it = std::remove(ma->second.begin(), ma->second.end(), s); + ma->second.erase(it, ma->second.end()); } { by_name_t::accessor pa; if (!by_pretty.find(pa, s->getPrettyName())) { assert(!"by_pretty.find(pa, s->getPrettyName())"); } - std::remove(pa->second.begin(), pa->second.end(), s); + auto it = std::remove(pa->second.begin(), pa->second.end(), s); + pa->second.erase(it, pa->second.end()); } { by_name_t::accessor ta; if (!by_typed.find(ta, s->getTypedName())) { assert(!"by_typed.find(ta, s->getTypedName())"); } - std::remove(ta->second.begin(), ta->second.end(), s); + auto it = std::remove(ta->second.begin(), ta->second.end(), s); + ta->second.erase(it, ta->second.end()); } } } From 27134d88b9a077a46021b936bc45f2e1a419a39d Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Wed, 12 Aug 2026 08:23:49 -0400 Subject: [PATCH 7/9] Fix RHEL 10 and 8.10 --- .github/workflows/rocprofiler-systems.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rocprofiler-systems.yaml b/.github/workflows/rocprofiler-systems.yaml index 696829fbb6..3b676cfc3b 100644 --- a/.github/workflows/rocprofiler-systems.yaml +++ b/.github/workflows/rocprofiler-systems.yaml @@ -213,7 +213,7 @@ jobs: APT_SYSTEM_DEPS: ${{ matrix.kind == 'system_deps' && matrix.system_deps_apt || '' }} run: | packages="${APT_COMPILER} ${APT_SYSTEM_DEPS}" - if [ -z "${packages// /}" ] && [ "${{ matrix.os_major }}" != "10" ]; then + if [ "${{ matrix.is_rhel }}" != "true" ] && [ -z "${packages// /}" ]; then echo "No extra packages needed for this entry" exit 0 fi @@ -226,6 +226,19 @@ jobs: return 1 } if [ "${{ matrix.is_rhel }}" = "true" ]; then + # RHEL 8 ships glibc 2.28, but the trace_processor_shell that the + # perfetto python package downloads on demand needs 2.29, so every + # perfetto assertion fails before it reads the trace. The tests read + # ROCPROFSYS_TRACE_PROC_SHELL for exactly this case (see + # tests/pytest/rocprofsys/validators.py); v47.0 is the build + # upstream pins for it. + if [ "${{ matrix.os_major }}" = "8" ]; then + mkdir -p /opt/trace_processor/bin + retry curl -fsSL -o /opt/trace_processor/bin/trace_processor_shell \ + https://commondatastorage.googleapis.com/perfetto-luci-artifacts/v47.0/linux-amd64/trace_processor_shell + chmod +x /opt/trace_processor/bin/trace_processor_shell + echo "ROCPROFSYS_TRACE_PROC_SHELL=/opt/trace_processor/bin/trace_processor_shell" >> "${GITHUB_ENV}" + fi # mpich lives in the crb repository on RHEL 10. if [ "${{ matrix.os_major }}" = "10" ]; then retry dnf install -y --enablerepo=crb mpich mpich-devel @@ -343,6 +356,19 @@ jobs: --name "${CDASH_NAME}" \ -B build + # trace_processor_shell is a server process the validation script talks to + # over HTTP, and upstream carries this same step because on RHEL it has + # been seen to outlive the run and hold the job open. + - name: Kill Perfetto + if: ${{ matrix.is_rhel && (success() || failure()) }} + continue-on-error: true + run: | + procs=$(pgrep trace_processor_shell || true) + if [ -n "${procs}" ]; then + # shellcheck disable=SC2086 + kill -9 ${procs} + fi + - name: CDash link if: always() run: | From 431b616423f3a3a4a1955508a4eac49f07719dc0 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Wed, 12 Aug 2026 12:20:24 -0400 Subject: [PATCH 8/9] More instrument --- .github/workflows/rocprofiler-systems.yaml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rocprofiler-systems.yaml b/.github/workflows/rocprofiler-systems.yaml index 3b676cfc3b..1c134dcf6e 100644 --- a/.github/workflows/rocprofiler-systems.yaml +++ b/.github/workflows/rocprofiler-systems.yaml @@ -292,7 +292,13 @@ jobs: # The trailing -L narrows the suite to the tests that actually exercise # Dyninst. binary_rewrite and runtime_instrument reach ctest as labels # because tests/pytest/conftest.py emits the parametrised `mode` of each - # test as a CTest label; -LE matches the upstream exclusion. + # test as a CTest label. `instrument` is the marker on + # TestRocprofilerSystemsInstrument in tests/pytest/test_binaries.py, which + # drives the rocprof-sys-instrument CLI itself rather than a mutatee + # (--simulate, --exe-only, --max-library-functions and the like). Both + # that marker and the file-wide rocprof_binary appear nowhere else in the + # suite, so naming it adds those cases and nothing besides. -LE matches + # the upstream exclusion. - name: Generate CI scripts timeout-minutes: 5 run: | @@ -319,7 +325,7 @@ jobs: ${{ matrix.cmake_flags }} \ -DROCPROFSYS_BUILD_NUMBER=${{ github.run_attempt }} \ -- \ - -L "binary_rewrite|runtime_instrument" \ + -L "binary_rewrite|runtime_instrument|instrument" \ -LE "network|gpu" - name: Configure From 07e66c7714fbf055683d61c8506db496071e46f2 Mon Sep 17 00:00:00 2001 From: Kian Cossettini Date: Thu, 13 Aug 2026 09:27:47 -0400 Subject: [PATCH 9/9] Remove smoke tests --- .github/workflows/build.yaml | 40 +--- .gitignore | 2 - scripts/smoke/CMakeLists.txt | 27 --- scripts/smoke/binary_rewrite.cpp | 55 ----- scripts/smoke/mutatee.c | 28 --- scripts/smoke/run-smoke.sh | 331 --------------------------- scripts/smoke/runtime_instrument.cpp | 56 ----- scripts/smoke/smoke_mutator.h | 71 ------ 8 files changed, 4 insertions(+), 606 deletions(-) delete mode 100644 scripts/smoke/CMakeLists.txt delete mode 100644 scripts/smoke/binary_rewrite.cpp delete mode 100644 scripts/smoke/mutatee.c delete mode 100755 scripts/smoke/run-smoke.sh delete mode 100644 scripts/smoke/runtime_instrument.cpp delete mode 100644 scripts/smoke/smoke_mutator.h diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index dc51cc8ae0..0f00f10d5e 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -1,7 +1,10 @@ name: Build Dyninst # Reusable builder. Builds Dyninst's third-party libraries from source (see -# scripts/build-tpls.sh for why), then builds, installs and smoke-tests Dyninst. +# scripts/build-tpls.sh for why), then builds and installs Dyninst. +# +# Functional coverage lives in the rocprofiler-systems workflow; this job proves +# only that the tree configures, compiles and installs. # # The TPLs are built in a separate job because actions/cache skips its save step # when a job fails: building them inline would mean a Dyninst build failure @@ -158,27 +161,6 @@ jobs: - name: Install run: cmake --install build - # Rewrites a binary with parseThat and instruments a live process with - # BPatch::processCreate. A link-only check would prove neither, and the - # runtime half is the only one that exercises ProcControlAPI and the - # injection of libdyninstAPI_RT. - # - # --retries applies only to a timeout, never to a failed assertion, so a - # genuine regression still fails on the first attempt. It is here because - # ProcControl startup was seen to hang rather than fail; the step timeout - # is the outer backstop if even the retries wedge. - - name: Smoke test - timeout-minutes: 10 - run: | - bash scripts/smoke/run-smoke.sh \ - --prefix "${INSTALL_PREFIX}" \ - --tpl-prefix "${TPL_PREFIX}" \ - --cc ${{ inputs.cc }} \ - --cxx ${{ inputs.cxx }} \ - --workdir "${GITHUB_WORKSPACE}/.smoke" \ - --timeout 120 \ - --retries 1 - - name: ccache stats if: always() run: ccache -s @@ -193,17 +175,3 @@ jobs: build/CMakeFiles/CMakeConfigureLog.yaml build/CMakeFiles/*.log if-no-files-found: ignore - - # The captured stdout of each stage is what distinguishes "instrumentation - # did not run" from "the mutatee itself failed". - # - # include-hidden-files is required because .smoke is a dot directory, and - # upload-artifact skips those by default without treating it as an error. - - name: Upload smoke test output - if: failure() - uses: actions/upload-artifact@v7 - with: - name: smoke-${{ inputs.name }}-${{ matrix.build-type }} - include-hidden-files: true - path: .smoke/*.txt - if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index 9fdb9772f1..7600603c8a 100644 --- a/.gitignore +++ b/.gitignore @@ -123,8 +123,6 @@ build*/ cmake-build-*/ # Staging prefix produced by scripts/build-tpls.sh .tpls/ -# Scratch directory produced by scripts/smoke/run-smoke.sh -.smoke/ .project .cproject .settings diff --git a/scripts/smoke/CMakeLists.txt b/scripts/smoke/CMakeLists.txt deleted file mode 100644 index b39f61f4da..0000000000 --- a/scripts/smoke/CMakeLists.txt +++ /dev/null @@ -1,27 +0,0 @@ -# Builds the smoke-test mutators against an installed Dyninst. -# -# This goes through find_package rather than a hand-written compiler line -# because the installed public headers include third-party headers of their own -# -- among them -- and the include directories that -# resolve those are carried by the exported targets. Listing them by hand means -# rediscovering each one as a CI failure, and on a machine that happens to have -# the library installed system-wide the omission does not show up at all. -# -# Going through the package config also puts the same consumer path -# rocprofiler-systems uses under test, so an install tree that is unusable from -# outside the build directory fails here rather than downstream. -# -# TBB_ROOT_DIR and ElfUtils_ROOT_DIR are read by the find modules the config -# ships; pass the same values the Dyninst build was given. - -cmake_minimum_required(VERSION 3.14) - -project(dyninst-smoke LANGUAGES CXX) - -find_package(Dyninst REQUIRED COMPONENTS dyninstAPI) - -foreach(_mutator binary_rewrite runtime_instrument) - add_executable(${_mutator} "${CMAKE_CURRENT_SOURCE_DIR}/${_mutator}.cpp") - target_compile_features(${_mutator} PRIVATE cxx_std_17) - target_link_libraries(${_mutator} PRIVATE Dyninst::dyninstAPI) -endforeach() diff --git a/scripts/smoke/binary_rewrite.cpp b/scripts/smoke/binary_rewrite.cpp deleted file mode 100644 index 50d6799a34..0000000000 --- a/scripts/smoke/binary_rewrite.cpp +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Binary-rewrite smoke test. - * - * Opens the mutatee with BPatch::openBinary, inserts a call to its - * dyninst_marker() at the entry of work(), and writes the result out. The - * script then runs that binary and greps for the marker. - * - * This exists because "the rewritten binary still produces the right answer" is - * not evidence that anything was instrumented: a rewriter that selects zero - * functions and writes back a working copy passes that check. Downstream - * rocprofiler-systems tests fail exactly that way, reporting a successful - * rewrite whose output carries no instrumentation, so the assertion here has to - * be the marker firing rather than the mutatee's own output. - * - * Usage: binary_rewrite - */ - -#include "BPatch.h" -#include "BPatch_binaryEdit.h" - -#include "smoke_mutator.h" - -#include - -int main(int argc, char* argv[]) -{ - if(argc != 3) - { - std::fprintf(stderr, "usage: %s \n", argv[0]); - return 2; - } - - const char* mutatee = argv[1]; - const char* output = argv[2]; - - BPatch bpatch; - - BPatch_binaryEdit* binary = bpatch.openBinary(mutatee); - if(binary == nullptr) - { - std::fprintf(stderr, "error: openBinary('%s') failed\n", mutatee); - return 1; - } - - if(!smoke::insert_marker(binary)) return 1; - - if(!binary->writeFile(output)) - { - std::fprintf(stderr, "error: writeFile('%s') failed\n", output); - return 1; - } - - std::fprintf(stderr, "wrote instrumented binary to %s\n", output); - return 0; -} diff --git a/scripts/smoke/mutatee.c b/scripts/smoke/mutatee.c deleted file mode 100644 index c9a48e2913..0000000000 --- a/scripts/smoke/mutatee.c +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Target program for the CI smoke tests. Shared by the binary-rewrite check and - * the runtime-instrumentation check. - * - * dyninst_marker() is deliberately never called. Its output appears only if a - * mutator inserted a call to it, which makes "did instrumentation actually - * execute" a grep rather than an inference from a tool's exit code. - */ - -#include - -__attribute__((noinline, used)) void dyninst_marker(void) -{ - printf("dyninst-marker-ran\n"); - fflush(stdout); -} - -__attribute__((noinline, used)) int work(int x) -{ - return x * 2; -} - -int main(void) -{ - printf("mutatee-ok %d\n", work(21)); - fflush(stdout); - return 0; -} diff --git a/scripts/smoke/run-smoke.sh b/scripts/smoke/run-smoke.sh deleted file mode 100755 index 25e311f7f0..0000000000 --- a/scripts/smoke/run-smoke.sh +++ /dev/null @@ -1,331 +0,0 @@ -#!/usr/bin/env bash -# -# Smoke-test an installed Dyninst against a trivial mutatee. -# -# Two checks, covering the two halves of Dyninst that a link-only test misses: -# -# binary rewrite parseThat writes out a new binary, and a mutator built -# against the install writes out another with a snippet of -# our own in it -# runtime instrument a mutator built against the install uses -# BPatch::processCreate to instrument a live process -# -# The runtime check is the only one that exercises ProcControlAPI and the -# injection of libdyninstAPI_RT, so it fails on a class of breakage that binary -# rewriting cannot see. -# -# Both checks assert that an inserted snippet ran rather than that the tools -# exited zero and the mutatee still works. A rewriter that selects no functions -# at all satisfies the weaker condition, and that is the shape of the binary -# rewriting failures seen downstream in rocprofiler-systems. -# -# Every Dyninst invocation is wrapped in a timeout. ProcControl process startup -# has been observed to hang rather than fail: the mutatee sits in ptrace_stop -# while the mutator's event thread waits on a futex that nothing will post, so -# without a bound the job stalls until the runner kills it and the log ends -# mid-step with no indication of which stage was to blame. -# -# Usage: -# run-smoke.sh --prefix DIR [--tpl-prefix DIR] [--cc CC] [--cxx CXX] -# [--workdir DIR] [--timeout SECONDS] [--retries N] -# -# --prefix Dyninst install prefix (must contain bin/parseThat and -# lib/libdyninstAPI_RT.so) -# --tpl-prefix third-party library prefix from build-tpls.sh, added to -# LD_LIBRARY_PATH so the install can resolve elfutils and TBB -# --timeout per-invocation limit in seconds (default 120) -# --retries extra attempts for the runtime check only, which is the one -# stage observed to hang nondeterministically (default 0) - -set -euo pipefail - -script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - -prefix="" -tpl_prefix="" -cc="${CC:-gcc}" -cxx="${CXX:-g++}" -workdir="" -timeout_secs=120 -retries=0 - -usage() { - sed -n '3,30p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' -} - -while [[ $# -gt 0 ]]; do - case "$1" in - --prefix) prefix="$2"; shift 2 ;; - --tpl-prefix) tpl_prefix="$2"; shift 2 ;; - --cc) cc="$2"; shift 2 ;; - --cxx) cxx="$2"; shift 2 ;; - --workdir) workdir="$2"; shift 2 ;; - --timeout) timeout_secs="$2"; shift 2 ;; - --retries) retries="$2"; shift 2 ;; - -h|--help) usage; exit 0 ;; - *) echo "error: unknown argument '$1'" >&2; usage >&2; exit 2 ;; - esac -done - -if [[ -z "${prefix}" ]]; then - echo "error: --prefix is required" >&2 - exit 2 -fi -prefix="$(cd "${prefix}" && pwd)" - -if [[ -z "${workdir}" ]]; then - workdir="$(mktemp -d)" - trap 'rm -rf "${workdir}"' EXIT -fi -mkdir -p "${workdir}" - -libdir="${prefix}/lib" -[[ -d "${libdir}" ]] || libdir="${prefix}/lib64" - -# Honoured by the ppc, aarch64 and FreeBSD backends. The x86_64 Linux backend -# ignores it and resolves the runtime library by search instead, which is what -# DYNINST_REWRITER_PATHS below is for. -export DYNINSTAPI_RT_LIB="${libdir}/libdyninstAPI_RT.so" - -# How PCProcess::setEnvPreload finds the library it LD_PRELOADs into the -# mutatee. On x86_64 Linux it calls BinaryEdit::getResolvedLibraryPath with the -# bare name "libdyninstAPI_RT.so" and takes the first hit from, in order, this -# variable, LD_LIBRARY_PATH, and the compiler's search directories. Naming -# ${libdir} here pins the runtime library to the install under test and lets the -# runtime check keep ${libdir} off LD_LIBRARY_PATH: without it the search either -# finds nothing, and process creation fails during bootstrap with no explanation -# unless DYNINST_DEBUG_STARTUP is set, or finds some other Dyninst that happens -# to be on the path and instruments the mutatee with the wrong runtime. -export DYNINST_REWRITER_PATHS="${libdir}" - -tpl_ld_path="${tpl_prefix:+${tpl_prefix}/elfutils/lib:${tpl_prefix}/tbb/lib}" - -# Two paths, because the runtime check runs without ${libdir} on it: the mutator -# reaches its own libraries through the RPATH cmake links it with, and leaving -# the directory off means nothing on the loader's search path can stand in for -# the install being tested. The third-party entries have to stay, because -# libdyninstAPI.so's own dependencies resolve through LD_LIBRARY_PATH rather -# than through the mutator's RUNPATH, which is not transitive. -# -# The inherited value is filtered the same way rather than trusted, since a -# developer shell or a container image can point it at a different Dyninst. -strip_dyninst_dirs() { - local result="" entry - local -a entries=() - IFS=':' read -r -a entries <<< "$1" - for entry in "${entries[@]}"; do - [[ -z "${entry}" ]] && continue - if compgen -G "${entry}/libdyninstAPI*.so*" > /dev/null; then - echo "note: dropping Dyninst library directory '${entry}' from the runtime search path" >&2 - continue - fi - result="${result:+${result}:}${entry}" - done - printf '%s' "${result}" -} - -runtime_ld_path="${tpl_ld_path}" -if [[ -n "${LD_LIBRARY_PATH:-}" ]]; then - runtime_ld_path="${runtime_ld_path:+${runtime_ld_path}:}${LD_LIBRARY_PATH}" -fi -runtime_ld_path="$(strip_dyninst_dirs "${runtime_ld_path}")" - -if [[ -n "${runtime_ld_path}" ]]; then - runtime_env=(env "LD_LIBRARY_PATH=${runtime_ld_path}") -else - # An empty LD_LIBRARY_PATH is not the same as an unset one: the loader reads - # the empty entry as the current directory. - runtime_env=(env -u LD_LIBRARY_PATH) -fi - -export LD_LIBRARY_PATH="${libdir}${runtime_ld_path:+:${runtime_ld_path}}" - -if [[ ! -f "${DYNINSTAPI_RT_LIB}" ]]; then - echo "error: ${DYNINSTAPI_RT_LIB} not found" >&2 - exit 1 -fi - -step() { printf '\n=== %s ===\n' "$1"; } - -# SIGKILL rather than the default SIGTERM: a mutator wedged inside ProcControl -# is not reliably servicing signals, and a timeout that itself hangs is worse -# than no timeout at all. GNU timeout then reports 137 instead of 124. -is_timeout() { [[ "$1" -eq 124 || "$1" -eq 137 ]]; } - -# Runs a command under the timeout and tees its output, leaving the status in -# run_rc. errexit is lifted around the pipeline so the caller can tell a hang -# apart from a non-zero exit instead of the script dying on the spot. -run_logged() { - local logfile="$1" - shift - set +e - timeout -s KILL "${timeout_secs}" "$@" 2>&1 | tee "${logfile}" - run_rc="${PIPESTATUS[0]}" - set -e -} - -report_failure() { - if is_timeout "$2"; then - echo "error: $1 did not finish within ${timeout_secs}s" >&2 - echo " this is a hang, not a failed assertion; see ${3}" >&2 - else - echo "error: $1 exited $2" >&2 - fi -} - -# --------------------------------------------------------------------------- -# Baseline -# --------------------------------------------------------------------------- - -step "build the mutatee" -# -O0 keeps work() and dyninst_marker() as distinct, findable functions. -"${cc}" -g -O0 -o "${workdir}/mutatee" "${script_dir}/mutatee.c" - -step "run the mutatee uninstrumented" -run_logged "${workdir}/baseline.txt" "${workdir}/mutatee" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "the uninstrumented mutatee" "${run_rc}" "${workdir}/baseline.txt" - exit 1 -fi -grep -q 'mutatee-ok 42' "${workdir}/baseline.txt" -# Nothing has been instrumented yet, so the marker must be silent. If this -# fires, the mutatee itself calls dyninst_marker() and the runtime check below -# would pass without Dyninst doing anything. -if grep -q 'dyninst-marker-ran' "${workdir}/baseline.txt"; then - echo "error: marker ran without instrumentation" >&2 - exit 1 -fi - -# --------------------------------------------------------------------------- -# Mutators -# --------------------------------------------------------------------------- - -step "build the mutators" -# Without this, a prefix missing its headers falls through to whatever Dyninst -# happens to sit in a default include path, and the failure arrives as a wall of -# template errors from the wrong version. -if [[ ! -f "${prefix}/include/BPatch.h" ]]; then - echo "error: ${prefix}/include/BPatch.h not found; is --prefix an install tree?" >&2 - exit 1 -fi - -# See scripts/smoke/CMakeLists.txt for why this is a CMake build and not a -# compiler invocation. The mutators end up with a build-tree RPATH covering the -# install's libraries, which is what lets the runtime check below drop them from -# LD_LIBRARY_PATH. -mutator_build="${workdir}/mutators" -cmake_args=( - -S "${script_dir}" - -B "${mutator_build}" - -DCMAKE_BUILD_TYPE=RelWithDebInfo - -DCMAKE_CXX_COMPILER="${cxx}" - -DCMAKE_PREFIX_PATH="${prefix}" -) -if [[ -n "${tpl_prefix}" ]]; then - cmake_args+=( - -DTBB_ROOT_DIR="${tpl_prefix}/tbb" - -DElfUtils_ROOT_DIR="${tpl_prefix}/elfutils" - ) -fi - -run_logged "${workdir}/cmake.txt" cmake "${cmake_args[@]}" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "configuring the mutators" "${run_rc}" "${workdir}/cmake.txt" - exit 1 -fi - -run_logged "${workdir}/cmake-build.txt" cmake --build "${mutator_build}" --parallel -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "building the mutators" "${run_rc}" "${workdir}/cmake-build.txt" - exit 1 -fi - -# --------------------------------------------------------------------------- -# Binary rewrite -# --------------------------------------------------------------------------- - -step "rewrite the mutatee with parseThat" -# -i 1 instruments function entries, so this covers instrumentation rather than -# only parse-and-write-back. -run_logged "${workdir}/rewrite.txt" \ - "${prefix}/bin/parseThat" -i 1 \ - --binary-edit="${workdir}/mutatee.rewritten" \ - "${workdir}/mutatee" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "parseThat" "${run_rc}" "${workdir}/rewrite.txt" - exit 1 -fi - -step "run the rewritten mutatee" -test -x "${workdir}/mutatee.rewritten" -run_logged "${workdir}/rewritten.txt" "${workdir}/mutatee.rewritten" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "the rewritten mutatee" "${run_rc}" "${workdir}/rewritten.txt" - exit 1 -fi -grep -q 'mutatee-ok 42' "${workdir}/rewritten.txt" - -step "rewrite the mutatee with a snippet of our own" -# parseThat above shows the rewriter produces a working binary, which it also -# does when it instruments nothing at all. This inserts a call we can observe so -# the run below can tell those two outcomes apart. -run_logged "${workdir}/rewrite-snippet.txt" \ - "${mutator_build}/binary_rewrite" "${workdir}/mutatee" "${workdir}/mutatee.snippet" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "binary rewriting" "${run_rc}" "${workdir}/rewrite-snippet.txt" - exit 1 -fi - -step "run the rewritten mutatee carrying the snippet" -test -x "${workdir}/mutatee.snippet" -run_logged "${workdir}/snippet.txt" "${workdir}/mutatee.snippet" -if [[ "${run_rc}" -ne 0 ]]; then - report_failure "the rewritten mutatee" "${run_rc}" "${workdir}/snippet.txt" - exit 1 -fi -# The mutatee never calls dyninst_marker() itself, so this line can only come -# from the snippet written into the entry of work(). -grep -q 'dyninst-marker-ran' "${workdir}/snippet.txt" -grep -q 'mutatee-ok 42' "${workdir}/snippet.txt" - -# --------------------------------------------------------------------------- -# Runtime instrumentation -# --------------------------------------------------------------------------- - -step "instrument a live process with BPatch::processCreate" -# Logged unconditionally, so that a hang here can be read against the search -# path that produced it without having to reconstruct what was filtered. -echo "runtime LD_LIBRARY_PATH: ${runtime_ld_path:-(unset)}" - -# Only a hang is retried, and only when --retries asks for it. A mutator that -# runs to completion with the wrong output is a regression and must stay red. -attempt=0 -while true; do - run_logged "${workdir}/runtime.txt" \ - "${runtime_env[@]}" "${mutator_build}/runtime_instrument" "${workdir}/mutatee" - - if [[ "${run_rc}" -eq 0 ]]; then - break - fi - - if is_timeout "${run_rc}" && [[ "${attempt}" -lt "${retries}" ]]; then - attempt=$((attempt + 1)) - echo "warning: runtime instrumentation hung, retrying (${attempt}/${retries})" >&2 - continue - fi - - report_failure "runtime instrumentation" "${run_rc}" "${workdir}/runtime.txt" - if is_timeout "${run_rc}"; then - echo " re-run with DYNINST_DEBUG_STARTUP=1 DYNINST_DEBUG_PROCCONTROL=1" >&2 - echo " to see which runtime library was injected and how far" >&2 - echo " bootstrap got before it stopped" >&2 - fi - exit 1 -done - -# The mutatee never calls dyninst_marker() itself, so this line can only come -# from the snippet inserted at the entry of work(). -grep -q 'dyninst-marker-ran' "${workdir}/runtime.txt" -grep -q 'mutatee-ok 42' "${workdir}/runtime.txt" - -printf '\nall smoke checks passed\n' diff --git a/scripts/smoke/runtime_instrument.cpp b/scripts/smoke/runtime_instrument.cpp deleted file mode 100644 index 75ca332d63..0000000000 --- a/scripts/smoke/runtime_instrument.cpp +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Runtime-instrumentation smoke test. - * - * Creates a process with BPatch::processCreate, inserts a call to the mutatee's - * dyninst_marker() at the entry of work(), and runs it to completion. This is - * the half of Dyninst that binary rewriting never touches: ProcControlAPI - * process control, injection of libdyninstAPI_RT into a live process, and code - * patching of a running mutatee. - * - * Mirrors the runtime_instrument mode of the rocprofiler-systems test suite, so - * a break here shows up before the much slower downstream job runs. - * - * Usage: runtime_instrument - */ - -#include "BPatch.h" -#include "BPatch_process.h" - -#include "smoke_mutator.h" - -#include - -int main(int argc, char* argv[]) -{ - if(argc != 2) - { - std::fprintf(stderr, "usage: %s \n", argv[0]); - return 2; - } - - const char* mutatee = argv[1]; - const char* mutatee_argv[] = { mutatee, nullptr }; - - BPatch bpatch; - BPatch_process* proc = bpatch.processCreate(mutatee, mutatee_argv); - if(proc == nullptr) - { - std::fprintf(stderr, "error: processCreate('%s') failed\n", mutatee); - return 1; - } - - if(!smoke::insert_marker(proc)) return 1; - - if(!proc->continueExecution()) - { - std::fprintf(stderr, "error: continueExecution() failed\n"); - return 1; - } - - while(!proc->isTerminated()) - bpatch.waitForStatusChange(); - - int exit_code = proc->getExitCode(); - std::fprintf(stderr, "mutatee terminated, exit code %d\n", exit_code); - return exit_code; -} diff --git a/scripts/smoke/smoke_mutator.h b/scripts/smoke/smoke_mutator.h deleted file mode 100644 index fe69c3c784..0000000000 --- a/scripts/smoke/smoke_mutator.h +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Shared by the two smoke-test mutators. - * - * Both checks insert the same snippet into the same mutatee and differ only in - * how the address space is obtained and what is done with it afterwards, so the - * instrumentation itself lives here rather than being written twice. - */ - -#pragma once - -#include "BPatch_addressSpace.h" -#include "BPatch_function.h" -#include "BPatch_image.h" -#include "BPatch_point.h" -#include "BPatch_snippet.h" - -#include - -namespace smoke -{ - -inline BPatch_function* find_only(BPatch_image* image, const char* name) -{ - BPatch_Vector funcs; - image->findFunction(name, funcs); - - // Anything other than exactly one match means the mutatee was built - // differently than this test assumes, not that Dyninst is broken. - if(funcs.size() != 1) - { - std::fprintf(stderr, "error: expected exactly one '%s', found %lu\n", name, - static_cast(funcs.size())); - return nullptr; - } - return funcs[0]; -} - -// Inserts a call to the mutatee's dyninst_marker() at the entry of work(). -inline bool insert_marker(BPatch_addressSpace* aspace) -{ - BPatch_image* image = aspace->getImage(); - if(image == nullptr) - { - std::fprintf(stderr, "error: getImage() returned null\n"); - return false; - } - - BPatch_function* work = find_only(image, "work"); - BPatch_function* marker = find_only(image, "dyninst_marker"); - if(work == nullptr || marker == nullptr) return false; - - BPatch_Vector* entry = work->findPoint(BPatch_locEntry); - if(entry == nullptr || entry->empty()) - { - std::fprintf(stderr, "error: no entry point found for work()\n"); - return false; - } - - BPatch_Vector no_args; - BPatch_funcCallExpr call_marker(*marker, no_args); - - if(aspace->insertSnippet(call_marker, *entry) == nullptr) - { - std::fprintf(stderr, "error: insertSnippet() failed\n"); - return false; - } - - return true; -} - -} // namespace smoke