Skip to content

docs(hip): say that HRR capture ignores its output variable in privileged programs - #12903

Closed
andraghetti wants to merge 3 commits into
developfrom
users/andraghetti/hrr-docs-secure-exec
Closed

andraghetti wants to merge 3 commits into
developfrom
users/andraghetti/hrr-docs-secure-exec

Conversation

@andraghetti

@andraghetti andraghetti commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

#12252 makes HRR capture ignore HIP_HRR_CAPTURE_OUTPUT in privileged processes. This adds that rule to the HIP how-to on record and replay. It is a separate pull request because this file needs a HIP docs review, and #12252 can then merge on the HRR review alone.

Technical Details

  • One paragraph in projects/hip/docs/how-to/hip_record_replay.rst, after the note on what an archive contains.
  • It says that on Linux a set-user-ID, set-group-ID or file-capability program ignores the variable. Such a program records nothing and prints one line on standard error.
  • Merge it after fix(hrr): ignore the capture output variable in privileged processes #12252, because the behaviour it describes comes with that change.

Issue Tracking

JIRA ID: AIRUNTIME-2828

Test Plan

Documentation only. The docs build in CI renders the page.

Test Result

Pending CI.

Submission Checklist

… programs

The HRR how-to gains one paragraph: on Linux, a program the kernel
starts in secure-execution mode ignores HIP_HRR_CAPTURE_OUTPUT, records
nothing and says why on standard error. The behaviour comes with
#12252.
Copilot AI balanced review requested due to automatic review settings October 6, 2026 11:41
@andraghetti
andraghetti requested review from a team as code owners October 6, 2026 11:41
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: hip labels Oct 6, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🚫 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?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The runtime prerequisite #12252 remains unmerged, so the documented behavior requires merge-order confirmation before approval.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Documents the Linux secure-execution restriction introduced by #12252 in HIP’s record-and-replay guide.

Changes:

  • Adds a paragraph explaining that secure-execution programs ignore HIP_HRR_CAPTURE_OUTPUT, disable capture, and report ignored requests.
File Description
projects/​hip/​docs/​how-to/​hip_record_replay.rst Documents capture restrictions for secure-execution programs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@andraghetti
andraghetti force-pushed the users/andraghetti/hrr-docs-secure-exec branch from 52ba608 to 1f1ebea Compare October 6, 2026 18:34
@andraghetti
andraghetti requested a balanced review from Copilot October 6, 2026 18:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The documentation matches the linked implementation’s described behavior and addresses the prior feedback.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The documentation matches the prerequisite change, which has merged, and resolves the prior feedback.

Review effort: Balanced
Findings: None

@mangupta

mangupta commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (d79a9fbd6753) — no new findings at or above the posting threshold.

@danielbolsson danielbolsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Read the new paragraph against hip_capture.cpp (hrr_secure_exec via getauxval(AT_SECURE), stderr line only when capture was requested) and hip_capture_metadata.cpp; the wording matches the code.

@andraghetti
andraghetti requested a review from mangupta October 15, 2026 08:52
@andraghetti

Copy link
Copy Markdown
Contributor Author

Closing this: the same rule is already in the HRR docs since #12252, in the HIP_HRR_CAPTURE_OUTPUT row of projects/hrr/README.md and in the Enable Flag section of projects/hrr/DESIGN.md. The HIP how-to page links to that README for the full flag list.

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

Labels

documentation Improvements or additions to documentation organization: ROCm project: hip

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants