From 787e9e770eb0b6a4eaecad7c0fdcd0ca45ee20b4 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 22 Sep 2023 17:27:27 -0400 Subject: [PATCH] Retain baseless addresses correctly. --- InstructionSets/x86/Decoder.cpp | 14 ++++-- InstructionSets/x86/Instruction.hpp | 2 +- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 48 +++++++++++++------ 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/InstructionSets/x86/Decoder.cpp b/InstructionSets/x86/Decoder.cpp index d1d41184a..af86666a2 100644 --- a/InstructionSets/x86/Decoder.cpp +++ b/InstructionSets/x86/Decoder.cpp @@ -651,7 +651,6 @@ std::pair::InstructionT> Decoder::decode(con data_size(address_size_) }; displacement_size_ = sizes[mod]; - memreg = Source::Indirect; if(address_size_ == AddressSize::b32) { // 32-bit decoding: the range of potential indirections is expanded, @@ -659,21 +658,26 @@ std::pair::InstructionT> Decoder::decode(con sib_ = ScaleIndexBase(0, Source::None, reg_table[rm]); expects_sib = rm == 4; // Indirect via eSP isn't directly supported; it's the // escape indicator for reading a SIB. + memreg = Source::Indirect; } else { // Classic 16-bit decoding: mode picks a displacement size, // and a few fixed index+base pairs are defined. + // + // A base of eAX is meaningless, with the source type being the indicator + // that it should be ignored. ScaleIndexBase can't store a base of Source::None. constexpr ScaleIndexBase rm_table[8] = { ScaleIndexBase(0, Source::eSI, Source::eBX), ScaleIndexBase(0, Source::eDI, Source::eBX), ScaleIndexBase(0, Source::eSI, Source::eBP), ScaleIndexBase(0, Source::eDI, Source::eBP), - ScaleIndexBase(0, Source::eSI, Source::None), - ScaleIndexBase(0, Source::eDI, Source::None), - ScaleIndexBase(0, Source::eBP, Source::None), - ScaleIndexBase(0, Source::eBX, Source::None), + ScaleIndexBase(0, Source::eSI, Source::eAX), + ScaleIndexBase(0, Source::eDI, Source::eAX), + ScaleIndexBase(0, Source::eBP, Source::eAX), + ScaleIndexBase(0, Source::eBX, Source::eAX), }; sib_ = rm_table[rm]; + memreg = rm >= 4 ? Source::IndirectNoBase : Source::Indirect; } } diff --git a/InstructionSets/x86/Instruction.hpp b/InstructionSets/x86/Instruction.hpp index 304ca3374..de8a5c7bb 100644 --- a/InstructionSets/x86/Instruction.hpp +++ b/InstructionSets/x86/Instruction.hpp @@ -521,7 +521,7 @@ class ScaleIndexBase { constexpr ScaleIndexBase(int scale, Source index, Source base) noexcept : sib_(uint8_t( scale << 6 | - (int(index != Source::None ? index : Source::eSI) << 3) | + (int(index != Source::None ? index : Source::eSP) << 3) | int(base) )) {} constexpr ScaleIndexBase(Source index, Source base) noexcept : ScaleIndexBase(0, index, base) {} diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index b4a7edef7..5da9747cd 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -120,7 +120,7 @@ constexpr char TestSuiteHome[] = "/Users/tharte/Projects/ProcessorTests/8088/v1" std::string operand; using Source = InstructionSet::x86::Source; - const Source source = pointer.source(); + const Source source = pointer.source(); switch(source) { // to_string handles all direct register names correctly. default: return InstructionSet::x86::to_string(source, instruction.operation_size()); @@ -132,7 +132,8 @@ constexpr char TestSuiteHome[] = "/Users/tharte/Projects/ProcessorTests/8088/v1" ); case Source::DirectAddress: - case Source::Indirect: { + case Source::Indirect: + case Source::IndirectNoBase: { std::stringstream stream; if(!InstructionSet::x86::mnemonic_implies_data_size(instruction.operation)) { @@ -149,14 +150,24 @@ constexpr char TestSuiteHome[] = "/Users/tharte/Projects/ProcessorTests/8088/v1" stream << InstructionSet::x86::to_string(segment, InstructionSet::x86::DataSize::None) << ':'; stream << '['; - if(source == Source::Indirect) { - stream << InstructionSet::x86::to_string(pointer.base(), data_size(instruction.address_size())); - stream << '+' << InstructionSet::x86::to_string(pointer.index(), data_size(instruction.address_size())); - if(instruction.offset()) { - stream << '+' << to_hex(instruction.offset(), 4); - } - } else { - stream << to_hex(instruction.offset(), 4); + switch(source) { + default: break; + case Source::Indirect: + stream << InstructionSet::x86::to_string(pointer.base(), data_size(instruction.address_size())); + stream << '+' << InstructionSet::x86::to_string(pointer.index(), data_size(instruction.address_size())); + if(instruction.offset()) { + stream << '+' << to_hex(instruction.offset(), 4); + } + break; + case Source::IndirectNoBase: + stream << InstructionSet::x86::to_string(pointer.index(), data_size(instruction.address_size())); + if(instruction.offset()) { + stream << '+' << to_hex(instruction.offset(), 4); + } + break; + case Source::DirectAddress: + stream << to_hex(instruction.offset(), 4); + break; } stream << ']'; return stream.str(); @@ -187,11 +198,18 @@ constexpr char TestSuiteHome[] = "/Users/tharte/Projects/ProcessorTests/8088/v1" log_hex(); // Repeat operand conversions, for debugging. - Decoder().decode(data.data(), data.size()); - const auto destination = decoded.second.destination(); - to_string(destination, decoded.second); - const auto source = decoded.second.source(); - to_string(source, decoded.second); + Decoder decoder; + const auto instruction = decoder.decode(data.data(), data.size()); + const InstructionSet::x86::Source sources[] = { + instruction.second.source().source(), + instruction.second.destination().source(), + }; + (void)sources; + + const auto destination = instruction.second.destination(); + to_string(destination, instruction.second); + const auto source = instruction.second.source(); + to_string(source, instruction.second); return false; }