uweigand created this revision. uweigand added a reviewer: clayborg. uweigand added a subscriber: lldb-commits. Herald added subscribers: rengolin, aemerson.
Running the ARM instruction emulation test on a big-endian system would fail, since the code doesn't respect endianness properly. In EmulateInstructionARM::TestEmulation, code assumes that an instruction opcode read in from the test file is in target byte order, but it was in fact read in in host byte order. More difficult to fix, the EmulationStateARM structure models the overlapping sregs and dregs by a union in _sd_regs. This only works correctly if the host is a little-endian system. I've removed the union in favor of a simple array containing the 32 sregs, and changed any code accessing dregs to explicitly use the correct two sregs overlaying that dreg in the proper target order. Also, the EmulationStateARM::ReadPseudoMemory and WritePseudoMemory track memory as a map of uint32_t values in host byte order, and implement 64-bit memory accessing by splitting them up into two uint32_t ones. However, callers expect memory contents to be provided in the form of a byte array (in target byte order). This means the uint32_t contents need to be byte-swapped on BE systems, and when splitting up a 64-bit access into two 32-bit ones, byte order has to be respected. http://reviews.llvm.org/D18984 Files: source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp source/Plugins/Instruction/ARM/EmulationStateARM.cpp source/Plugins/Instruction/ARM/EmulationStateARM.h
Index: source/Plugins/Instruction/ARM/EmulationStateARM.h =================================================================== --- source/Plugins/Instruction/ARM/EmulationStateARM.h +++ source/Plugins/Instruction/ARM/EmulationStateARM.h @@ -82,11 +82,7 @@ uint32_t m_gpr[17]; struct _sd_regs { - union - { - uint32_t s_reg[2]; - uint64_t d_reg; - } sd_regs[16]; // sregs 0 - 31 & dregs 0 - 15 + uint32_t s_regs[32]; // sregs 0 - 31 & dregs 0 - 15 uint64_t d_regs[16]; // dregs 16-31 Index: source/Plugins/Instruction/ARM/EmulationStateARM.cpp =================================================================== --- source/Plugins/Instruction/ARM/EmulationStateARM.cpp +++ source/Plugins/Instruction/ARM/EmulationStateARM.cpp @@ -61,11 +61,15 @@ if (reg_ctx->ReadRegister (reg_info, reg_value)) { + uint64_t value = reg_value.GetAsUInt64(); uint32_t idx = i - dwarf_d0; if (i < 16) - m_vfp_regs.sd_regs[idx].d_reg = reg_value.GetAsUInt64(); + { + m_vfp_regs.s_regs[idx * 2] = (uint32_t)value; + m_vfp_regs.s_regs[idx * 2 + 1] = (uint32_t)(value >> 32); + } else - m_vfp_regs.d_regs[idx - 16] = reg_value.GetAsUInt64(); + m_vfp_regs.d_regs[idx - 16] = value; } else success = false; @@ -82,16 +86,18 @@ else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31)) { uint32_t idx = reg_num - dwarf_s0; - m_vfp_regs.sd_regs[idx / 2].s_reg[idx % 2] = (uint32_t) value; + m_vfp_regs.s_regs[idx] = (uint32_t)value; } else if ((dwarf_d0 <= reg_num) && (reg_num <= dwarf_d31)) { - if ((reg_num - dwarf_d0) < 16) + uint32_t idx = reg_num - dwarf_d0; + if (idx < 16) { - m_vfp_regs.sd_regs[reg_num - dwarf_d0].d_reg = value; + m_vfp_regs.s_regs[idx * 2] = (uint32_t)value; + m_vfp_regs.s_regs[idx * 2 + 1] = (uint32_t)(value >> 32); } else - m_vfp_regs.d_regs[reg_num - dwarf_d16] = value; + m_vfp_regs.d_regs[idx - 16] = value; } else return false; @@ -110,14 +116,15 @@ else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31)) { uint32_t idx = reg_num - dwarf_s0; - value = m_vfp_regs.sd_regs[idx / 2].s_reg[idx % 2]; + value = m_vfp_regs.d_regs[idx]; } else if ((dwarf_d0 <= reg_num) && (reg_num <= dwarf_d31)) { - if ((reg_num - dwarf_d0) < 16) - value = m_vfp_regs.sd_regs[reg_num - dwarf_d0].d_reg; + uint32_t idx = reg_num - dwarf_d0; + if (idx < 16) + value = (uint64_t)m_vfp_regs.s_regs[idx * 2] | ((uint64_t)m_vfp_regs.s_regs[idx * 2 + 1] >> 32); else - value = m_vfp_regs.d_regs[reg_num - dwarf_d16]; + value = m_vfp_regs.d_regs[idx]; } else success = false; @@ -131,8 +138,8 @@ for (int i = 0; i < 17; ++i) m_gpr[i] = 0; - for (int i = 0; i < 16; ++i) - m_vfp_regs.sd_regs[i].d_reg = 0; + for (int i = 0; i < 32; ++i) + m_vfp_regs.s_regs[i] = 0; for (int i = 0; i < 16; ++i) m_vfp_regs.d_regs[i] = 0; @@ -147,16 +154,7 @@ bool EmulationStateARM::StoreToPseudoAddress (lldb::addr_t p_address, uint64_t value, uint32_t size) { - 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; return true; } @@ -195,6 +193,8 @@ if (!success) return 0; + if (endian::InlHostByteOrder() == lldb::eByteOrderBig) + value = llvm::ByteSwap_32 (value); *((uint32_t *) dst) = value; } else if (length == 8) @@ -207,9 +207,13 @@ if (!success) return 0; - uint64_t value64 = value2; - value64 = (value64 << 32) | value1; - *((uint64_t *) dst) = value64; + if (endian::InlHostByteOrder() == lldb::eByteOrderBig) + { + value1 = llvm::ByteSwap_32 (value1); + value2 = llvm::ByteSwap_32 (value2); + } + ((uint32_t *) dst)[0] = value1; + ((uint32_t *) dst)[1] = value2; } else success = false; @@ -231,13 +235,32 @@ if (!baton) return 0; - bool success; EmulationStateARM *pseudo_state = (EmulationStateARM *) baton; - uint64_t value = *((const uint64_t *) dst); - success = pseudo_state->StoreToPseudoAddress (addr, value, length); - if (success) + + if (length <= 4) + { + uint32_t value = *((const uint32_t *) dst); + if (endian::InlHostByteOrder() == lldb::eByteOrderBig) + value = llvm::ByteSwap_32 (value); + + pseudo_state->StoreToPseudoAddress (addr, value, length); return length; - + } + else if (length == 8) + { + uint32_t value1 = ((const uint32_t *) dst)[0]; + uint32_t value2 = ((const uint32_t *) dst)[1]; + if (endian::InlHostByteOrder() == lldb::eByteOrderBig) + { + value1 = llvm::ByteSwap_32 (value1); + value2 = llvm::ByteSwap_32 (value2); + } + + pseudo_state->StoreToPseudoAddress (addr, value1, 4); + pseudo_state->StoreToPseudoAddress (addr + 4, value2, 4); + return length; + } + return 0; } @@ -289,27 +312,16 @@ match = false; } - for (int i = 0; match && i < 16; ++i) + for (int i = 0; match && i < 32; ++i) { - if (m_vfp_regs.sd_regs[i].s_reg[0] != other_state.m_vfp_regs.sd_regs[i].s_reg[0]) - match = false; - - if (m_vfp_regs.sd_regs[i].s_reg[1] != other_state.m_vfp_regs.sd_regs[i].s_reg[1]) + if (m_vfp_regs.s_regs[i] != other_state.m_vfp_regs.s_regs[i]) match = false; } - for (int i = 0; match && i < 32; ++i) + for (int i = 0; match && i < 16; ++i) { - if (i < 16) - { - if (m_vfp_regs.sd_regs[i].d_reg != other_state.m_vfp_regs.sd_regs[i].d_reg) - match = false; - } - else - { - if (m_vfp_regs.d_regs[i - 16] != other_state.m_vfp_regs.d_regs[i - 16]) - match = false; - } + if (m_vfp_regs.d_regs[i] != other_state.m_vfp_regs.d_regs[i]) + match = false; } return match; Index: source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp =================================================================== --- source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp +++ source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp @@ -13683,14 +13683,14 @@ { m_opcode_mode = eModeThumb; if (test_opcode < 0x10000) - m_opcode.SetOpcode16 (test_opcode, GetByteOrder()); + m_opcode.SetOpcode16 (test_opcode, endian::InlHostByteOrder()); else - m_opcode.SetOpcode32 (test_opcode, GetByteOrder()); + m_opcode.SetOpcode32 (test_opcode, endian::InlHostByteOrder()); } else if (arch.GetTriple().getArch() == llvm::Triple::arm) { m_opcode_mode = eModeARM; - m_opcode.SetOpcode32 (test_opcode, GetByteOrder()); + m_opcode.SetOpcode32 (test_opcode, endian::InlHostByteOrder()); } else {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits