Open
Conversation
f95942e to
7793490
Compare
d09c054 to
9acc94a
Compare
itrowbri
reviewed
Jan 26, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Outdated
Show resolved
Hide resolved
itrowbri
reviewed
Jan 26, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Outdated
Show resolved
Hide resolved
itrowbri
reviewed
Jan 26, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Show resolved
Hide resolved
bwelton
reviewed
Jan 28, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Outdated
Show resolved
Hide resolved
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Show resolved
Hide resolved
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_session.cpp
Outdated
Show resolved
Hide resolved
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Outdated
Show resolved
Hide resolved
9acc94a to
cc1e939
Compare
bwelton
reviewed
Feb 5, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/common/wait_for_atomic.hpp
Show resolved
Hide resolved
bwelton
approved these changes
Feb 5, 2026
itrowbri
approved these changes
Feb 10, 2026
1 task
c309d6a to
3d22639
Compare
Contributor
Author
|
changes requested in #3265 |
c2c8dfb to
7141d1c
Compare
itrowbri
reviewed
Mar 5, 2026
projects/rocprofiler-sdk/tests/bin/attachment-test/attachment_test.cpp
Outdated
Show resolved
Hide resolved
itrowbri
reviewed
Mar 5, 2026
projects/rocprofiler-sdk/tests/bin/attachment-test/attachment_test.cpp
Outdated
Show resolved
Hide resolved
itrowbri
reviewed
Mar 5, 2026
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_runner.cpp
Show resolved
Hide resolved
itrowbri
approved these changes
Mar 9, 2026
7369719 to
c103e22
Compare
c103e22 to
a5ae6f7
Compare
Contributor
Author
|
@ROCm/clr-reviewers I've temporarily integrated a commit from #4169 to get PSDB build passing so I can inspect test results. Review is not required and this commit will be removed from this branch before merging. |
a5ae6f7 to
1120621
Compare
- Reworks ptrace calls to originate from the same thread per target pid, as required
…l agnostic for signal test
- Refactored ptrace_runner to hide the class used
Co-authored-by: itrowbri <Ian.Trowbridge@amd.com>
1120621 to
f2b4c22
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Crashes were observed when extraneous signals were sent to an attached target. Bug introduced in #1653
Prior to #1653, all ptrace operations were handled from a singleton thread in rocattach.cpp. This was changed in favor of re-entrancy support to allow the library to attach to multiple processes simultaneously. This particular detail was lost in the transition.
Technical Details
ptrace session are tracked per thread ID, so all ptrace operations targetting a given PID must originate from the same thread.
This reworks ptrace operations into a new class called PTraceRunner which aims to be as similar to the original ptrace call as possible.
PTraceRunner is a simple single worker thread that handles ptrace operations serially with mutex and atomic mailbox. Each PTraceSession owns a single PTraceRunner that is shared between the main thread and the signal handler thread via shared_ptr.
Aside: ptrace is defined as
long ptrace (int op, ...). For consistency's sake, we choose to define it asuint64_t ptrace(__ptrace_operation op, void* addr, void* data). Some changes are made in PTraceSession to enforce the stricter typing.JIRA ID
Test Plan
Ran parallel-attach test and resized window during test to send extraneous signals (SIGWINCH).
@itrowbri has developed a test which provides a signal mid-attach to ensure this behavior in the future
Test Result
Logging indicated the extraneous signal was handled and sent to the target program.
Submission Checklist