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

Deprecate kernel-vmmap-via-page-tables in favor of kernel-vmmap #1415

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

gsingh93
Copy link
Member

@gsingh93 gsingh93 commented Dec 5, 2022

Users should be able to completely disable vmmap when debugging kernels, so that issues with gdb-pt-dump or monitor info mem don't become blockers, and for performance reasons.

For aarch64, monitor info mem isn't supported, so we gracefully hand that case and warn the user.

Fixes #1403.

@codecov-commenter
Copy link

Codecov Report

Merging #1415 (0cf84ed) into dev (94d1ebb) will decrease coverage by 0.01%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##              dev    #1415      +/-   ##
==========================================
- Coverage   58.15%   58.13%   -0.02%     
==========================================
  Files         155      155              
  Lines       19601    19611      +10     
  Branches     1844     1848       +4     
==========================================
+ Hits        11398    11400       +2     
- Misses       7643     7651       +8     
  Partials      560      560              
Impacted Files Coverage Δ
pwndbg/gdblib/vmmap.py 48.30% <14.28%> (-0.90%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

# these cases in a `finally` block instead of an `except` block.
if monitor_info_mem is None or "unknown command" in monitor_info_mem:
# TODO: Find out which other architectures don't support this command
if pwndbg.gdblib.arch.name == "aarch64":
Copy link
Member

Choose a reason for hiding this comment

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

arch.name vs arch.current?

Copy link
Member

Choose a reason for hiding this comment

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

wondering what's the difference; oh i guess current returns some struct

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they should be the same thing:

# TODO: `current` is the old name for the arch name, and it's now an
# alias for `name`. It's used throughout the codebase, do we want to
# migrate these uses to `name`?

@disconnect3d disconnect3d merged commit 947024e into pwndbg:dev Dec 5, 2022
@gsingh93 gsingh93 deleted the kernel-vmmap branch December 24, 2022 10:01
@gsingh93 gsingh93 added the kernel label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Don't allow use of monitor info mem on qemu-system-aarch64
3 participants