fix(rocprofiler-sdk): calculate load bias via candidate search over all mappings by adjordje-amd · Pull Request #9831 · ROCm/rocm-systems

adjordje-amd · 2026-08-07T12:54:45Z

Motivation

rocprofiler-sdk-rocattach could fail to resolve symbols (e.g. rocprofiler_register_detach) in a target process, breaking attach/detach:

[rocprofiler-sdk-rocattach] First target mapping for <path> has file offset 0x<offset>, but the first PT_LOAD segment starts at file page 0x0
[rocprofiler-sdk-rocattach] Failed to resolve <lib>::rocprofiler_register_detach from target pid <pid> ELF mappings
[rocprofiler-sdk-rocattach] call_function failed to find target symbol address

calculate_load_bias() assumed the lowest-address mapping of the target library always corresponds to its lowest-p_vaddr PT_LOAD segment. This breaks whenever the kernel/loader doesn't produce a 1:1 address-ordered mapping-to-segment correspondence, causing every downstream symbol lookup in that object to fail.

Technical Details

symbol_lookup.cpp, calculate_load_bias(): replaced the single-first-mapping assumption with a search over every (mapping, PT_LOAD segment) pair. A candidate bias is derived per pair and kept only if every file-backed PT_LOAD segment is self-consistently mapped under that bias; a unique surviving candidate is required. This is a strict superset of the old check — anything it accepted, this still accepts.

Issue Tracking

JIRA ID: AIPROFSDK-1023

Test Plan

Rebuilt rocprofiler-sdk-rocattach-shared-library against current develop.
Ran existing tests/rocattach/rocattach-symbol-lookup-test (normal/GNU-hash/SysV-hash/shifted-offset/ambiguous-a/ambiguous-b fixtures): PASSED, no regression.

Test Result

Build clean, existing rocattach test suite passes.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

therock-pr-bot · 2026-08-07T12:55:44Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like `test_<name>.py` / `test_<name>.cpp` (or `<name>_test.`). Current:* code file(s) changed: `projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/symbol_lookup.cpp`; no test file found
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

mcao59

Thanks for opening the PR. It seems that no existing test triggered a failure that would reflect this issue. Is there a plan to add regression test in rocprofiler-systems or in this PR?

AlexandarDjordjevic · 2026-08-17T19:22:32Z

Thanks for opening the PR. It seems that no existing test triggered a failure that would reflect this issue. Is there a plan to add regression test in rocprofiler-systems or in this PR?

There is a PR for attach feature validation on the rocprofiler-systems side, but we need to merge this one first

…pings calculate_load_bias() assumed the lowest-address mapping of a target library always corresponds to its lowest-p_vaddr PT_LOAD segment. When that assumption doesn't hold (loader/kernel doesn't produce a 1:1 address-ordered mapping-to-segment correspondence), the strict check failed and every downstream symbol lookup in that library failed. Replace it with a search over every (mapping, PT_LOAD segment) pair, keeping only candidate biases where all file-backed PT_LOAD segments are self-consistently mapped, and require a unique result.

…ad-bias

mcao59

Approving here but please make sure that rocprofiler-systems will have tests for this use case as regression test

…ad-bias

adjordje-amd · 2026-09-21T19:54:36Z

Approving here but please make sure that rocprofiler-systems will have tests for this use case as regression test

rocprofiler-systems regression test #5414

…ad-bias

meserve-amd

Minor comment wording update. Looks good.

…symbol_lookup.cpp Co-authored-by: Mark Meserve <mark.meserve@amd.com>

systems-assistant · 2026-10-07T16:50:49Z

TheRock Submodule Bump Activity

Newest first

2026-10-07 — Included in TheRock via ROCm/TheRock#8809.

