Repository navigation
[FEA]: Debugger pretty-printers: cuda::mr::shared_resource - #10229
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesAdds GDB and LLDB pretty-printers for shared_resource pretty-printers
Assessment against linked issues
Merge Risk: ⚪ Minimal · up to This PR adds localized GDB and LLDB pretty-printers for cuda::mr::shared_resource with accompanying tests and registration. No actionable merge-blocking risk remains beyond normal checks and review. Comment |
|
/ok to test 6a96e8a |
This comment has been minimized.
This comment has been minimized.
|
Note for reviewers: the two red Both fail before any compilation, at environment creation: The available This PR adds Python pretty-printer modules and a debugging lit test; it touches no headers RAPIDS compiles against. The rest of the matrix is green (100%/71, 85 successful checks). |
miscco
left a comment
miscco
left a comment
There was a problem hiding this comment.
Thanks a lot for improving the debugging experience 🎉
Jacobfaib
left a comment
Jacobfaib
left a comment
There was a problem hiding this comment.
Thank you for the patch! Just a few minor fixups required
| inspect_shared(second_reference); | ||
| inspect_alias(aliased_resource); | ||
| inspect_moved_into(moved_resource); | ||
| inspect_moved_from(donor_resource); |
There was a problem hiding this comment.
Can you include a scenario where a shared resource is inspected before and after it is copied? I.e.
auto resource = ...;
inspect(resource);
auto copy = resource;
inspect(resource);
inspect(copy);and the same for move? I realize you have similar scenarios already, but we want to make sure that the debuggers update the information for the same object before and after. LLDB pretty printers for example cache the output.
There was a problem hiding this comment.
Added, for both operations. inspect_before_copy and inspect_after_copy print the same variable, copy_source, on either side of copy_target = copy_source, so its use count has to read 1 then 2; the second stop also prints copy_target. The move pair does the same for move_source, which goes from owning to empty, and prints move_target at the second stop.
The old shared, moved_into, and moved_from cases fold into these pairs, since they pinned the same states without showing that the debuggers refresh them.
| return _SHARED_RESOURCE_PATTERN.fullmatch(type_name) is not None | ||
|
|
||
|
|
||
| def shared_resource_summary( |
There was a problem hiding this comment.
This should be a proper pretty printer, not just a summary. Its output should be modeled like std::shared_ptr.
This is probably based on the summary printer for the regular resource, but that needed a summary because gdb/lldb don't play nice when sub-printers produce newlines (so e.g. in buffer, we show the memory resource but need to do so without newlines).
There was a problem hiding this comment.
Done. LLDB now registers a synthetic provider next to the summary, and the GDB printer grew a matching children(). Both expose the owned resource as a resource child, so a handle can be expanded to reach the resource it owns, while the summary keeps carrying the use count and the address the way std::shared_ptr keeps strong=/weak= in its summary and the pointer in its child.
The child is the address rather than the resource itself, deliberately: yielding the payload object made GDB print the whole resource inline, which put cuda::mr::__4::, static __available_flags, and the resource private members into the golden file. That is exactly what public_type_name() exists to keep out, and it would break the test on an ABI namespace bump. One extra expansion step gets you the same drill-down without pinning any of it.
The summary is registered with --expand so frame variable shows the child, and update() returns False so LLDB re-reads after a copy or a move instead of serving what it cached.
There was a problem hiding this comment.
One correction to what I wrote above: the follow-up commit takes the address out of the summary as well, so it now appears once, in the resource child, instead of in both places. The summary carries use_count=N for an owning handle, which is closer to how std::shared_ptr splits the two.
The two formats you asked for are unaffected: an empty handle still reads use_count=0, resource=nullptr, and a resource with no load address still reads use_count=N, resource=<invalid address>.
| if address == lldb.LLDB_INVALID_ADDRESS: | ||
| return f"{type_name} use_count={use_count}" |
There was a problem hiding this comment.
We should print the address nonetheless, something like <invalid address>.
There was a problem hiding this comment.
Now prints resource=<invalid address>. Both printers carry a comment noting the branch guards an unreadable frame rather than any state this scenario reaches, since the harness always builds at -O0 -g and a control block reached through a live pointer always has an address.
| if control_block.GetValueAsUnsigned(0) == 0: | ||
| return f"{type_name} empty" |
There was a problem hiding this comment.
We should still print the information in the same format as normal. So type_name={type_name} use_count=0, resource=nullptr
There was a problem hiding this comment.
Both printers now report an empty handle as cuda::mr::shared_resource<...> use_count=0, resource=nullptr.
I read "the same format as normal" as keeping the bare type name that the non-empty branch already prints, so there is no literal type_name= label in front of it. Say the word if you meant the label and I will add it to both branches in both printers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ff11c9d8-8d21-4bff-83d1-a1863aed1692
📒 Files selected for processing (6)
libcudacxx/share/libcudacxx/gdb/shared_resource.pylibcudacxx/share/libcudacxx/lldb/shared_resource.pylibcudacxx/test/debugging/shared_resource/CMakeLists.txtlibcudacxx/test/debugging/shared_resource/gdb.expectedlibcudacxx/test/debugging/shared_resource/lldb.expectedlibcudacxx/test/debugging/shared_resource/source.cu
🚧 Files skipped from review as they are similar to previous changes (3)
- libcudacxx/test/debugging/shared_resource/CMakeLists.txt
- libcudacxx/test/debugging/shared_resource/lldb.expected
- libcudacxx/share/libcudacxx/gdb/shared_resource.py
| return False | ||
|
|
||
| def num_children(self) -> int: | ||
| return 1 if self.resource.IsValid() else 0 |
There was a problem hiding this comment.
Nit: can reuse has_children() here, or rework has_children() to reuse num_children()
| cuda::mr::shared_resource<cuda::mr::legacy_managed_memory_resource> use_count=1, resource=<address> = { | ||
| resource = <address> |
There was a problem hiding this comment.
We don't need to print resource=... in the summary now that we have it in the brackets
| (const shared_resource_type) cuda::mr::shared_resource<cuda::mr::legacy_managed_memory_resource> use_count=1, resource=<address> { | ||
| resource = <address> |
There was a problem hiding this comment.
Ditto printing resource twice
|
/ok to test 0a2020e |
This comment has been minimized.
This comment has been minimized.
|
All four review points are addressed, across
Ready for |
Jacobfaib
left a comment
Jacobfaib
left a comment
There was a problem hiding this comment.
LGTM, thanks for the contribution!
|
/ok to test 15300fd |
This comment has been minimized.
This comment has been minimized.
Head branch was pushed to by a user without write access
|
@ecgang can you rebase your branch over latest |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@Jacobfaib rebased onto latest The conflicts git reported were just the printer registries and the
Goldens regenerated under gdb 15.1 and lldb 18.1.3, then re-verified against the untouched Needs a |
|
@ecgang sorry I totally missed that this never merged. Can you rebase again and I will make sure it gets in this time. |
Summarize shared ownership instead of the raw control block: the printers report the public type, the reference count read from the control block's atomic, and the address of the wrapped resource, so two handles to the same resource are distinguishable at a glance. A moved-from handle renders as empty. Registers the new module in both entry points and adds a pretty-printer scenario covering the unique, shared, aliased, moved-into, and moved-from states. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Make the LLDB formatter a real pretty printer: a synthetic provider now sits next to the summary, and the GDB printer grew a matching children(). Both expose the owned resource as a `resource` child, the way std::shared_ptr exposes its pointer, so a handle can be expanded to reach the resource it owns. The child is the address rather than the resource itself, which keeps the ABI inline namespace and the resource's private members out of the output. LLDB's provider reports no caching, so a copy or a move is picked up on the next stop instead of being served from what LLDB already computed. Report an empty handle in the normal format, `use_count=0, resource=nullptr`, rather than as `empty`, and print `<invalid address>` when the owned resource has no load address, so the fields are always in the same places. Cover copying and moving on the same variable across two stops: the copy pair prints `copy_source` on either side of the copy, so its use count has to go 1 -> 2, and the move pair prints `move_source` going from owning to empty. Both stops also print the new handle. These replace the old shared, moved-into, and moved-from cases, which pinned the same states without showing that the debuggers refresh them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e summary The summary printed the owned resource's address next to the use count, and the `resource` child then printed it again. Keep it in the child alone, so a handle reads `use_count=N` and expands to the resource, the way std::shared_ptr keeps strong=/weak= in its summary and the pointer in its child. The two states with no child keep reporting the address in the summary, since there is nothing else to carry it: an empty handle stays `use_count=0, resource=nullptr`, and a resource with no readable address stays `resource=<invalid address>`. That makes the summary depend on agreeing with the child about which state a handle is in, so the LLDB formatters now read one signal, _resource_pointer(), rather than asking through two different APIs. The GDB printer already evaluates the same expression in both places. Have num_children() ask has_children() rather than repeat the check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebasing over main lands this scenario on a harness and a printer API that both changed underneath it. The runner no longer takes a frame index, and it evaluates each expression in the breakpoint's own frame rather than the caller's, so every `--case` drops its `1` and names the inspected parameter. Two of the breakpoints inspected a second variable that only main could see; each one splits into a breakpoint of its own, which is also how every other scenario is written. The scenario picks up the KEEP_FOR_DEBUGGER macro that replaced the keep_for_debugger template. The type helpers the GDB printer borrowed from memory_resource now live in cccl_common, and memory_resource no longer exports them, so the printer imported a name that is gone. Both printers take them from cccl_common instead, which also gets them reference handling they now need: the value under inspection arrives as `const T&` from the new frame convention. Two things that only the LLDB side hits. Stripping the reference has to happen before GetNonSyntheticValue(), or the control block reads as unavailable and every handle summarizes as None. And the reference count is a cuda::std::atomic, which main now formats with a synthetic `value` child, hiding the raw __a layout; reading it through GetNonSyntheticValue() keeps this formatter off another formatter's output. Goldens regenerated under gdb 15.1 and lldb 18.1.3 and verified against the untouched memory_resource scenario. The GDB golden is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
No worries — rebased onto
Three conflicts, all additive registry lists (the Re-ran both scenarios under gdb 15.1 and lldb 18.1.3 against the committed goldens with |
|
/ok to test 3381ae2 |
🥳 CI Workflow Results🟩 Finished in 56m 41s: Pass: 100%/134 | Total: 1d 06h | Max: 56m 33s | Hits: 99%/387525See results here. |
…10229) * [libcu++] Add debugger pretty-printers for cuda::mr::shared_resource
Closes #10101
Description
Adds GDB and LLDB pretty-printers for
cuda::mr::shared_resource, alongside the existingmemory_resourceprinters.Each prints the public type name and the reference count read from the control block's
atomic. LLDB registers a synthetic child provider next to the summary and the GDB printer
yields children, so a handle expands to a
resourcechild holding the address of the ownedresource: two handles to the same resource are distinguishable at a glance, and the address
appears once rather than in both places. This follows the way
std::shared_ptrsplits thecounts into the summary and the pointer into a child. The child is the address rather than
the resource object, which keeps the ABI inline namespace and the resource's private members
out of the goldens.
An empty (moved-from) handle keeps the fields in the same places,
use_count=0, resource=nullptr, and a resource with no load address readsresource=<invalid address>.Both debugger entry points register the new module. A
libcudacxx_add_pretty_printer_test()scenario covers a unique handle, a handle reached through a type alias, and copy and move
inspected on the same variable before and after the operation, so the goldens show the
debuggers refreshing one object's state rather than serving what they cached.
Verification: both debugger tests were run for real (the dev machine has no GPU) in an
aarch64 Linux container — nvcc 13.0.88, gdb 15.1, lldb 18.1.3 — invoking
run_pretty_printer_test.pywith the argument shapelibcudacxx_add_pretty_printer_test()generates, with
libcuda.so.1stubbed (the binaries reach onlycuInitandcuDeviceGetCountthroughcuGetProcAddress; no compared byte originates from the driver,and the harness normalizes addresses). Both
shared_resourcegoldens matched observed outputbyte-for-byte with golden updating turned off, and the existing
memory_resourcescenariopasses unchanged under the same setup as a control.
The one path not exercised locally is the CMake registration itself (a full configure needs
lit and a GPU-runner environment), so a CI run is what proves that wiring.
Rebased onto
9ef513d9c5on 2026-08-27, 144 commits on from the previous base. Git reportedthree conflicts, all additive registry lists: the
gdb/andlldb/__init__.pyprintertables and the
add_subdirectorylist intest/debugging/CMakeLists.txt. The rest of therange was checked by hand rather than assumed, because the things that move under a printer
do not conflict: the control-block layout this reads (
__payload,__ref_count,cuda::std::atomic's__a.__a_value), everycccl_commonhelper the two modules import,and
run_pretty_printer_test.py's--casearity are all unchanged.make_shared_resourcegained
[[nodiscard]]in that range; every call insource.cualready binds its result.Both scenarios were re-run under the container described above against the committed
goldens, with golden updating off and
memory_resourceas an untouched control: all fourcombinations pass byte-for-byte, so no
.expectedfile needed regenerating. The full CImatrix has not run since the rebase.
Checklist
docs/cccl/development/debugger_setup.rstenumerates the printer directories, not individual types, so no doc edit is needed.)This contribution was developed with AI assistance via terminalhire. The author has reviewed the change and takes responsibility for its content.