[Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
Tamas Berghammer via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 12 09:24:44 PDT 2016
tberghammer added a comment.
Generally looks good with 2 minor comment inline. I also run the test suite on Android ARM (little endian) and everything looked fine
Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:127
@@ -119,3 +126,3 @@
- value = m_vfp_regs.d_regs[reg_num - dwarf_d16];
+ value = m_vfp_regs.d_regs[idx];
I think you need "idx - 16" here
Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:157
@@ -149,12 +156,3 @@
- if (size > 8)
- return false;
- if (size <= 4)
- m_memory[p_address] = value;
- else if (size == 8)
- m_memory[p_address] = (value << 32) >> 32;
- m_memory[p_address + 4] = value << 32;
+ m_memory[p_address] = value;
> m_memory is a map with map type uint32? I think we will end up loosing data here if its larger than 4 bytes. if StoreToPseudoAddress isnt used elsewhere better change value from uint64 to uint32 ? Also size seems to be a redundant argument now.
All caller of the function ensures size<=4 so I think we should change value to a uint32_t and remove size as it is not used anymore.
More information about the lldb-commits