…in containerized environment (#12094) ## Motivation - `rocprof-sys-attach` computes the injected tool library path (`librocprof-sys-dl.so`) from the **attacher's** own filesystem view (`/proc/self/exe`) and forwards it into the target process unconditionally via `ROCP_TOOL_LIBRARIES`. - When the target is running in a different mount namespace (for example, a container) and that path doesn't exist there, `rocprofiler-sdk`'s registration code does a raw `dlopen()` on it and calls `ROCP_FATAL` on failure. This leads to the **the target process being aborted**, not the attach tool. ## Technical Details - Added `rocprofsys::common::path::check_target_visibility()` to validates an absolute library path against the target's mount-namespace view (`/proc/<pid>/root/<relative-path>`) before `rocprof-sys-attach` hands it to the target. Distinguishes confirmed absence (`ENOENT`/`ENOTDIR` → fail closed) from an indeterminate check (any other `stat()` error, e.g. restricted procfs access → fail open). - **NOTE**: A non-absolute override (bare SONAME) is left untouched, since that's meant to be resolved by the target's own dynamic linker search path. - Deliberately uses raw `stat()`/`errno` rather than `std::filesystem`. - `std::filesystem::is_regular_file`'s `error_code` is not a reliable found-vs-missing signal on this toolchain (verified: GCC 13/libstdc++ sets `ENOENT` via the `error_code` even for a plain missing file). This also matches the existing style already used. - `rocprof-sys-attach.cpp`: `setup_tool_library_env()` now returns the resolved absolute path; - `verify_tool_library_visible_to_target()` runs the check before `rocattach_attach()` is called, failing cleanly with a diagnostic instead of proceeding into a target-side abort. - Documented the container-attach behavior and the mount-namespace contract for `ROCPROF_ATTACH_TOOL_LIBRARY` in `docs/how-to/attaching-to-running-process.rst`. **IMPORTANT NOTE**: This PR depends on PR #9831 being merged in order to be able to successfully attach and calculate offsets on the targeted container environment. ## Issue Tracking JIRA ID: AIPROFSYST-712 JIRA ID: AIPROFSYST-713 ## Test Plan - New regression tests in `test_path.cpp`. - Manual end-to-end validation against real containerized targets (Docker, a GPU-kernel-looping HIP workload), covering: - Tool library invisible in the container's namespace → clean failure, target process survives untouched. - Tool library made visible via an explicit `ROCPROF_ATTACH_TOOL_LIBRARY` pointing at an independently installed (different-version) → attach succeeds without any file copying at attach time, satisfying the "no manual file copying, even across ROCm version differences" requirement. - Multiple attach/detach/reattach cycles against real container targets (including multiple different `rocprofiler-systems`/ROCm versions) resulting in every cycle producing correct output (`rocpd.db` / Perfetto trace / metadata / functions JSON). - Perfetto files and rocPD databases are validated against the actual workload data for the correct traces. ## Test Result - `rocprof-sys-unit-tests`: all 92 `PathTest` cases pass, including the 4 new `CheckTargetVisibility_*` cases. - Whole-project build compiles cleanly, no regression is introduced. - Manual container attach/detach/reattach validation: multiple clean cycles, correct output produced every time. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

…3666) ## Motivation `rocattach` can fail to calculate an ELF load bias when multiple adjacent `PT_LOAD` segments are represented by one `/proc/<pid>/maps` entry. This occurs with TheRock pip wheels after `patchelf` adds adjacent RW segments that Linux may merge into a single mapping. The failure prevents resolution of `rocprofiler_register_attach` and `rocprofiler_register_detach`, breaking tools built on rocattach. The fix must also preserve support introduced in #9831, where the lowest-address mapping does not correspond to the first `PT_LOAD` segment. ## Technical Details - Translate each `PT_LOAD` file page through any mapping that contains it, rather than requiring the mapping to start at the segment address. - Validate the linear relationship between mapping addresses and file offsets. - Continue searching all mapping/segment pairs without relying on mapping order. - Require every file-backed segment to agree on one unique load bias. - Retain checked arithmetic and fail closed for invalid or ambiguous layouts. ## Issue Tracking ROCM-32961 ## Test Plan - Add deterministic coverage for: - adjacent segments sharing one mapping - lower-address, nonzero-offset shadow mappings - both layouts occurring together - segments contained inside mappings that start earlier - incorrect offsets, range boundaries, and duplicate load biases ## Test Result - `tests.integration.execute.rocattach-symbol-lookup-test-execute`: passed ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

