Skip to content

[rocprofiler-compute] [Bugfix:] Handle native tool path detection gracefully#3501

Merged
vedithal-amd merged 2 commits intodevelopfrom
users/vedithal/rocprofiler-compute-standalonebinary-bugfix
Feb 25, 2026
Merged

[rocprofiler-compute] [Bugfix:] Handle native tool path detection gracefully#3501
vedithal-amd merged 2 commits intodevelopfrom
users/vedithal/rocprofiler-compute-standalonebinary-bugfix

Conversation

@vedithal-amd
Copy link
Contributor

Motivation

Handle native tool path detection gracefully

Technical Details

This fixes the issue where the standalone binary does not run if at least three parent directories are not available in cwd

With this change, we will be gracefully handling such issues instead of crashing.

JIRA ID

Test Plan

Create standalone binary with this PR and try to run it from /root and ensure no crash

Test Result

Tests pass

Submission Checklist

This fixes the issue where the standalone binary does not run if at
least three parent directories are not available in cwd

With this change, we will be gracefully handling such issues instead of
crashing.
Copilot AI review requested due to automatic review settings February 24, 2026 17:37
@vedithal-amd vedithal-amd requested a review from a team as a code owner February 24, 2026 17:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes a bug where the rocprofiler-compute standalone binary would crash when run from a directory with fewer than three parent directories (e.g., running from /root or /tmp). The issue occurred during native tool path detection, which attempted to access script_path.parents[2] without checking if that many parent directories exist.

Changes:

  • Added length check before accessing script_path.parents[2] to prevent IndexError
  • Falls back to Path() (current directory) when fewer than 3 parent directories exist
  • Gracefully handles path detection failure through existing try-except error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@xuchen-amd xuchen-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally - works.

Do we have test coverage for this?

@vedithal-amd
Copy link
Contributor Author

Do we have test coverage for this?

We don't have any unit tests for run_profiling function to test this behavior.
I wanted to keep this PR strictly scoped to graceful handling, adding tests for run_profiling function can be done in a different PR.

@JeniferC99
Copy link
Collaborator

JeniferC99 commented Feb 24, 2026

@vedithal-amd vedithal-amd merged commit 8239d64 into develop Feb 25, 2026
35 of 40 checks passed
@vedithal-amd vedithal-amd deleted the users/vedithal/rocprofiler-compute-standalonebinary-bugfix branch February 25, 2026 00:12
@JeniferC99 JeniferC99 restored the users/vedithal/rocprofiler-compute-standalonebinary-bugfix branch February 25, 2026 19:07
@JeniferC99 JeniferC99 deleted the users/vedithal/rocprofiler-compute-standalonebinary-bugfix branch February 25, 2026 19:08
radhaksri pushed a commit that referenced this pull request Feb 26, 2026
…cefully (#3501)

* Bugfix: Handle native tol path detection gracefully

This fixes the issue where the standalone binary does not run if at
least three parent directories are not available in cwd

With this change, we will be gracefully handling such issues instead of
crashing.

* Address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants