Skip to content

gdbclient: Read thread uid, gid and status from procfs#6263

Open
SELESTER11 wants to merge 1 commit into
rizinorg:devfrom
SELESTER11:fix-gdbr-thread-info
Open

gdbclient: Read thread uid, gid and status from procfs#6263
SELESTER11 wants to merge 1 commit into
rizinorg:devfrom
SELESTER11:fix-gdbr-thread-info

Conversation

@SELESTER11
Copy link
Copy Markdown

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository.
  • I made sure to follow the project's coding style.
  • I've documented every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

gdbr_threads_list was hardcoding uid, gid and status to -1 and RZ_DBG_PROC_STOP. I noticed gdbr_parse_processes_xml already does this for processes by reading /proc/<pid>/status over vFile, so I did the same for threads using /proc/<pid>/task/<tid>/status. Falls back to defaults if the remote doesn't support vFile or isn't Linux.

Test plan

This change requires a Linux gdbserver with a multi-threaded binary and vFile support to verify. Open to suggestions on how to add a test for this.

Closing issues

N/A

@github-actions github-actions Bot added the GDB label Apr 20, 2026
@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from 8558a82 to 6e20bea Compare April 20, 2026 19:44
@SELESTER11
Copy link
Copy Markdown
Author

@wargio Could you approve the CI workflows when you get a chance? Keeping the PR as draft until CI passes.

Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 0% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.39%. Comparing base (519cef5) to head (30176a0).
⚠️ Report is 31 commits behind head on dev.

Files with missing lines Patch % Lines
subprojects/rzgdb/src/gdbclient/core.c 0.00% 27 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
subprojects/rzgdb/src/gdbclient/core.c 26.47% <0.00%> (-0.54%) ⬇️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 519cef5...30176a0. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch 2 times, most recently from b192765 to 11a851d Compare April 21, 2026 03:56
Copy link
Copy Markdown
Contributor

@notxvilka notxvilka left a comment

Choose a reason for hiding this comment

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

Good idea but more work is needed

Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
@notxvilka notxvilka requested a review from wargio April 26, 2026 13:56
@notxvilka notxvilka added the waiting-for-author Used to mark PRs where more work is needed label Apr 26, 2026
@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from 11a851d to 613efc9 Compare April 27, 2026 16:26
@vkaramchanda
Copy link
Copy Markdown

@notxvilka I've addressed all the feedback. Could you re-review and mark this ready for review when you get a chance?

Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from 613efc9 to dafb52e Compare May 2, 2026 23:21
@SELESTER11 SELESTER11 marked this pull request as ready for review May 5, 2026 03:19
@wargio wargio requested a review from notxvilka May 6, 2026 02:58
@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from dafb52e to 756b537 Compare May 6, 2026 05:27
@SELESTER11
Copy link
Copy Markdown
Author

The char buffer caused a -Wpointer-sign error when passed to gdbr_read_file(ut8 *buf). Added (ut8 *) cast at the call site, matching how gdbr_parse_processes_xml does it in xml.c.

Comment thread subprojects/rzgdb/src/gdbclient/core.c Outdated
}

/**
* @brief Read thread uid, gid and status from /proc/<pid>/task/<tid>/status via vFile.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use \ instead of @

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, switched to \brief.

@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from 756b537 to 30176a0 Compare May 6, 2026 15:33
@SELESTER11
Copy link
Copy Markdown
Author

@notxvilka All three of your feedback points have been addressed. Could you re-review when you get a chance?

Copy link
Copy Markdown
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Please add a test

@SELESTER11
Copy link
Copy Markdown
Author

Please add a test

@Rot127 Happy to add a test. This change requires a Linux gdbserver with a multi-threaded binary and vFile support to verify. Could you point me to existing test infrastructure I should follow, or suggest a pattern for testing gdbserver-dependent functionality?

@Rot127
Copy link
Copy Markdown
Member

Rot127 commented May 20, 2026

There are already tests which use a gdb server in test/db/archos/linux-x64/dbg_gdbserver.
Please check those for examples.

For the binary: You can check the existing rizin-testbins repo for one.
But I think it is easier to just vibe code a tiny program, compile it and upload the binary + source code to the rizin-testbins repo.

@SELESTER11
Copy link
Copy Markdown
Author

There are already tests which use a gdb server in test/db/archos/linux-x64/dbg_gdbserver. Please check those for examples.

For the binary: You can check the existing rizin-testbins repo for one. But I think it is easier to just vibe code a tiny program, compile it and upload the binary + source code to the rizin-testbins repo.

Done, added test in test/db/archos/linux-x64/dbg_gdbserver. Binary + source in rizinorg/rizin-testbins#287.

@Rot127
Copy link
Copy Markdown
Member

Rot127 commented May 26, 2026

Merged it, please rebase

@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from ee10bc9 to 80ce64f Compare May 26, 2026 19:54
@SELESTER11 SELESTER11 requested a review from Rot127 May 26, 2026 19:55
Copy link
Copy Markdown
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Please fix the tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can the output of the test be more descriptive?
Just a * and - has no meaning.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed -- changed EOF2 to EOF (parser only accepts EOF) and updated the expected output to show the status character for each thread (* t and - t), which is more descriptive.

@SELESTER11 SELESTER11 force-pushed the fix-gdbr-thread-info branch from 80ce64f to e897506 Compare June 3, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GDB rz-test waiting-for-author Used to mark PRs where more work is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants