clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
A few comments:
- we need the DWARF opcode length in RegisterInfo, but not in the structured
data (XML or JSON)
- all macros that initialize RegisterInfo structs must be expended to null and
zero out the opcode pointer and size (in all register contexts for all archs)
- remove all virtual and overrides for GetDwarfOpcodeLength()
================
Comment at: include/lldb/Host/common/NativeRegisterContext.h:90-92
@@ -89,2 +89,5 @@
+ virtual size_t
+ GetDwarfOpcodeLength (const uint32_t i) const;
+
virtual uint32_t
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/Host/common/NativeRegisterContextRegisterInfo.h:40-42
@@ -39,2 +39,5 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
const RegisterInfoInterface&
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/Target/RegisterContext.h:53
@@ +52,3 @@
+ RegisterInfo* reg_info,
+ size_t opcode_len);
+
----------------
Remove opcode_len, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/lldb-private-types.h:57-58
@@ -56,2 +56,4 @@
// ax, ah, and al.
+ const uint8_t *dynamic_size_dwarf_expr_bytes; // A DWARF expression
that when evaluated gives
+ // the byte size of this
register.
};
----------------
So we either remove both fields from RegisterInfo, or we leave both in. If we
remove them both, then getting the register info from the RegisterContext will
update the register info automatically if needed as determined by the
RegisterContext subclass. I don't mind them being in RegisterInfo and I think
both should stay. In this case, we do need the length for the expression in the
RegisterInfo struct, we just don't need to *send* the length in the structured
data (XML or JSON).
================
Comment at: source/Host/common/NativeRegisterContext.cpp:273-278
@@ -272,2 +272,8 @@
+size_t
+NativeRegisterContext::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ return 0;
+}
+
uint32_t
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: source/Host/common/NativeRegisterContextRegisterInfo.cpp:52-58
@@ +51,8 @@
+
+size_t
+NativeRegisterContextRegisterInfo::GetDwarfOpcodeLength (const uint32_t i)
const
+{
+ const RegisterInfoInterface& reg_info_interface = GetRegisterInfoInterface
();
+ return reg_info_interface.GetDwarfOpcodeLength (i);
+}
+
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:317
@@ +316,3 @@
+ // Update the actual location
+ reg_info.dynamic_size_dwarf_expr_bytes =
m_dynamic_reg_size_map[i].data ();
+ }
----------------
set the length in RegisterInfo from the size of the bytes extracted.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:320
@@ +319,3 @@
+ else
+ reg_info.dynamic_size_dwarf_expr_bytes = NULL;
+ }
----------------
set the opcode length to zero here.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:424-428
@@ -396,2 +423,7 @@
+size_t
+DynamicRegisterInfo::GetDwarfOpcodeLength (uint32_t i)
+{
+ return m_dynamic_reg_size_map[i].size();
+}
----------------
remove.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:435
@@ -403,1 +434,3 @@
+ ConstString &set_name,
+ uint32_t dwarf_expr_bytes_len)
{
----------------
remove dwarf_expr_bytes_len.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.h:43
@@ -43,1 +42,3 @@
+ lldb_private::ConstString &set_name,
+ uint32_t dwarf_expr_bytes_len = 0);
----------------
Remove the dwarf_expr_bytes_len, the length needs to stay in the RegisterInfo.
If the opcode and length are non-zero, then a copy of the data will need to be
made so it can persist. We already do this with the value_regs and
invalidate_regs fields in RegisterInfo.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.h:57-59
@@ -55,1 +56,5 @@
+ // Get length of the Dwarf Opcode
+ size_t
+ GetDwarfOpcodeLength (uint32_t i);
+
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips.cpp:72-84
@@ -71,2 +71,15 @@
+size_t
+RegisterContextLinux_mips::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ if (i < GetRegisterCount ())
+ {
+ const RegisterInfo *reg_info = GetRegisterInfo() + i;
+
+ if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes)
+ return sizeof(dwarf_opcode_mips);
+ }
+ return 0;
+}
+
uint32_t
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips.h:34-35
@@ -33,1 +33,4 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
----------------
remove
================
Comment at:
source/Plugins/Process/Utility/RegisterContextLinux_mips64.cpp:124-136
@@ -123,2 +123,15 @@
+size_t
+RegisterContextLinux_mips64::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ if (i < GetRegisterCount ())
+ {
+ const RegisterInfo *reg_info = GetRegisterInfo() + i;
+
+ if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes)
+ return sizeof(dwarf_opcode_mips64);
+ }
+ return 0;
+}
+
uint32_t
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips64.h:36-38
@@ -35,2 +35,5 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
private:
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterInfoInterface.h:51-56
@@ -50,2 +50,8 @@
+ virtual size_t
+ GetDwarfOpcodeLength (const uint32_t i) const
+ {
+ return 0;
+ }
+
const lldb_private::ArchSpec&
----------------
remove
https://reviews.llvm.org/D20357
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
