clayborg added a comment.
A few things about a the RegisterContext class in case it would change and
thing that you are submitting here. The entire register context defines a
buffer of bytes that can contain all register values. Each RegisterInfo
contains the offset for the value of this register in this buffer. This buffer
is said to have a specific byte order (big, little, etc). When a register is
read, it should place its bytes into the RegisterContext's buffer of bytes and
mark itself as being valid in the register context. Some platforms read
multiple registers at a time (they don't have a "read one register value", they
just have "read all GPR registers") and lets say you are reading one GPR, and
this causes all GPR values to be read, then all bytes from all GPR values will
be copied into the register context data buffer and all GPRs should be marked
as valid. So to get a RegisterValue for a 32 bit register, we normally will
just ask the RegisterInfo for the offset of the register, and then extract the
bytes from the buffer using a DataExtractor object. If you have a 64 bit
register whose value also contains a 32 bit pseudo register (like rax contains
eax on x86), then you should have a RegisterInfo defined for "rax" that says
its offset is N, and for a big endian system, you would say that the register
offset for "eax" is N + 4. Extracting the value simply becomes extracting the
bytes from the buffer without the need for any tricks. After reading all of
this, do you still believe you have the right fix in here? It doesn't seem like
you ever should need to use DataExtractor::CopyByteOrderedData???
================
Comment at: source/Core/DataExtractor.cpp:949-958
@@ +948,12 @@
+ // Validate the source and destination info
+ assert(dst_void_ptr != nullptr || src_void_ptr != nullptr);
+ assert(dst_len > 0 || src_len > 0);
+ assert(src_len <= dst_len);
+ assert(dst_byte_order == eByteOrderBig || dst_byte_order ==
eByteOrderLittle);
+ assert(src_byte_order == eByteOrderBig || src_byte_order ==
eByteOrderLittle);
+
+ // Validate that only a word- or register-sized dst is byte swapped
+ assert(dst_byte_order == src_byte_order || dst_len == 1 || dst_len == 2 ||
+ dst_len == 4 || dst_len == 8 || dst_len == 10 || dst_len == 16 ||
+ dst_len == 32);
+
----------------
change all assert calls to lldbassert so they don't crash running program and
add if statements that return if any of the assertion fails. We can't just
crash when we are unhappy with function input. I know llvm and clang do this,
but it has caused way too many crashes for LLDB.
================
Comment at: source/Core/DataExtractor.cpp:1024-1026
@@ -942,5 +1023,5 @@
lldb::offset_t
DataExtractor::CopyByteOrderedData(offset_t src_offset, offset_t src_len,
void *dst_void_ptr, offset_t dst_len,
ByteOrder dst_byte_order) const {
// Validate the source info
----------------
Should probably have this call the above new function so we don't duplicate
functionality.
https://reviews.llvm.org/D24124
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
