Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LLDB: Extend the dump_page methods #4420

Merged
merged 3 commits into from
Apr 27, 2021

Conversation

eightbitraptor
Copy link
Contributor

I frequently use dump_page in lldb to visualise heap pages, but the process of seeing the heap page containing a VALUE is annoying. First we have to heap_page obj to get the page and then call dump_page on the returned page address.

This PR adds dump_page_rvalue that combines both steap, dumping the page containing the VALUE passed as an argument - and also highlights that slots position in the page.

try:
flidx = "%3d" % freelist.index(obj_addr)
except ValueError:
flidx = ' '
Copy link
Member

Choose a reason for hiding this comment

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

It looks like objects that are not T_NONE will get an empty { } in the output for flidx. I think this is some confusing output for objects that are not T_NONE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that was the intended behaviour. It allows you to easily visualise which slots are part of the freelist and which ones aren't. Like this:

bits: [     ] T_NONE     [401]: {  2} Addr: 0x1007ebed0 (flags: 0x0)
bits: [     ] T_NONE     [402]: {  1} Addr: 0x1007ebef8 (flags: 0x0)
bits: [     ] T_NONE     [403]: {  0} Addr: 0x1007ebf20 (flags: 0x0)
bits: [LM   ] T_CLASS    [404]: {   } Addr: 0x1007ebf48 (flags: 0x100001062)
bits: [LM   ] T_PAYLOAD  [405]: {   } Addr: 0x1007ebf70 (flags: 0x3077)

It's easy to see where slots have been assigned values and where the freelist pointer (and list) is going. Although I suspect we could make it more obvious in the output that { } was the freelist index? How about:

bits: [     ] T_NONE     idx: [401] freelist_idx: {  2} Addr: 0x1007ebed0 (flags: 0x0)
bits: [     ] T_NONE     idx: [402] freelist_idx: {  1} Addr: 0x1007ebef8 (flags: 0x0)
bits: [     ] T_NONE     idx: [403] freelist_idx: {  0} Addr: 0x1007ebf20 (flags: 0x0)
bits: [LM   ] T_CLASS    idx: [404] freelist_idx: {   } Addr: 0x1007ebf48 (flags: 0x100001062)
bits: [LM   ] T_PAYLOAD  idx: [405] freelist_idx: {   } Addr: 0x1007ebf70 (flags: 0x3077)

the empty { } mirrors the behaviour of rp - it shows an empty [ ] section when no bits are set

bits: [     ]
T_NONE: (RBasic) *$35 = (flags = 0x0000000000000000, klass = 0x00000001007ebea8)

Copy link
Member

Choose a reason for hiding this comment

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

I like your second one 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I'll patch that and push it up.

Comment on lines 572 to 588
dump_bits(target, result, page, obj_addr, end= " ")
flags = obj.GetChildMemberWithName('flags').GetValueAsUnsigned()
flType = flags & RUBY_T_MASK

flidx = ' '
if flType == RUBY_T_NONE:
try:
flidx = "%3d" % freelist.index(obj_addr)
except ValueError:
flidx = ' '

result_str = "%s [%3d]: {%s} Addr: %0#x (flags: %0#x)" % (rb_type(flags, ruby_type_map), page_index, flidx, obj_addr, flags)

if highlight == obj_addr:
result_str = ' '.join([result_str, "<<<<<"])

print(result_str, file=result)
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if this shared logic with rp, that way the output would be more consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean so that we display the same output here and for rp? If so then I think this is a different use case - we only ever want to display a single dense line of info here so that we can see all of the slots on a page in a compact arrangement.

rp does a lot more stuff to display the body of the structs, almost all of which we're not using in this context.

rather than having to do this in a two step process:

1. heap_page obj
2. dump_page $2 (or whatever lldb variable heap_page set)

we can now just

dump_page_rvalue obj
@peterzhu2118 peterzhu2118 merged commit 1c1c915 into ruby:master Apr 27, 2021
@peterzhu2118 peterzhu2118 deleted the mvh-lldb-dump-page-extend branch April 27, 2021 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants