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

GDB: Improve tools to debug vm structures #1396

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

franciscozdo
Copy link
Collaborator

@franciscozdo franciscozdo commented Jun 20, 2023

In this PR I move all commands that operate on VM structures to be subcommands of vm command and create new ones

Commands added in this PR
  • vm map -- (replace kdump vm_map) and interpret optional argument - pid of process we want to see the vm map
  • vm pmap_kernel -- (replace kdump pmap)
  • vm pmap_user -- dump pmap of specified proc (for current if not specified)
  • vm segment -- see info about specified segment in current proc vm_map
  • vm segment_proc -- see info about specified segment in vm_map of proc specified by pid
  • vm freepages -- (replace kdump freepages)
  • vm physseg -- (replace kdump physseg)

@franciscozdo franciscozdo added the WiP not ready for code review label Jun 20, 2023
@franciscozdo franciscozdo mentioned this pull request Jul 7, 2023
sys/debug/utils.py Outdated Show resolved Hide resolved
sys/debug/virtmem.py Outdated Show resolved Hide resolved
sys/debug/virtmem.py Outdated Show resolved Hide resolved
sys/debug/vm_map.py Outdated Show resolved Hide resolved
sys/debug/vm_map.py Outdated Show resolved Hide resolved
sys/debug/vm_map.py Outdated Show resolved Hide resolved
sys/debug/virtmem.py Outdated Show resolved Hide resolved
sys/debug/vm_map.py Outdated Show resolved Hide resolved
sys/debug/utils.py Outdated Show resolved Hide resolved
Copy link
Owner

@cahirwpz cahirwpz left a comment

Choose a reason for hiding this comment

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

Some remarks left. What do you think about employing argparse to parse arguments in __call__ method. It'd be nice if help user-defined showed all subcommands, so the user does not have to consult the source code to know how to use a command.

if arch in gdb.architecture_names():
return arch
print('Current architecture is not supported')
raise KeyError
Copy link
Owner

Choose a reason for hiding this comment

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

Should not it be NotImplementedError ?

def __init__(self, pmap):
self._pmap = pmap

def print(self):
Copy link
Owner

Choose a reason for hiding this comment

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

If you don't plan to add more methods to the class in this PR I suggest you convert it to a function. Right now the class looks like an overkill.

class PageTableAArch64():
def __init__(self, pmap):
self._pmap = pmap
print("Page table not implemented for AArch64")
Copy link
Owner

Choose a reason for hiding this comment

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

raise NotImplementedError ?



def _segment_info(proc, segment):
MAX_SEGMENT_ID = 4096
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't that be a global variable?

@property
def pages(self):
size = self.end - self.start
return size // 4096
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe define global variable PAGESIZE ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WiP not ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants