wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
let's try to have tests soon. It seems that the code can be simplified and
tests will be very handy
================
Comment at: lldb/include/lldb/lldb-enumerations.h:976
+ eInstructionControlFlowKindUnknown = 0,
+ /// The instruction is something not listed below, i.e. it's sequential
+ /// instruction that doesn't affect the control flow of the program.
----------------
================
Comment at: lldb/source/Core/Disassembler.cpp:730-740
+/// \param[out] opcode
+/// Primary opcode of the instruction.
+///
+/// \param[out] modrm
+/// ModR/M byte of the instruction.
+/// Bit[7:6] indicates MOD. Bit[5:3] specifies a register and R/M bit[2:0]
+/// may contain a register or specify an addressing mode, depending on MOD.
----------------
I'm noticing that MapOpcodeIntoControlFlowKind only uses PTI_MAP_0 and
PTI_MAP_1, which are opcodes of 1 and 2 bytes. Any opcode of 3 bytes and even
amd3dnow is not used at all. That means that you don't need that enum, so
please delete it. Instead, use the length of the opcode throughout the code.
================
Comment at: lldb/source/Core/Disassembler.cpp:882
+
+ if (m_opcode.GetOpcodeBytes() == nullptr || m_opcode.GetByteSize() <= 0) {
+ return lldb::eInstructionControlFlowKindUnknown;
----------------
does this mean that all x86 and x86_64 instructions are categorized as
Opcode::Type::eTypeBytes?
I'm asking because after reading GetOpcodeBytes, it only returns data if the
type is eTypeBytes. Did you check that?
================
Comment at: lldb/source/Core/Disassembler.cpp:885
+ }
+ memcpy(inst_bytes, m_opcode.GetOpcodeBytes(), m_opcode.GetByteSize());
+
----------------
you don't need to copy GetOpcodeBytes, you can just pass it directly to
InstructionLengthDecode
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128477/new/
https://reviews.llvm.org/D128477
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