adjordje-amd requested review from a team as code owners August 7, 2026 12:54

github-actions Bot added the project: rocprofiler-sdk label Aug 7, 2026

systems-assistant Bot added the organization: ROCm label Aug 7, 2026

mcao59 reviewed Aug 11, 2026

View reviewed changes

mcao59 reviewed Aug 20, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/symbol_lookup.cpp Resolved

adjordje-amd force-pushed the users/adjordje-amd/fix-rocattach-stale-load-bias branch from e067b8f to 12be477 Compare August 24, 2026 21:08

adjordje-amd requested a review from mcao59 September 3, 2026 20:30

adjordje-amd added 2 commits September 14, 2026 14:30

Merge branch 'develop' into users/adjordje-amd/fix-rocattach-stale-lo…

a875a5f

…ad-bias

Merge branch 'develop' into users/adjordje-amd/fix-rocattach-stale-lo…

4784608

…ad-bias

mcao59 approved these changes Sep 21, 2026

View reviewed changes

Merge branch 'develop' into users/adjordje-amd/fix-rocattach-stale-lo…

68619bf

…ad-bias

mcao59 changed the title ~~fix(rocattach): calculate load bias via candidate search over all mappings~~ fix(rocprofiler-sdk): calculate load bias via candidate search over all mappings Sep 21, 2026

marantic-amd mentioned this pull request Sep 23, 2026

feature(rocprofiler-systems): Support attaching to a process running in containerized environment #12094

Merged

1 task done

adjordje-amd added 2 commits September 24, 2026 10:36

Merge branch 'develop' into users/adjordje-amd/fix-rocattach-stale-lo…

14adf4f

…ad-bias

Merge branch 'develop' into users/adjordje-amd/fix-rocattach-stale-lo…

906adaf

…ad-bias

meserve-amd approved these changes Sep 29, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/symbol_lookup.cpp Resolved

adjordje-amd and others added 2 commits September 30, 2026 15:48

Update projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/…

aef3268

…symbol_lookup.cpp Co-authored-by: Mark Meserve <mark.meserve@amd.com>

style(rocprofiler-sdk): fix comment indentation in calculate_load_bias

6658565

adjordje-amd merged commit 8677978 into develop Oct 5, 2026
275 of 281 checks passed

adjordje-amd deleted the users/adjordje-amd/fix-rocattach-stale-load-bias branch October 5, 2026 19:17

bgopesh mentioned this pull request Oct 14, 2026

docs(rocprofiler-sdk): add 1.4.2 CHANGELOG entries for ROCm 10.2.0 #13452

Merged

1 task done

mcao59 mentioned this pull request Oct 16, 2026

fix(rocprofiler-sdk): handle merged PT_LOAD mappings in rocattach #13666

Merged

1 task done

Navigation Menu

Repository navigation

fix(rocprofiler-sdk): calculate load bias via candidate search over all mappings - #9831

fix(rocprofiler-sdk): calculate load bias via candidate search over all mappings#9831
adjordje-amd merged 8 commits into
developfrom
users/adjordje-amd/fix-rocattach-stale-load-bias

adjordje-amd commented Aug 7, 2026

Uh oh!

therock-pr-bot Bot commented Aug 7, 2026

Uh oh!

mcao59 left a comment

Uh oh!

AlexandarDjordjevic commented Aug 17, 2026

Uh oh!

Uh oh!

mcao59 left a comment

Uh oh!

adjordje-amd commented Sep 21, 2026

Uh oh!

meserve-amd left a comment

Uh oh!

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

adjordje-amd commented Aug 7, 2026

Motivation

Technical Details

Issue Tracking

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Aug 7, 2026

✅ All Policy Checks Passed

Uh oh!

mcao59 left a comment

Choose a reason for hiding this comment

Uh oh!

AlexandarDjordjevic commented Aug 17, 2026

Uh oh!

Uh oh!

mcao59 left a comment

Choose a reason for hiding this comment

Uh oh!

adjordje-amd commented Sep 21, 2026

Uh oh!

meserve-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 7, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants