From e07a421dd587f596b3b34ac2f79081402089f878 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Fri, 13 Jan 2023 15:50:37 +0000 Subject: [PATCH] [lldb] Show register fields using bitfield struct types This change uses the information from target.xml sent by the GDB stub to produce C types that we can use to print register fields. lldb-server *does not* produce this information yet. This will only work with GDB stubs that do. gdbserver or qemu are 2 I know of. Testing is added that uses a mocked lldb-server. ``` (lldb) register read cpsr x0 fpcr fpsr x1 cpsr = 0x60001000 = (N = 0, Z = 1, C = 1, V = 0, TCO = 0, DIT = 0, UAO = 0, PAN = 0, SS = 0, IL = 0, SSBS = 1, BTYPE = 0, D = 0, A = 0, I = 0, F = 0, nRW = 0, EL = 0, SP = 0) ``` Only "register read" will display fields, and only when we are not printing a register block. For example, cpsr is a 32 bit register. Using the target's scratch type system we construct a type: ``` struct __attribute__((__packed__)) cpsr { uint32_t N : 1; uint32_t Z : 1; ... uint32_t EL : 2; uint32_t SP : 1; }; ``` If this register had unallocated bits in it, those would have been filled in by RegisterFlags as anonymous fields. A new option "SetChildPrintingDecider" is added so we can disable printing those. Important things about this type: * It is packed so that sizeof(struct cpsr) == sizeof(the real register). (this will hold for all flags types we create) * Each field has the same storage type, which is the same as the type of the raw register value. This prevents fields being spilt over into more storage units, as is allowed by most ABIs. * Each bitfield size matches that of its register field. * The most significant field is first. The last point is required because the most significant bit (MSB) being on the left/top of a print out matches what you'd expect to see in an architecture manual. In addition, having lldb print a different field order on big/little endian hosts is not acceptable. As a consequence, if the target is little endian we have to reverse the order of the fields in the value. The value of each field remains the same. For example 0b01 doesn't become 0b10, it just shifts up or down. This is needed because clang's type system assumes that for a struct like the one above, the least significant bit (LSB) will be first for a little endian target. We need the MSB to be first. Finally, if lldb's host is a different endian to the target we have to byte swap the host endian value to match the endian of the target's typesystem. | Host Endian | Target Endian | Field Order Swap | Byte Order Swap | |-------------|---------------|------------------|-----------------| | Little | Little | Yes | No | | Big | Little | Yes | Yes | | Little | Big | No | Yes | | Big | Big | No | No | Testing was done as follows: * Little -> Little * LE AArch64 native debug. * Big -> Little * s390x lldb running under QEMU, connected to LE AArch64 target. * Little -> Big * LE AArch64 lldb connected to QEMU's GDB stub, which is running an s390x program. * Big -> Big * s390x lldb running under QEMU, connected to another QEMU's GDB stub, which is running an s390x program. As we are not allowed to link core code to plugins directly, I have added a new plugin RegisterTypeBuilder. There is one implementation of this, RegisterTypeBuilderClang, which uses TypeSystemClang to build the CompilerType from the register fields. Reviewed By: jasonmolenda Differential Revision: https://reviews.llvm.org/D145580 --- lldb/include/lldb/Core/DumpRegisterValue.h | 7 +- lldb/include/lldb/Core/PluginManager.h | 9 + .../lldb/DataFormatters/DumpValueObjectOptions.h | 5 + lldb/include/lldb/Target/RegisterFlags.h | 17 + lldb/include/lldb/Target/RegisterTypeBuilder.h | 35 ++ lldb/include/lldb/Target/Target.h | 4 + lldb/include/lldb/lldb-forward.h | 3 + lldb/include/lldb/lldb-private-interfaces.h | 2 + lldb/source/Commands/CommandObjectRegister.cpp | 12 +- lldb/source/Core/DumpRegisterValue.cpp | 97 +++- lldb/source/Core/PluginManager.cpp | 39 ++ .../DataFormatters/DumpValueObjectOptions.cpp | 9 +- lldb/source/DataFormatters/ValueObjectPrinter.cpp | 10 +- lldb/source/Plugins/CMakeLists.txt | 1 + .../Plugins/RegisterTypeBuilder/CMakeLists.txt | 10 + .../RegisterTypeBuilderClang.cpp | 82 ++++ .../RegisterTypeBuilder/RegisterTypeBuilderClang.h | 40 ++ lldb/source/Target/Target.cpp | 9 + .../gdb_remote_client/TestXMLRegisterFlags.py | 497 +++++++++++++++++++++ lldb/unittests/Target/RegisterFlagsTest.cpp | 16 + llvm/docs/ReleaseNotes.rst | 6 + 21 files changed, 902 insertions(+), 8 deletions(-) create mode 100644 lldb/include/lldb/Target/RegisterTypeBuilder.h create mode 100644 lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt create mode 100644 lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp create mode 100644 lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py diff --git a/lldb/include/lldb/Core/DumpRegisterValue.h b/lldb/include/lldb/Core/DumpRegisterValue.h index 08bf0af..9dc579d 100644 --- a/lldb/include/lldb/Core/DumpRegisterValue.h +++ b/lldb/include/lldb/Core/DumpRegisterValue.h @@ -10,6 +10,7 @@ #define LLDB_CORE_DUMPREGISTERVALUE_H #include "lldb/lldb-enumerations.h" +#include "lldb/lldb-forward.h" #include namespace lldb_private { @@ -21,11 +22,15 @@ class Stream; // The default value of 0 for reg_name_right_align_at means no alignment at // all. +// Set print_flags to true to print register fields if they are available. +// If you do so, target_sp must be non-null for it to work. void DumpRegisterValue(const RegisterValue ®_val, Stream *s, const RegisterInfo *reg_info, bool prefix_with_name, bool prefix_with_alt_name, lldb::Format format, uint32_t reg_name_right_align_at = 0, - ExecutionContextScope *exe_scope = nullptr); + ExecutionContextScope *exe_scope = nullptr, + bool print_flags = false, + lldb::TargetSP target_sp = nullptr); } // namespace lldb_private diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 4e036e0..82cee28 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -253,6 +253,15 @@ public: static void AutoCompleteProcessName(llvm::StringRef partial_name, CompletionRequest &request); + // Register Type Provider + static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description, + RegisterTypeBuilderCreateInstance create_callback); + + static bool + UnregisterPlugin(RegisterTypeBuilderCreateInstance create_callback); + + static lldb::RegisterTypeBuilderSP GetRegisterTypeBuilder(Target &target); + // ScriptInterpreter static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description, lldb::ScriptLanguage script_lang, diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h index dedb645..b622c34 100644 --- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h +++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h @@ -53,6 +53,8 @@ public: const DumpValueObjectOptions &, Stream &)> DeclPrintingHelper; + typedef std::function ChildPrintingDecider; + static const DumpValueObjectOptions DefaultOptions() { static DumpValueObjectOptions g_default_options; @@ -70,6 +72,8 @@ public: DumpValueObjectOptions &SetDeclPrintingHelper(DeclPrintingHelper helper); + DumpValueObjectOptions &SetChildPrintingDecider(ChildPrintingDecider decider); + DumpValueObjectOptions &SetShowTypes(bool show = false); DumpValueObjectOptions &SetShowLocation(bool show = false); @@ -136,6 +140,7 @@ public: lldb::LanguageType m_varformat_language = lldb::eLanguageTypeUnknown; PointerDepth m_max_ptr_depth; DeclPrintingHelper m_decl_printing_helper; + ChildPrintingDecider m_child_printing_decider; PointerAsArraySettings m_pointer_as_array; bool m_use_synthetic : 1; bool m_scope_already_checked : 1; diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h index 89058d5..2dd20f9 100644 --- a/lldb/include/lldb/Target/RegisterFlags.h +++ b/lldb/include/lldb/Target/RegisterFlags.h @@ -71,6 +71,23 @@ public: RegisterFlags(std::string id, unsigned size, const std::vector &fields); + // Reverse the order of the fields, keeping their values the same. + // For example a field from bit 31 to 30 with value 0b10 will become bits + // 1 to 0, with the same 0b10 value. + // Use this when you are going to show the register using a bitfield struct + // type. If that struct expects MSB first and you are on little endian where + // LSB would be first, this corrects that (and vice versa for big endian). + template T ReverseFieldOrder(T value) const { + T ret = 0; + unsigned shift = 0; + for (auto field : GetFields()) { + ret |= field.GetValue(value) << shift; + shift += field.GetSizeInBits(); + } + + return ret; + } + const std::vector &GetFields() const { return m_fields; } const std::string &GetID() const { return m_id; } unsigned GetSize() const { return m_size; } diff --git a/lldb/include/lldb/Target/RegisterTypeBuilder.h b/lldb/include/lldb/Target/RegisterTypeBuilder.h new file mode 100644 index 0000000..f7b5814 --- /dev/null +++ b/lldb/include/lldb/Target/RegisterTypeBuilder.h @@ -0,0 +1,35 @@ +//===-- RegisterTypeBuilder.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_TARGET_REGISTER_TYPE_BUILDER_H +#define LLDB_TARGET_REGISTER_TYPE_BUILDER_H + +#include "lldb/Core/PluginInterface.h" +#include "lldb/lldb-private.h" + +namespace lldb_private { + +class RegisterTypeBuilder : public PluginInterface { +public: + ~RegisterTypeBuilder() override = default; + + virtual CompilerType GetRegisterType(const std::string &name, + const lldb_private::RegisterFlags &flags, + uint32_t byte_size) = 0; + +protected: + RegisterTypeBuilder() = default; + +private: + RegisterTypeBuilder(const RegisterTypeBuilder &) = delete; + const RegisterTypeBuilder &operator=(const RegisterTypeBuilder &) = delete; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_REGISTER_TYPE_BUILDER_H diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index b322ff7..b557dd2 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1243,6 +1243,10 @@ public: /// if none can be found. llvm::Expected GetEntryPointAddress(); + CompilerType GetRegisterType(const std::string &name, + const lldb_private::RegisterFlags &flags, + uint32_t byte_size); + // Target Stop Hooks class StopHook : public UserID { public: diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index e36bdcb..262afd2 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -175,6 +175,7 @@ class REPL; class RecognizedStackFrame; class RegisterCheckpoint; class RegisterContext; +class RegisterTypeBuilder; class RegisterValue; class RegularExpression; class RichManglingContext; @@ -371,6 +372,8 @@ typedef std::shared_ptr ProcessLaunchInfoSP; typedef std::weak_ptr ProcessWP; typedef std::shared_ptr RegisterCheckpointSP; typedef std::shared_ptr RegisterContextSP; +typedef std::shared_ptr + RegisterTypeBuilderSP; typedef std::shared_ptr RegularExpressionSP; typedef std::shared_ptr QueueSP; typedef std::weak_ptr QueueWP; diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 1eb37c0..236a7f8 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -82,6 +82,8 @@ typedef lldb::PlatformSP (*PlatformCreateInstance)(bool force, typedef lldb::ProcessSP (*ProcessCreateInstance)( lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, const FileSpec *crash_file_path, bool can_connect); +typedef lldb::RegisterTypeBuilderSP (*RegisterTypeBuilderCreateInstance)( + Target &target); typedef lldb::ScriptInterpreterSP (*ScriptInterpreterCreateInstance)( Debugger &debugger); typedef SymbolFile *(*SymbolFileCreateInstance)(lldb::ObjectFileSP objfile_sp); diff --git a/lldb/source/Commands/CommandObjectRegister.cpp b/lldb/source/Commands/CommandObjectRegister.cpp index c61a1f6..4b0d455 100644 --- a/lldb/source/Commands/CommandObjectRegister.cpp +++ b/lldb/source/Commands/CommandObjectRegister.cpp @@ -84,7 +84,8 @@ public: Options *GetOptions() override { return &m_option_group; } bool DumpRegister(const ExecutionContext &exe_ctx, Stream &strm, - RegisterContext *reg_ctx, const RegisterInfo *reg_info) { + RegisterContext *reg_ctx, const RegisterInfo *reg_info, + bool print_flags) { if (reg_info) { RegisterValue reg_value; @@ -95,7 +96,8 @@ public: bool prefix_with_name = !prefix_with_altname; DumpRegisterValue(reg_value, &strm, reg_info, prefix_with_name, prefix_with_altname, m_format_options.GetFormat(), 8, - exe_ctx.GetBestExecutionContextScope()); + exe_ctx.GetBestExecutionContextScope(), print_flags, + exe_ctx.GetTargetSP()); if ((reg_info->encoding == eEncodingUint) || (reg_info->encoding == eEncodingSint)) { Process *process = exe_ctx.GetProcessPtr(); @@ -142,7 +144,8 @@ public: if (primitive_only && reg_info && reg_info->value_regs) continue; - if (DumpRegister(exe_ctx, strm, reg_ctx, reg_info)) + if (DumpRegister(exe_ctx, strm, reg_ctx, reg_info, + /*print_flags=*/false)) ++available_count; else ++unavailable_count; @@ -218,7 +221,8 @@ protected: reg_info = reg_ctx->GetRegisterInfoByName(arg_str); if (reg_info) { - if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info)) + if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info, + /*print_flags=*/true)) strm.Printf("%-12s = error: unavailable\n", reg_info->name); } else { result.AppendErrorWithFormat("Invalid register name '%s'.\n", diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp index 14659e0..d1bde2c 100644 --- a/lldb/source/Core/DumpRegisterValue.cpp +++ b/lldb/source/Core/DumpRegisterValue.cpp @@ -8,19 +8,63 @@ #include "lldb/Core/DumpRegisterValue.h" #include "lldb/Core/DumpDataExtractor.h" +#include "lldb/Core/ValueObject.h" +#include "lldb/Core/ValueObjectConstResult.h" +#include "lldb/DataFormatters/DumpValueObjectOptions.h" #include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/Endian.h" #include "lldb/Utility/RegisterValue.h" #include "lldb/Utility/StreamString.h" #include "lldb/lldb-private-types.h" using namespace lldb; +static uint32_t swap_value(uint32_t v) { return __builtin_bswap32(v); } +static uint64_t swap_value(uint64_t v) { return __builtin_bswap64(v); } + +template +static void dump_type_value(lldb_private::CompilerType &fields_type, T value, + lldb_private::ExecutionContextScope *exe_scope, + const lldb_private::RegisterInfo ®_info, + lldb_private::Stream &strm) { + lldb::ByteOrder target_order = exe_scope->CalculateProcess()->GetByteOrder(); + + // For the bitfield types we generate, it is expected that the fields are + // in what is usually a big endian order. Most significant field first. + // This is also clang's internal ordering and the order we want to print + // them. On a big endian host this all matches up, for a little endian + // host we have to swap the order of the fields before display. + if (target_order == lldb::ByteOrder::eByteOrderLittle) { + value = reg_info.flags_type->ReverseFieldOrder(value); + } + + // Then we need to match the target's endian on a byte level as well. + if (lldb_private::endian::InlHostByteOrder() != target_order) + value = swap_value(value); + + lldb_private::DataExtractor data_extractor{ + &value, sizeof(T), lldb_private::endian::InlHostByteOrder(), 8}; + + lldb::ValueObjectSP vobj_sp = lldb_private::ValueObjectConstResult::Create( + exe_scope, fields_type, lldb_private::ConstString(), data_extractor); + lldb_private::DumpValueObjectOptions dump_options; + lldb_private::DumpValueObjectOptions::ChildPrintingDecider decider = + [](lldb_private::ConstString varname) { + // Unnamed bit-fields are padding that we don't want to show. + return varname.GetLength(); + }; + dump_options.SetChildPrintingDecider(decider).SetHideRootType(true); + + vobj_sp->Dump(strm, dump_options); +} + void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s, const RegisterInfo *reg_info, bool prefix_with_name, bool prefix_with_alt_name, Format format, uint32_t reg_name_right_align_at, - ExecutionContextScope *exe_scope) { + ExecutionContextScope *exe_scope, + bool print_flags, TargetSP target_sp) { DataExtractor data; if (!reg_val.GetData(data)) return; @@ -76,4 +120,55 @@ void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s, 0, // item_bit_size 0, // item_bit_offset exe_scope); + + if (!print_flags || !reg_info->flags_type || !exe_scope || !target_sp || + (reg_info->byte_size != 4 && reg_info->byte_size != 8)) + return; + + CompilerType fields_type = target_sp->GetRegisterType( + reg_info->name, *reg_info->flags_type, reg_info->byte_size); + + // Use a new stream so we can remove a trailing newline later. + StreamString fields_stream; + + if (reg_info->byte_size == 4) { + dump_type_value(fields_type, reg_val.GetAsUInt32(), exe_scope, *reg_info, + fields_stream); + } else { + dump_type_value(fields_type, reg_val.GetAsUInt64(), exe_scope, *reg_info, + fields_stream); + } + + // Registers are indented like: + // (lldb) register read foo + // foo = 0x12345678 + // So we need to indent to match that. + + // First drop the extra newline that the value printer added. The register + // command will add one itself. + llvm::StringRef fields_str = fields_stream.GetString().drop_back(); + + // End the line that contains " foo = 0x12345678". + s->EOL(); + + // Then split the value lines and indent each one. + bool first = true; + while (fields_str.size()) { + std::pair split = fields_str.split('\n'); + fields_str = split.second; + // Indent as far as the register name did. + s->Printf(fmt.c_str(), ""); + + // Lines after the first won't have " = " so compensate for that. + if (!first) + (*s) << " "; + first = false; + + (*s) << split.first; + + // On the last line we don't want a newline because the command will add + // one too. + if (fields_str.size()) + s->EOL(); + } } diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 05ca0ff..0ef0d1e 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -875,6 +875,45 @@ void PluginManager::AutoCompleteProcessName(llvm::StringRef name, } } +#pragma mark RegisterTypeBuilder + +struct RegisterTypeBuilderInstance + : public PluginInstance { + RegisterTypeBuilderInstance(llvm::StringRef name, llvm::StringRef description, + CallbackType create_callback) + : PluginInstance(name, description, + create_callback) {} +}; + +typedef PluginInstances + RegisterTypeBuilderInstances; + +static RegisterTypeBuilderInstances &GetRegisterTypeBuilderInstances() { + static RegisterTypeBuilderInstances g_instances; + return g_instances; +} + +bool PluginManager::RegisterPlugin( + llvm::StringRef name, llvm::StringRef description, + RegisterTypeBuilderCreateInstance create_callback) { + return GetRegisterTypeBuilderInstances().RegisterPlugin(name, description, + create_callback); +} + +bool PluginManager::UnregisterPlugin( + RegisterTypeBuilderCreateInstance create_callback) { + return GetRegisterTypeBuilderInstances().UnregisterPlugin(create_callback); +} + +lldb::RegisterTypeBuilderSP +PluginManager::GetRegisterTypeBuilder(Target &target) { + const auto &instances = GetRegisterTypeBuilderInstances().GetInstances(); + // We assume that RegisterTypeBuilderClang is the only instance of this plugin + // type and is always present. + assert(instances.size()); + return instances[0].create_callback(target); +} + #pragma mark ScriptInterpreter struct ScriptInterpreterInstance diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp index 3fbff86..c5e8481 100644 --- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp +++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp @@ -16,7 +16,8 @@ using namespace lldb_private; DumpValueObjectOptions::DumpValueObjectOptions() : m_summary_sp(), m_root_valobj_name(), m_max_ptr_depth(PointerDepth{PointerDepth::Mode::Default, 0}), - m_decl_printing_helper(), m_pointer_as_array(), m_use_synthetic(true), + m_decl_printing_helper(), m_child_printing_decider(), + m_pointer_as_array(), m_use_synthetic(true), m_scope_already_checked(false), m_flat_output(false), m_ignore_cap(false), m_show_types(false), m_show_location(false), m_use_objc(false), m_hide_root_type(false), m_hide_root_name(false), m_hide_name(false), @@ -50,6 +51,12 @@ DumpValueObjectOptions::SetDeclPrintingHelper(DeclPrintingHelper helper) { return *this; } +DumpValueObjectOptions & +DumpValueObjectOptions::SetChildPrintingDecider(ChildPrintingDecider decider) { + m_child_printing_decider = decider; + return *this; +} + DumpValueObjectOptions &DumpValueObjectOptions::SetShowTypes(bool show) { m_show_types = show; return *this; diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp index 30f69d8..8e44f72 100644 --- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp +++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp @@ -696,6 +696,9 @@ void ValueObjectPrinter::PrintChildren( for (size_t idx = 0; idx < num_children; ++idx) { if (ValueObjectSP child_sp = GenerateChild(synth_m_valobj, idx)) { + if (m_options.m_child_printing_decider && + !m_options.m_child_printing_decider(child_sp->GetName())) + continue; if (!any_children_printed) { PrintChildrenPreamble(value_printed, summary_printed); any_children_printed = true; @@ -744,14 +747,19 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) { if (num_children) { m_stream->PutChar('('); + bool did_print_children = false; for (uint32_t idx = 0; idx < num_children; ++idx) { lldb::ValueObjectSP child_sp(synth_m_valobj->GetChildAtIndex(idx, true)); if (child_sp) child_sp = child_sp->GetQualifiedRepresentationIfAvailable( m_options.m_use_dynamic, m_options.m_use_synthetic); if (child_sp) { - if (idx) + if (m_options.m_child_printing_decider && + !m_options.m_child_printing_decider(child_sp->GetName())) + continue; + if (idx && did_print_children) m_stream->PutCString(", "); + did_print_children = true; if (!hide_names) { const char *name = child_sp.get()->GetName().AsCString(); if (name && *name) { diff --git a/lldb/source/Plugins/CMakeLists.txt b/lldb/source/Plugins/CMakeLists.txt index 855c9f6..63f2c5b 100644 --- a/lldb/source/Plugins/CMakeLists.txt +++ b/lldb/source/Plugins/CMakeLists.txt @@ -15,6 +15,7 @@ add_subdirectory(OperatingSystem) add_subdirectory(Platform) add_subdirectory(Process) add_subdirectory(REPL) +add_subdirectory(RegisterTypeBuilder) add_subdirectory(ScriptInterpreter) add_subdirectory(StructuredData) add_subdirectory(SymbolFile) diff --git a/lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt b/lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt new file mode 100644 index 0000000..7abd34a --- /dev/null +++ b/lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt @@ -0,0 +1,10 @@ +add_lldb_library(lldbPluginRegisterTypeBuilderClang PLUGIN + RegisterTypeBuilderClang.cpp + + LINK_LIBS + lldbCore + lldbTarget + + LINK_COMPONENTS + Support + ) diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp new file mode 100644 index 0000000..abd6afb --- /dev/null +++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp @@ -0,0 +1,82 @@ +//===-- RegisterTypeBuilderClang.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 "clang/AST/DeclCXX.h" + +#include "Plugins/TypeSystem/Clang/TypeSystemClang.h" +#include "RegisterTypeBuilderClang.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/lldb-enumerations.h" + +using namespace lldb_private; + +LLDB_PLUGIN_DEFINE(RegisterTypeBuilderClang) + +void RegisterTypeBuilderClang::Initialize() { + static llvm::once_flag g_once_flag; + llvm::call_once(g_once_flag, []() { + PluginManager::RegisterPlugin(GetPluginNameStatic(), + GetPluginDescriptionStatic(), CreateInstance); + }); +} + +void RegisterTypeBuilderClang::Terminate() {} + +lldb::RegisterTypeBuilderSP +RegisterTypeBuilderClang::CreateInstance(Target &target) { + return std::make_shared(target); +} + +RegisterTypeBuilderClang::RegisterTypeBuilderClang(Target &target) + : m_target(target) {} + +CompilerType RegisterTypeBuilderClang::GetRegisterType( + const std::string &name, const lldb_private::RegisterFlags &flags, + uint32_t byte_size) { + lldb::TypeSystemClangSP type_system = + ScratchTypeSystemClang::GetForTarget(m_target); + assert(type_system); + + std::string register_type_name = "__lldb_register_fields_"; + register_type_name += name; + // See if we have made this type before and can reuse it. + CompilerType fields_type = + type_system->GetTypeForIdentifier( + ConstString(register_type_name.c_str())); + + if (!fields_type) { + // In most ABI, a change of field type means a change in storage unit. + // We want it all in one unit, so we use a field type the same as the + // register's size. + CompilerType field_uint_type = + type_system->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint, + byte_size * 8); + + fields_type = type_system->CreateRecordType( + nullptr, OptionalClangModuleID(), lldb::eAccessPublic, + register_type_name, clang::TTK_Struct, lldb::eLanguageTypeC); + type_system->StartTagDeclarationDefinition(fields_type); + + // We assume that RegisterFlags has padded and sorted the fields + // already. + for (const RegisterFlags::Field &field : flags.GetFields()) { + type_system->AddFieldToRecordType(fields_type, field.GetName(), + field_uint_type, lldb::eAccessPublic, + field.GetSizeInBits()); + } + + type_system->CompleteTagDeclarationDefinition(fields_type); + // So that the size of the type matches the size of the register. + type_system->SetIsPacked(fields_type); + + // This should be true if RegisterFlags padded correctly. + assert(*fields_type.GetByteSize(nullptr) == flags.GetSize()); + } + + return fields_type; +} diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h new file mode 100644 index 0000000..a094b22 --- /dev/null +++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h @@ -0,0 +1,40 @@ +//===-- RegisterTypeBuilderClang.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_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H +#define LLDB_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H + +#include "lldb/Target/RegisterTypeBuilder.h" +#include "lldb/Target/Target.h" + +namespace lldb_private { +class RegisterTypeBuilderClang : public RegisterTypeBuilder { +public: + RegisterTypeBuilderClang(Target &target); + + static void Initialize(); + static void Terminate(); + static llvm::StringRef GetPluginNameStatic() { + return "register-types-clang"; + } + llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } + static llvm::StringRef GetPluginDescriptionStatic() { + return "Create register types using TypeSystemClang"; + } + static lldb::RegisterTypeBuilderSP CreateInstance(Target &target); + + CompilerType GetRegisterType(const std::string &name, + const lldb_private::RegisterFlags &flags, + uint32_t byte_size) override; + +private: + Target &m_target; +}; +} // namespace lldb_private + +#endif // LLDB_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index bda8966..a6e6f21 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -46,6 +46,7 @@ #include "lldb/Target/Language.h" #include "lldb/Target/LanguageRuntime.h" #include "lldb/Target/Process.h" +#include "lldb/Target/RegisterTypeBuilder.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/StackFrame.h" #include "lldb/Target/StackFrameRecognizer.h" @@ -2358,6 +2359,14 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language, create_on_demand); } +CompilerType Target::GetRegisterType(const std::string &name, + const lldb_private::RegisterFlags &flags, + uint32_t byte_size) { + RegisterTypeBuilderSP provider = PluginManager::GetRegisterTypeBuilder(*this); + assert(provider); + return provider->GetRegisterType(name, flags, byte_size); +} + std::vector Target::GetScratchTypeSystems(bool create_on_demand) { if (!m_valid) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py new file mode 100644 index 0000000..2c930d6 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py @@ -0,0 +1,497 @@ +""" Check that register fields found in target XML are properly processed. + +These tests make XML out of string substitution. This can lead to some strange +failures. Check that the final XML is valid and each child is indented more than +the parent tag. +""" + +from textwrap import dedent +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import * +from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase + +class MultiDocResponder(MockGDBServerResponder): + # docs is a dictionary of filename -> file content. + def __init__(self, docs): + super().__init__() + self.docs = docs + + def qXferRead(self, obj, annex, offset, length): + try: + return self.docs[annex], False + except KeyError: + return None, + + def readRegister(self, regnum): + return "E01" + + def readRegisters(self): + return ''.join([ + # Data for all registers requested by the tests below. + # 0x7 and 0xE are used because their lsb and msb are opposites, which + # is needed for a byte order test. + '77777777EEEEEEEE', # 64 bit x0/r0 + '7777EEEE', # 32 bit cpsr/fpc + '0000000000000000', # 64 bit pc/pswa + ]) + +class TestXMLRegisterFlags(GDBRemoteTestBase): + def setup_multidoc_test(self, docs): + self.server.responder = MultiDocResponder(docs) + target = self.dbg.CreateTarget('') + + if self.TraceOn(): + self.runCmd("log enable gdb-remote packets process") + self.addTearDownHook( + lambda: self.runCmd("log disable gdb-remote packets process")) + + process = self.connect(target) + lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, + [lldb.eStateStopped]) + + def setup_register_test(self, registers): + self.setup_multidoc_test( + # This *must* begin with the opening tag, leading whitespace is not allowed. + {'target.xml' : dedent("""\ + + + aarch64 + + {} + + """).format(registers)}) + + def setup_flags_test(self, flags): + # pc is required here though we don't look at it in the tests. + # x0 is only used by some tests but always including it keeps the data ordering + # the same throughout. + self.setup_register_test("""\ + + {} + + + + """.format( + flags)) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_no_flags(self): + self.setup_flags_test("") + self.expect("register read cpsr", substrs=["= 0xeeee7777"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_single_field_pad_msb(self): + self.setup_flags_test("""""") + # Pads from 31 to 1. + self.expect("register read cpsr", substrs=["(SP = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_single_field_pad_lsb(self): + self.setup_flags_test("""""") + self.expect("register read cpsr", substrs=["(SP = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_multiple_fields_sorted(self): + self.setup_flags_test(""" + """) + + # Fields should be sorted with MSB on the left. + self.expect("register read cpsr", substrs=["(EL = 3, SP = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_ignore_invalid_start_end(self): + self.setup_flags_test( + # Is valid so is used. + '' + # Start/end cannot be negative, ignored. + '' + '' + # Start is not <= end, ignored. + '' + # Start cannot be >= (size of register in bits) + '' + # End cannot be >= (size of register in bits) + '') + + self.expect("register read cpsr", substrs=["(EL = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_field_overlap(self): + self.setup_flags_test( + '' + # A overlaps B + '' + '') + + # Ignore the whole flags set, it is unlikely to be valid. + self.expect("register read cpsr", substrs=["("], matching=False) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_field_required_attributes(self): + # Fields must have a name, start and end. Any without are ignored. + self.setup_flags_test( + # Missing name + '' + # Missing start + '' + # Missing end + '' + # Valid + '') + + self.expect("register read cpsr", substrs=["(C = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_little_endian_target_order(self): + # We are using little endian AArch64 here. + self.setup_register_test("""\ + + + + + + + + + + + """) + + # If lldb used the wrong byte ordering for the value for printing fields, + # these field values would flip. Since the top and bottom bits of 0x7 and 0xE + # are different. + self.expect("register read cpsr x0", substrs=[ + " cpsr = 0xeeee7777\n" + " = (msb = 1, lsb = 1)\n" + " x0 = 0xeeeeeeee77777777\n" + " = (msb = 1, lsb = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + # Unlike AArch64, we do need the backend present for this test to work. + @skipIfLLVMTargetMissing("SystemZ") + def test_big_endian_target_order(self): + # s390x/SystemZ is big endian. + self.setup_multidoc_test({ + 'target.xml' : dedent("""\ + + + s390x + + + + + + + + + + + + + + """)}) + + # If we did not swap correctly, these fields would show as 1s when run on + # a little endian host. + self.expect("register read r0 fpc", substrs=[ + " r0 = 0x77777777eeeeeeee\n" + " = (msb = 0, lsb = 0)\n" + " fpc = 0x7777eeee\n" + " = (msb = 0, lsb = 0)\n" + ]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_many_flag_sets(self): + self.setup_register_test("""\ + + + + + + + + + + + + """) + + self.expect("register read cpsr x0", substrs=[ + " cpsr = 0xeeee7777\n" + " = (correct = 1)\n" + " x0 = 0xeeeeeeee77777777\n" + " = (foo = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_repeated_flag_set(self): + # The second definition of "cpsr_flags" should be ignored. + # This is because we assign the types to registers as we go. If we allowed + # the later flag set, it would destroy the first definition, making the + # pointer to the flags invalid. + self.setup_register_test("""\ + + + + + + + + """) + + self.expect("register read cpsr", substrs=["(correct = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_missing_flags(self): + self.setup_register_test("""\ + + """) + + # Register prints with default formatting only if we can't find the + # flags type. + self.expect("register read cpsr", substrs=["cpsr = 0xeeee7777"]) + self.expect("register read cpsr", substrs=["("], matching=False) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_invalid_size(self): + # We're not using the size for anything yet so just check that we handle + # it not being a positive integer. + self.setup_register_test("""\ + + + + + + + + + + + """) + + # Only the final set has a valid size, use that. + self.expect("register read cpsr", substrs=["(C = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_unknown_attribute(self): + # Unknown attributes on flags or field are ignored. + self.setup_register_test("""\ + + + + + """) + + self.expect("register read cpsr", substrs=["(A = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_requried_attributes(self): + # flags must have an id and size so the flags with "C" is the only valid one + # here. + self.setup_register_test("""\ + + + + + + + + + + + """) + + self.expect("register read cpsr", substrs=["(C = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_set_even_if_format_set(self): + # lldb also sends "format". If that is set, we should still read the + # flags type. + self.setup_register_test("""\ + + + + + """) + + self.expect("register read cpsr", substrs=["(B = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_set_even_if_encoding_set(self): + # lldb also sends "encoding". If that is set, we should still read the + # flags type. + self.setup_register_test("""\ + + + + + """) + + self.expect("register read cpsr", substrs=["(B = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_set_even_if_encoding_and_format_set(self): + # As above but both encoding and format are set. + self.setup_register_test("""\ + + + + + """) + + self.expect("register read cpsr", substrs=["(B = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_multiple_lines(self): + # Since we use C types they follow lldb's usual decisions as to whether + # to print them on one line or many. Long field names will usually mean + # many lines. + self.setup_flags_test( + '' + '' + '' + '') + + self.expect("register read cpsr", substrs=[ + " cpsr = 0xeeee7777\n" + " = {\n" + " this_is_a_long_field_3 = 0\n" + " this_is_a_long_field_2 = 1\n" + " this_is_a_long_field_1 = 1\n" + " this_is_a_long_field_0 = 1\n" + " }"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_flags_child_limit(self): + # Flags print like C types so they should follow the child limit setting. + self.runCmd("settings set target.max-children-count 3") + self.setup_flags_test( + '' + '' + '') + + self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_xml_includes(self): + # Certain targets e.g. s390x QEMU split their defintions over multiple + # files that are included into target.xml. + self.setup_multidoc_test({ + # The formatting is very specific here. lldb doesn't like leading + # spaces, and nested tags must be indented more than their parent. + 'target.xml' : dedent("""\ + + + aarch64 + + """), + 'core.xml' : dedent("""\ + + + + + + + + + + """), + }) + + self.expect("register read cpsr", substrs=["(B = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_xml_includes_multiple(self): + self.setup_multidoc_test({ + 'target.xml' : dedent("""\ + + + aarch64 + + + """), + 'core.xml' : dedent("""\ + + + + + + + + """), + 'core-2.xml' : dedent("""\ + + + + + + + + """), + }) + + self.expect("register read x0 cpsr", substrs=["(B = 1)", "(C = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_xml_includes_flags_redefined(self): + self.setup_multidoc_test({ + 'target.xml' : dedent("""\ + + + aarch64 + + + """), + # Treating xi:include as a textual include, my_flags is first defined + # in core.xml. The second definition in core-2.xml + # is ignored. + 'core.xml' : dedent("""\ + + + + + + + + """), + # The my_flags here is ignored, so cpsr will use the my_flags from above. + 'core-2.xml' : dedent("""\ + + + + + + + + """), + }) + + self.expect("register read x0", substrs=["(correct = 1)"]) + self.expect("register read cpsr", substrs=["(correct = 1)"]) diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp index 04edb0c..3819c6f 100644 --- a/lldb/unittests/Target/RegisterFlagsTest.cpp +++ b/lldb/unittests/Target/RegisterFlagsTest.cpp @@ -121,3 +121,19 @@ TEST(RegisterFlagsTest, RegisterFlagsPadding) { make_field(20, 21), make_field(12, 19), make_field(8, 11), make_field(0, 7)}); } + +TEST(RegisterFieldsTest, ReverseFieldOrder) { + // Unchanged + RegisterFlags rf("", 4, {make_field(0, 31)}); + ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678)); + + // Swap the two halves around. + RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)}); + ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678)); + + // Many small fields. + RegisterFlags rf3("", 4, + {make_field(31, 31), make_field(30, 30), make_field(29, 29), + make_field(28, 28)}); + ASSERT_EQ(0x00000005ULL, rf3.ReverseFieldOrder(0xA0000000)); +} diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index d747752..cd35844 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -251,6 +251,12 @@ Changes to LLDB * LLDB is now able to show the subtype of signals found in a core file. For example memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``. +* LLDB can now display register fields if they are described in target XML sent + by a debug server such as ``gdbserver`` (``lldb-server`` does not currently produce + this information). Fields are only printed when reading named registers, for + example ``register read cpsr``. They are not shown when reading a register set, + ``register read -s 0``. + Changes to Sanitizers --------------------- * For Darwin users that override weak symbols, note that the dynamic linker will -- 2.7.4