From 29e556fc2ba93028f0dc45c4c2636da6283e9c28 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 14 Apr 2022 11:56:44 +0100 Subject: [PATCH] [lldb] Change implementation of memory read --show-tags option This does 2 things: * Moves it after the short options. Which makes sense given it's a niche, default off option. (if 2 files for one option seems a bit much, I am going to reuse them for "memory find" later) * Fixes the use of repeated commands. For example: memory read buf --show-tags memory read Added tests for the repetition and updated existing help tests. Reviewed By: omjavaid Differential Revision: https://reviews.llvm.org/D125089 --- .../lldb/Interpreter/OptionGroupMemoryTag.h | 40 +++++++++++++++ lldb/source/Commands/CommandObjectMemory.cpp | 18 ++++--- lldb/source/Commands/Options.td | 2 - lldb/source/Interpreter/CMakeLists.txt | 1 + lldb/source/Interpreter/OptionGroupMemoryTag.cpp | 59 ++++++++++++++++++++++ lldb/test/API/commands/help/TestHelp.py | 6 +-- .../TestAArch64LinuxMTEMemoryTagAccess.py | 30 +++++++++++ 7 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h create mode 100644 lldb/source/Interpreter/OptionGroupMemoryTag.cpp diff --git a/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h new file mode 100644 index 0000000..956ec3f --- /dev/null +++ b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h @@ -0,0 +1,40 @@ +//===-- OptionGroupMemoryTag.h ---------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H +#define LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H + +#include "lldb/Interpreter/OptionValueBoolean.h" +#include "lldb/Interpreter/Options.h" + +namespace lldb_private { + +class OptionGroupMemoryTag : public OptionGroup { +public: + OptionGroupMemoryTag(); + + ~OptionGroupMemoryTag() override = default; + + llvm::ArrayRef GetDefinitions() override; + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value, + ExecutionContext *execution_context) override; + + void OptionParsingStarting(ExecutionContext *execution_context) override; + + bool AnyOptionWasSet() const { return m_show_tags.OptionWasSet(); } + + OptionValueBoolean GetShowTags() { return m_show_tags; }; + +protected: + OptionValueBoolean m_show_tags; +}; + +} // namespace lldb_private + +#endif // LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp index e94306c..f13f749 100644 --- a/lldb/source/Commands/CommandObjectMemory.cpp +++ b/lldb/source/Commands/CommandObjectMemory.cpp @@ -16,6 +16,7 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Interpreter/OptionArgParser.h" #include "lldb/Interpreter/OptionGroupFormat.h" +#include "lldb/Interpreter/OptionGroupMemoryTag.h" #include "lldb/Interpreter/OptionGroupOutputFile.h" #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h" #include "lldb/Interpreter/OptionValueLanguage.h" @@ -90,10 +91,6 @@ public: error = m_offset.SetValueFromString(option_value); break; - case '\x01': - m_show_tags = true; - break; - default: llvm_unreachable("Unimplemented option"); } @@ -107,7 +104,6 @@ public: m_force = false; m_offset.Clear(); m_language_for_type.Clear(); - m_show_tags = false; } Status FinalizeSettings(Target *target, OptionGroupFormat &format_options) { @@ -281,7 +277,6 @@ public: bool m_force; OptionValueUInt64 m_offset; OptionValueLanguage m_language_for_type; - bool m_show_tags = false; }; // Read memory from the inferior process @@ -336,6 +331,8 @@ public: m_option_group.Append(&m_outfile_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1 | LLDB_OPT_SET_2 | LLDB_OPT_SET_3); m_option_group.Append(&m_varobj_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_3); + m_option_group.Append(&m_memory_tag_options, LLDB_OPT_SET_ALL, + LLDB_OPT_SET_ALL); m_option_group.Finalize(); } @@ -555,11 +552,13 @@ protected: if (!m_format_options.AnyOptionWasSet() && !m_memory_options.AnyOptionWasSet() && !m_outfile_options.AnyOptionWasSet() && - !m_varobj_options.AnyOptionWasSet()) { + !m_varobj_options.AnyOptionWasSet() && + !m_memory_tag_options.AnyOptionWasSet()) { m_format_options = m_prev_format_options; m_memory_options = m_prev_memory_options; m_outfile_options = m_prev_outfile_options; m_varobj_options = m_prev_varobj_options; + m_memory_tag_options = m_prev_memory_tag_options; } } @@ -753,6 +752,7 @@ protected: m_prev_memory_options = m_memory_options; m_prev_outfile_options = m_outfile_options; m_prev_varobj_options = m_varobj_options; + m_prev_memory_tag_options = m_memory_tag_options; m_prev_compiler_type = compiler_type; std::unique_ptr output_stream_storage; @@ -864,7 +864,7 @@ protected: size_t bytes_dumped = DumpDataExtractor( data, output_stream_p, 0, format, item_byte_size, item_count, num_per_line / target->GetArchitecture().GetDataByteSize(), addr, 0, 0, - exe_scope, m_memory_options.m_show_tags); + exe_scope, m_memory_tag_options.GetShowTags().GetCurrentValue()); m_next_addr = addr + bytes_dumped; output_stream_p->EOL(); return true; @@ -875,12 +875,14 @@ protected: OptionGroupReadMemory m_memory_options; OptionGroupOutputFile m_outfile_options; OptionGroupValueObjectDisplay m_varobj_options; + OptionGroupMemoryTag m_memory_tag_options; lldb::addr_t m_next_addr = LLDB_INVALID_ADDRESS; lldb::addr_t m_prev_byte_size = 0; OptionGroupFormat m_prev_format_options; OptionGroupReadMemory m_prev_memory_options; OptionGroupOutputFile m_prev_outfile_options; OptionGroupValueObjectDisplay m_prev_varobj_options; + OptionGroupMemoryTag m_prev_memory_tag_options; CompilerType m_prev_compiler_type; }; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index f7b6b09..6b17b76 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -491,8 +491,6 @@ let Command = "memory read" in { "display data.">; def memory_read_force : Option<"force", "r">, Groups<[1,2,3]>, Desc<"Necessary if reading over target.max-memory-read-size bytes.">; - def memory_read_show_tags : Option<"show-tags", "\\x01">, Group<1>, - Desc<"Include memory tags in output (does not apply to binary output).">; } let Command = "memory find" in { diff --git a/lldb/source/Interpreter/CMakeLists.txt b/lldb/source/Interpreter/CMakeLists.txt index d22e94a..c8c7a38 100644 --- a/lldb/source/Interpreter/CMakeLists.txt +++ b/lldb/source/Interpreter/CMakeLists.txt @@ -18,6 +18,7 @@ add_lldb_library(lldbInterpreter OptionGroupBoolean.cpp OptionGroupFile.cpp OptionGroupFormat.cpp + OptionGroupMemoryTag.cpp OptionGroupPythonClassWithDict.cpp OptionGroupOutputFile.cpp OptionGroupPlatform.cpp diff --git a/lldb/source/Interpreter/OptionGroupMemoryTag.cpp b/lldb/source/Interpreter/OptionGroupMemoryTag.cpp new file mode 100644 index 0000000..f63dfd6 --- /dev/null +++ b/lldb/source/Interpreter/OptionGroupMemoryTag.cpp @@ -0,0 +1,59 @@ +//===-- OptionGroupMemoryTag.cpp -----------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Interpreter/OptionGroupMemoryTag.h" + +#include "lldb/Host/OptionParser.h" + +using namespace lldb; +using namespace lldb_private; + +OptionGroupMemoryTag::OptionGroupMemoryTag() : m_show_tags(false, false) {} + +static const uint32_t SHORT_OPTION_SHOW_TAGS = 0x54414753; // 'tags' + +static constexpr OptionDefinition g_option_table[] = { + {LLDB_OPT_SET_1, + false, + "show-tags", + SHORT_OPTION_SHOW_TAGS, + OptionParser::eNoArgument, + nullptr, + {}, + 0, + eArgTypeNone, + "Include memory tags in output (does not apply to binary output)."}, +}; + +llvm::ArrayRef OptionGroupMemoryTag::GetDefinitions() { + return llvm::makeArrayRef(g_option_table); +} + +Status +OptionGroupMemoryTag::SetOptionValue(uint32_t option_idx, + llvm::StringRef option_arg, + ExecutionContext *execution_context) { + assert(option_idx == 0 && "Only one option in memory tag group!"); + + switch (g_option_table[0].short_option) { + case SHORT_OPTION_SHOW_TAGS: + m_show_tags.SetCurrentValue(true); + m_show_tags.SetOptionWasSet(); + break; + + default: + llvm_unreachable("Unimplemented option"); + } + + return {}; +} + +void OptionGroupMemoryTag::OptionParsingStarting( + ExecutionContext *execution_context) { + m_show_tags.Clear(); +} diff --git a/lldb/test/API/commands/help/TestHelp.py b/lldb/test/API/commands/help/TestHelp.py index 49f8341..14f4472 100644 --- a/lldb/test/API/commands/help/TestHelp.py +++ b/lldb/test/API/commands/help/TestHelp.py @@ -262,9 +262,9 @@ class HelpCommandTestCase(TestBase): """Test that we put a break between the usage and the options help lines, and between the options themselves.""" self.expect("help memory read", substrs=[ - "[]\n\n --show-tags", - # Starts with the end of the show-tags line - "output).\n\n -A"]) + "[]\n\n -A ( --show-all-children )", + # Starts with the end of the show-all-children line + "to show.\n\n -D"]) @no_debug_info_test def test_help_detailed_information_ordering(self): diff --git a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py index 1423307..8ca5db4 100644 --- a/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py +++ b/lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py @@ -411,3 +411,33 @@ class AArch64LinuxMTEMemoryTagAccessTestCase(TestBase): self.expect("memory read mte_buf mte_buf+32 -f \"uint8_t[]\" -s 16 -l 1 --show-tags", patterns=["0x[0-9A-Fa-f]+00: \{(0x00 ){15}0x00\} \(tag: 0x0\)\n" "0x[0-9A-Fa-f]+10: \{(0x00 ){15}0x00\} \(tag: 0x1\)"]) + + @skipUnlessArch("aarch64") + @skipUnlessPlatform(["linux"]) + @skipUnlessAArch64MTELinuxCompiler + def test_mte_memory_read_tag_display_repeated(self): + """Test that the --show-tags option is kept when repeating the memory read command.""" + self.setup_mte_test() + + self.expect("memory read mte_buf mte_buf+16 -f \"x\" --show-tags", + patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+ \(tag: 0x0\)"]) + # Equivalent to just pressing enter on the command line. + self.expect("memory read", + patterns=["0x[0-9A-fa-f]+10: 0x0+ 0x0+ 0x0+ 0x0+ \(tag: 0x1\)"]) + + # You can add the argument to an existing repetition without resetting + # the whole command. Though all other optional arguments will reset to + # their default values when you do this. + self.expect("memory read mte_buf mte_buf+16 -f \"x\"", + patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+"]) + self.expect("memory read", + patterns=["0x[0-9A-fa-f]+10: 0x0+ 0x0+ 0x0+ 0x0+"]) + # Note that the formatting returns to default here. + self.expect("memory read --show-tags", + patterns=["0x[0-9A-fa-f]+20: (00 )+ \.+ \(tag: 0x2\)"]) + self.expect("memory read", + patterns=["0x[0-9A-fa-f]+30: (00 )+ \.+ \(tag: 0x3\)"]) + + # A fresh command reverts to the default of tags being off. + self.expect("memory read mte_buf mte_buf+16 -f \"x\"", + patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+"]) -- 2.7.4