From 108a8e4c886be9a0a9e1aa53dce8db838b01f4a4 Mon Sep 17 00:00:00 2001 From: Itamar Date: Sat, 10 Dec 2022 20:00:09 +0200 Subject: [PATCH] LibX86: Only pass ProcessorMode to Instruction constructor We previously passed both OperandSize and AddressSize to the constructor. Both values were only ever 32-bit at construction. We used AddressSize::Size64 to signify Long mode which was needlessly complicated. --- .../DevTools/UserspaceEmulator/Emulator.cpp | 6 ++--- Userland/Libraries/LibX86/Disassembler.h | 4 ++-- Userland/Libraries/LibX86/Instruction.h | 23 ++++++++----------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/Userland/DevTools/UserspaceEmulator/Emulator.cpp b/Userland/DevTools/UserspaceEmulator/Emulator.cpp index 4c60a12c5a..e694ad305f 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator.cpp @@ -234,7 +234,7 @@ int Emulator::exec() while (!m_shutdown) { if (m_steps_til_pause) [[likely]] { m_cpu->save_base_eip(); - auto insn = X86::Instruction::from_stream(*m_cpu, X86::OperandSize::Size32, X86::AddressSize::Size32); + auto insn = X86::Instruction::from_stream(*m_cpu, X86::ProcessorMode::Protected); // Exec cycle if constexpr (trace) { outln("{:p} \033[33;1m{}\033[0m", m_cpu->base_eip(), insn.to_deprecated_string(m_cpu->base_eip(), symbol_provider)); @@ -301,7 +301,7 @@ void Emulator::handle_repl() // FIXME: Function names (base, call, jump) auto saved_eip = m_cpu->eip(); m_cpu->save_base_eip(); - auto insn = X86::Instruction::from_stream(*m_cpu, X86::OperandSize::Size32, X86::AddressSize::Size32); + auto insn = X86::Instruction::from_stream(*m_cpu, X86::ProcessorMode::Protected); // FIXME: This does not respect inlining // another way of getting the current function is at need if (auto symbol = symbol_at(m_cpu->base_eip()); symbol.has_value()) { @@ -311,7 +311,7 @@ void Emulator::handle_repl() outln("==> {}", create_instruction_line(m_cpu->base_eip(), insn)); for (int i = 0; i < 7; ++i) { m_cpu->save_base_eip(); - insn = X86::Instruction::from_stream(*m_cpu, X86::OperandSize::Size32, X86::AddressSize::Size32); + insn = X86::Instruction::from_stream(*m_cpu, X86::ProcessorMode::Protected); outln(" {}", create_instruction_line(m_cpu->base_eip(), insn)); } // We don't want to increase EIP here, we just want the instructions diff --git a/Userland/Libraries/LibX86/Disassembler.h b/Userland/Libraries/LibX86/Disassembler.h index c42695135a..9cc2b998ea 100644 --- a/Userland/Libraries/LibX86/Disassembler.h +++ b/Userland/Libraries/LibX86/Disassembler.h @@ -23,10 +23,10 @@ public: if (!m_stream.can_read()) return {}; #if ARCH(I386) - return Instruction::from_stream(m_stream, OperandSize::Size32, AddressSize::Size32); + return Instruction::from_stream(m_stream, ProcessorMode::Protected); #else # if ARCH(X86_64) - return Instruction::from_stream(m_stream, OperandSize::Size32, AddressSize::Size64); + return Instruction::from_stream(m_stream, ProcessorMode::Long); # else dbgln("Unsupported platform"); return {}; diff --git a/Userland/Libraries/LibX86/Instruction.h b/Userland/Libraries/LibX86/Instruction.h index 238e070285..1b86444f43 100644 --- a/Userland/Libraries/LibX86/Instruction.h +++ b/Userland/Libraries/LibX86/Instruction.h @@ -618,7 +618,7 @@ private: class Instruction { public: template - static Instruction from_stream(InstructionStreamType&, OperandSize, AddressSize); + static Instruction from_stream(InstructionStreamType&, ProcessorMode); ~Instruction() = default; ALWAYS_INLINE MemoryOrRegisterReference& modrm() const { return m_modrm; } @@ -699,7 +699,7 @@ public: private: template - Instruction(InstructionStreamType&, OperandSize, AddressSize); + Instruction(InstructionStreamType&, ProcessorMode); void to_deprecated_string_internal(StringBuilder&, u32 origin, SymbolProvider const*, bool x32) const; @@ -961,9 +961,9 @@ ALWAYS_INLINE typename CPU::ValueWithShadowType256 MemoryOrRegisterReference::re } template -ALWAYS_INLINE Instruction Instruction::from_stream(InstructionStreamType& stream, OperandSize operand_size, AddressSize address_size) +ALWAYS_INLINE Instruction Instruction::from_stream(InstructionStreamType& stream, ProcessorMode mode) { - return Instruction(stream, operand_size, address_size); + return Instruction(stream, mode); } ALWAYS_INLINE unsigned Instruction::length() const @@ -1002,19 +1002,14 @@ ALWAYS_INLINE Optional to_segment_prefix(u8 op) } template -ALWAYS_INLINE Instruction::Instruction(InstructionStreamType& stream, OperandSize operand_size, AddressSize address_size) - : m_operand_size(operand_size) - , m_address_size(address_size) +ALWAYS_INLINE Instruction::Instruction(InstructionStreamType& stream, ProcessorMode mode) + : m_mode(mode) { - VERIFY(operand_size != OperandSize::Size64); - // Use address_size as the hint to switch into long mode. + m_operand_size = OperandSize::Size32; // m_address_size refers to the default size of displacements/immediates, which is 32 even in long mode (2.2.1.3 Displacement, 2.2.1.5 Immediates), // with the exception of moffset (see below). - if (address_size == AddressSize::Size64) { - m_operand_size = OperandSize::Size32; - m_address_size = AddressSize::Size32; - m_mode = ProcessorMode::Long; - } + m_address_size = AddressSize::Size32; + u8 prefix_bytes = 0; for (;; ++prefix_bytes) { u8 opbyte = stream.read8();