Skip to content

Commit

Permalink
Revert "[lldb] Add --all option to "memory region""
Browse files Browse the repository at this point in the history
This reverts commit 8e648f1
due to test failures on Windows:
https://lab.llvm.org/buildbot/#/builders/83/builds/19094
  • Loading branch information
DavidSpickett committed May 18, 2022
1 parent 1584b2c commit 3e928c4
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 182 deletions.
194 changes: 50 additions & 144 deletions lldb/source/Commands/CommandObjectMemory.cpp
Expand Up @@ -1641,117 +1641,19 @@ class CommandObjectMemoryHistory : public CommandObjectParsed {
// CommandObjectMemoryRegion
#pragma mark CommandObjectMemoryRegion

#define LLDB_OPTIONS_memory_region
#include "CommandOptions.inc"

class CommandObjectMemoryRegion : public CommandObjectParsed {
public:
class OptionGroupMemoryRegion : public OptionGroup {
public:
OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}

~OptionGroupMemoryRegion() override = default;

llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
return llvm::makeArrayRef(g_memory_region_options);
}

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
ExecutionContext *execution_context) override {
Status status;
const int short_option = g_memory_region_options[option_idx].short_option;

switch (short_option) {
case 'a':
m_all.SetCurrentValue(true);
m_all.SetOptionWasSet();
break;
default:
llvm_unreachable("Unimplemented option");
}

return status;
}

void OptionParsingStarting(ExecutionContext *execution_context) override {
m_all.Clear();
}

OptionValueBoolean m_all;
};

CommandObjectMemoryRegion(CommandInterpreter &interpreter)
: CommandObjectParsed(interpreter, "memory region",
"Get information on the memory region containing "
"an address in the current target process.",
"memory region <address-expression> (or --all)",
"memory region ADDR",
eCommandRequiresProcess | eCommandTryTargetAPILock |
eCommandProcessMustBeLaunched) {
// Address in option set 1.
m_arguments.push_back(CommandArgumentEntry{CommandArgumentData(
eArgTypeAddressOrExpression, eArgRepeatPlain, LLDB_OPT_SET_1)});
// "--all" will go in option set 2.
m_option_group.Append(&m_memory_region_options);
m_option_group.Finalize();
}
eCommandProcessMustBeLaunched) {}

~CommandObjectMemoryRegion() override = default;

Options *GetOptions() override { return &m_option_group; }

protected:
void DumpRegion(CommandReturnObject &result, Target &target,
const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
lldb::addr_t resolve_addr = load_addr;
if (m_memory_region_options.m_all)
resolve_addr = range_info.GetRange().GetRangeBase();

lldb_private::Address addr;
ConstString section_name;
if (target.ResolveLoadAddress(resolve_addr, addr)) {
SectionSP section_sp(addr.GetSection());
if (section_sp) {
// Got the top most section, not the deepest section
while (section_sp->GetParent())
section_sp = section_sp->GetParent();
section_name = section_sp->GetName();
}
}

ConstString name = range_info.GetName();
result.AppendMessageWithFormatv(
"[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
range_info.GetRange().GetRangeBase(),
range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
name, section_name ? " " : "", section_name);
MemoryRegionInfo::OptionalBool memory_tagged = range_info.GetMemoryTagged();
if (memory_tagged == MemoryRegionInfo::OptionalBool::eYes)
result.AppendMessage("memory tagging: enabled");

const llvm::Optional<std::vector<addr_t>> &dirty_page_list =
range_info.GetDirtyPageList();
if (dirty_page_list.hasValue()) {
const size_t page_count = dirty_page_list.getValue().size();
result.AppendMessageWithFormat(
"Modified memory (dirty) page list provided, %zu entries.\n",
page_count);
if (page_count > 0) {
bool print_comma = false;
result.AppendMessageWithFormat("Dirty pages: ");
for (size_t i = 0; i < page_count; i++) {
if (print_comma)
result.AppendMessageWithFormat(", ");
else
print_comma = true;
result.AppendMessageWithFormat("0x%" PRIx64,
dirty_page_list.getValue()[i]);
}
result.AppendMessageWithFormat(".\n");
}
}
}

