[Lldb-commits] [lldb] 3e928c4 - Revert "[lldb] Add --all option to "memory region""
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Wed May 18 04:57:54 PDT 2022
Author: David Spickett
Date: 2022-05-18T11:57:20Z
New Revision: 3e928c4b9dfb01efd2cb968795e605760828e873
URL: https://github.com/llvm/llvm-project/commit/3e928c4b9dfb01efd2cb968795e605760828e873
DIFF: https://github.com/llvm/llvm-project/commit/3e928c4b9dfb01efd2cb968795e605760828e873.diff
LOG: Revert "[lldb] Add --all option to "memory region""
This reverts commit 8e648f195c3d57e573fdd8023edcfd80e0516c61
due to test failures on Windows:
https://lab.llvm.org/buildbot/#/builders/83/builds/19094
Added:
Modified:
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/Options.td
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
llvm/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 88fbf05fc057..e94306ce33bf 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -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) {
@@ -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,
@@ -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;
}
@@ -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
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 1468748120b4..f7b6b09b08d2 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -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<
diff --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index cad54c8f892f..61e64d44e794 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -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()
@@ -45,15 +39,7 @@ 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)
@@ -61,28 +47,18 @@ def test(self):
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")
diff --git a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
index cbd772ecebbc..0b5f341574e0 100644
--- a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
+++ b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -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
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 7b49416b7151..54f2029aa25d 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -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
---------------------
More information about the lldb-commits
mailing list