bool DoExecute(Args &command, CommandReturnObject &result) override {
ProcessSP process_sp = m_exe_ctx.GetProcessSP();
if (!process_sp) {
Expand All @@ -1768,13 +1670,6 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
const lldb::ABISP &abi = process_sp->GetABI();

if (argc == 1) {
if (m_memory_region_options.m_all) {
result.AppendError(
"The \"--all\" option cannot be used when an address "
"argument is given");
return false;
}

auto load_addr_str = command[0].ref();
// Non-address bits in this will be handled later by GetMemoryRegion
load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
Expand All @@ -1788,53 +1683,67 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
// When we're repeating the command, the previous end address is
// used for load_addr. If that was 0xF...F then we must have
// reached the end of memory.
(argc == 0 && !m_memory_region_options.m_all &&
load_addr == LLDB_INVALID_ADDRESS) ||
(argc == 0 && load_addr == LLDB_INVALID_ADDRESS) ||
// If the target has non-address bits (tags, limited virtual
// address size, etc.), the end of mappable memory will be lower
// than that. So if we find any non-address bit set, we must be
// at the end of the mappable range.
(abi && (abi->FixAnyAddress(load_addr) != load_addr))) {
result.AppendErrorWithFormat(
"'%s' takes one argument or \"--all\" option:\nUsage: %s\n",
m_cmd_name.c_str(), m_cmd_syntax.c_str());
result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
m_cmd_name.c_str(), m_cmd_syntax.c_str());
return false;
}

lldb_private::MemoryRegionInfos region_list;
if (m_memory_region_options.m_all) {
// We don't use GetMemoryRegions here because it doesn't include unmapped
// areas like repeating the command would. So instead, emulate doing that.
lldb::addr_t addr = 0;
while (error.Success() && addr != LLDB_INVALID_ADDRESS &&
// When there are non-address bits the last range will not extend
// to LLDB_INVALID_ADDRESS but to the max virtual address.
// This prevents us looping forever if that is the case.
(abi && (abi->FixAnyAddress(addr) == addr))) {
lldb_private::MemoryRegionInfo region_info;
error = process_sp->GetMemoryRegionInfo(addr, region_info);

if (error.Success()) {
region_list.push_back(region_info);
addr = region_info.GetRange().GetRangeEnd();
lldb_private::MemoryRegionInfo range_info;
error = process_sp->GetMemoryRegionInfo(load_addr, range_info);
if (error.Success()) {
lldb_private::Address addr;
ConstString name = range_info.GetName();
ConstString section_name;
if (process_sp->GetTarget().ResolveLoadAddress(load_addr, addr)) {
SectionSP section_sp(addr.GetSection());
if (section_sp) {
// Got the top most section, not the deepest section
while (section_sp->GetParent())
section_sp = section_sp->GetParent();
section_name = section_sp->GetName();
}
}

// Even if we read nothing, don't error for --all
error.Clear();
} else {
lldb_private::MemoryRegionInfo region_info;
error = process_sp->GetMemoryRegionInfo(load_addr, region_info);
if (error.Success())
region_list.push_back(region_info);
}

if (error.Success()) {
for (MemoryRegionInfo &range_info : region_list) {
DumpRegion(result, process_sp->GetTarget(), range_info, load_addr);
m_prev_end_addr = range_info.GetRange().GetRangeEnd();
result.AppendMessageWithFormatv(
"[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
range_info.GetRange().GetRangeBase(),
range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
name, section_name ? " " : "", section_name);
MemoryRegionInfo::OptionalBool memory_tagged =
range_info.GetMemoryTagged();
if (memory_tagged == MemoryRegionInfo::OptionalBool::eYes)
result.AppendMessage("memory tagging: enabled");

const llvm::Optional<std::vector<addr_t>> &dirty_page_list =
range_info.GetDirtyPageList();
if (dirty_page_list.hasValue()) {
const size_t page_count = dirty_page_list.getValue().size();
result.AppendMessageWithFormat(
"Modified memory (dirty) page list provided, %zu entries.\n",
page_count);
if (page_count > 0) {
bool print_comma = false;
result.AppendMessageWithFormat("Dirty pages: ");
for (size_t i = 0; i < page_count; i++) {
if (print_comma)
result.AppendMessageWithFormat(", ");
else
print_comma = true;
result.AppendMessageWithFormat("0x%" PRIx64,
dirty_page_list.getValue()[i]);
}
result.AppendMessageWithFormat(".\n");
}
}

m_prev_end_addr = range_info.GetRange().GetRangeEnd();
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
Expand All @@ -1851,9 +1760,6 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
}

lldb::addr_t m_prev_end_addr = LLDB_INVALID_ADDRESS;

OptionGroupOptions m_option_group;
OptionGroupMemoryRegion m_memory_region_options;
};

// CommandObjectMemory
Expand Down
6 changes: 0 additions & 6 deletions lldb/source/Commands/Options.td
Expand Up @@ -515,12 +515,6 @@ let Command = "memory write" in {
Desc<"Start writing bytes from an offset within the input file.">;
}

let Command = "memory region" in {
def memory_region_all : Option<"all", "a">, Group<2>, Required,
Desc<"Show all memory regions. This is equivalent to starting from address "
"0 and repeating the command. Unmapped areas are included.">;
}

let Command = "memory tag write" in {
def memory_write_end_addr : Option<"end-addr", "e">, Group<1>,
Arg<"AddressOrExpression">, Desc<
Expand Down
28 changes: 2 additions & 26 deletions lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
Expand Up @@ -23,12 +23,6 @@ def setUp(self):
'main.cpp',
'// Run here before printing memory regions')

def test_help(self):
""" Test that help shows you must have one of address or --all, not both."""
self.expect("help memory region",
substrs=["memory region <address-expression>",
"memory region -a"])

def test(self):
self.build()

Expand All @@ -45,44 +39,26 @@ def test(self):
# Test that the first 'memory region' command prints the usage.
interp.HandleCommand("memory region", result)
self.assertFalse(result.Succeeded())
self.assertEqual(result.GetError(),
"error: 'memory region' takes one argument or \"--all\" option:\n"
"Usage: memory region <address-expression> (or --all)\n")

# We allow --all or an address argument, not both
interp.HandleCommand("memory region --all 0", result)
self.assertFalse(result.Succeeded())
self.assertRegexpMatches(result.GetError(),
"The \"--all\" option cannot be used when an address argument is given")
self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")

# Test that when the address fails to parse, we show an error and do not continue
interp.HandleCommand("memory region not_an_address", result)
self.assertFalse(result.Succeeded())
self.assertEqual(result.GetError(),
"error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")

# Accumulate the results to compare with the --all output
all_regions = ""

# Now let's print the memory region starting at 0 which should always work.
interp.HandleCommand("memory region 0x0", result)
self.assertTrue(result.Succeeded())
self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
all_regions += result.GetOutput()

# Keep printing memory regions until we printed all of them.
while True:
interp.HandleCommand("memory region", result)
if not result.Succeeded():
break
all_regions += result.GetOutput()

# Now that we reached the end, 'memory region' should again print the usage.
interp.HandleCommand("memory region", result)
self.assertFalse(result.Succeeded())
self.assertRegexpMatches(result.GetError(), "Usage: memory region <address\-expression> \(or \-\-all\)")

# --all should match what repeating the command gives you
interp.HandleCommand("memory region --all", result)
self.assertTrue(result.Succeeded())
self.assertEqual(result.GetOutput(), all_regions)
self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
Expand Up @@ -55,7 +55,7 @@ def test_mte_regions(self):
if result.Succeeded():
repeats += 1
else:
self.assertRegexpMatches(result.GetError(), "Usage: memory region")
self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
break

# This time repeat until we get the last region. At that
Expand Down
5 changes: 0 additions & 5 deletions llvm/docs/ReleaseNotes.rst
Expand Up @@ -170,11 +170,6 @@ Changes to the LLVM tools
Changes to LLDB
---------------------------------

* The "memory region" command now has a "--all" option to list all
memory regions (including unmapped ranges). This is the equivalent
of using address 0 then repeating the command until all regions
have been listed.

Changes to Sanitizers
---------------------

Expand Down

0 comments on commit 3e928c4

Please sign in to comment.