diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 0411f16fb..c43cd59fc 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -184,8 +184,7 @@ IntT *resolve( // If execution has reached here then a memory fetch is required. // Do it and exit. - const Source segment = pointer.segment(instruction.segment_override()); - return &memory.template access(segment, target_address); + return &memory.template access(instruction.data_segment(), target_address); }; namespace Primitive { @@ -859,7 +858,7 @@ void call_far(InstructionT &instruction, break; } - const Source source_segment = pointer.segment(instruction.segment_override()); + const Source source_segment = instruction.data_segment(); const uint16_t offset = memory.template access(source_segment, source_address); source_address += 2; @@ -891,7 +890,7 @@ void jump_far(InstructionT &instruction, break; } - const Source source_segment = pointer.segment(instruction.segment_override()); + const Source source_segment = instruction.data_segment(); const uint16_t offset = memory.template access(source_segment, source_address); source_address += 2; @@ -932,7 +931,7 @@ void ld( ) { const auto pointer = instruction.source(); auto source_address = address(instruction, pointer, registers, memory); - const Source source_segment = pointer.segment(instruction.segment_override()); + const Source source_segment = instruction.data_segment(); destination = memory.template access(source_segment, source_address); source_address += 2; @@ -959,15 +958,12 @@ void xlat( MemoryT &memory, RegistersT ®isters ) { - Source source_segment = instruction.segment_override(); - if(source_segment == Source::None) source_segment = Source::DS; - AddressT address; if constexpr (std::is_same_v) { address = registers.bx() + registers.al(); } - registers.al() = memory.template access(source_segment, address); + registers.al() = memory.template access(instruction.data_segment(), address); } template @@ -1414,10 +1410,7 @@ void cmps(const InstructionT &instruction, AddressT &eCX, AddressT &eSI, Address return; } - Source source_segment = instruction.segment_override(); - if(source_segment == Source::None) source_segment = Source::DS; - - IntT lhs = memory.template access(source_segment, eSI); + IntT lhs = memory.template access(instruction.data_segment(), eSI); const IntT rhs = memory.template access(Source::ES, eDI); eSI += status.direction() * sizeof(IntT); eDI += status.direction() * sizeof(IntT); @@ -1447,10 +1440,7 @@ void lods(const InstructionT &instruction, AddressT &eCX, AddressT &eSI, IntT &e return; } - Source source_segment = instruction.segment_override(); - if(source_segment == Source::None) source_segment = Source::DS; - - eAX = memory.template access(source_segment, eSI); + eAX = memory.template access(instruction.data_segment(), eSI); eSI += status.direction() * sizeof(IntT); repeat(eCX, flow_controller); @@ -1462,10 +1452,7 @@ void movs(const InstructionT &instruction, AddressT &eCX, AddressT &eSI, Address return; } - Source source_segment = instruction.segment_override(); - if(source_segment == Source::None) source_segment = Source::DS; - - memory.template access(Source::ES, eDI) = memory.template access(source_segment, eSI); + memory.template access(Source::ES, eDI) = memory.template access(instruction.data_segment(), eSI); eSI += status.direction() * sizeof(IntT); eDI += status.direction() * sizeof(IntT); @@ -1491,9 +1478,7 @@ void outs(const InstructionT &instruction, AddressT &eCX, uint16_t port, Address return; } - Source source_segment = instruction.segment_override(); - if(source_segment == Source::None) source_segment = Source::DS; - io.template out(port, memory.template access(source_segment, eSI)); + io.template out(port, memory.template access(instruction.data_segment(), eSI)); eSI += status.direction() * sizeof(IntT); repeat(eCX, flow_controller); diff --git a/InstructionSets/x86/Instruction.cpp b/InstructionSets/x86/Instruction.cpp index 7a7a02863..a911ae560 100644 --- a/InstructionSets/x86/Instruction.cpp +++ b/InstructionSets/x86/Instruction.cpp @@ -425,14 +425,7 @@ std::string InstructionSet::x86::to_string( } stream << '['; - Source segment = instruction.segment_override(); - if(segment == Source::None) { - segment = pointer.default_segment(); - if(segment == Source::None) { - segment = Source::DS; - } - } - stream << InstructionSet::x86::to_string(segment, InstructionSet::x86::DataSize::None) << ':'; + stream << InstructionSet::x86::to_string(instruction.data_segment(), InstructionSet::x86::DataSize::None) << ':'; bool addOffset = false; switch(source) { @@ -492,7 +485,7 @@ std::string InstructionSet::x86::to_string( case Operation::INS_REP: case Operation::OUTS: case Operation::OUTS_REP: - switch(instruction.second.segment_override()) { + switch(instruction.second.data_segment()) { default: break; case Source::ES: operation += "es "; break; case Source::CS: operation += "cs "; break; diff --git a/InstructionSets/x86/Instruction.hpp b/InstructionSets/x86/Instruction.hpp index ba6e28dca..b0fa9ba2b 100644 --- a/InstructionSets/x86/Instruction.hpp +++ b/InstructionSets/x86/Instruction.hpp @@ -644,13 +644,6 @@ class DataPointer { } } - constexpr Source segment(Source segment_override) const { - // TODO: remove conditionality here. - if(segment_override != Source::None) return segment_override; - if(const auto segment = default_segment(); segment != Source::None) return segment; - return Source::DS; - } - constexpr Source base() const { return sib_.base(); } @@ -767,11 +760,9 @@ template class Instruction { return AddressSize(mem_exts_source_ >> 7); } - /// @returns @c Source::None if no segment override was found; the overridden segment otherwise. - /// On x86 a segment override cannot modify the segment used as a destination in string instructions, - /// or that used by stack instructions, but this function does not spend the time necessary to provide - /// the correct default for those. - Source segment_override() const { + /// @returns @c Source::None if no segment is applicable; the segment to use for any + /// memory-accessing source otherwise. + Source data_segment() const { return Source( int(Source::ES) + ((source_data_dest_sib_ >> 10) & 7) @@ -826,7 +817,6 @@ template class Instruction { source_data_dest_sib_(uint16_t( (int(data_size) << 14) | (lock ? (1 << 13) : 0) | - ((int(segment_override)&7) << 10) | ((uint8_t(sib) & 0xf8) << 2) | int(destination) | (destination == Source::Indirect ? (uint8_t(sib) & 7) : 0) @@ -843,6 +833,13 @@ template class Instruction { extensions_[extension] = ImmediateT(displacement); ++extension; } + + // Patch in a fully-resolved segment. + Source segment = segment_override; + if(segment == Source::None) segment = this->source().default_segment(); + if(segment == Source::None) segment = this->destination().default_segment(); + if(segment == Source::None) segment = Source::DS; + source_data_dest_sib_ |= (int(segment)&7) << 10; } }; diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index e13fd750e..7e7f12775 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -419,10 +419,9 @@ struct FailedExecution { // Attempt clerical reconciliation: // - // The test suite retains a distinction between SHL and SAL, which the decoder doesn't. So consider that - // a potential point of difference. - // - // Also, the decoder treats INT3 and INT 3 as the same thing. So allow for a meshing of those. + // * the test suite retains a distinction between SHL and SAL, which the decoder doesn't; + // * the decoder treats INT3 and INT 3 as the same thing; and + // * the decoder doesn't record whether a segment override was present, just the final segment. int adjustment = 7; while(!isEqual && adjustment) { NSString *alteredName = [test[@"name"] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; @@ -433,6 +432,9 @@ struct FailedExecution { if(adjustment & 1) { alteredName = [alteredName stringByReplacingOccurrencesOfString:@"int3" withString:@"int 3h"]; } + if(adjustment & 4) { + alteredName = [@"ds " stringByAppendingString:alteredName]; + } isEqual = compare_decoding(alteredName); --adjustment; diff --git a/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm b/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm index 6502ce841..b246d9383 100644 --- a/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm +++ b/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm @@ -410,7 +410,7 @@ decode(const std::initializer_list &stream, bool set_32_bit = false) { // add DWORD PTR [edi-0x42],0x9f683aa9 // lock jp 0xfffffff0 (from 0000000e) test(instructions[0], DataSize::DWord, Operation::INC, Source::eDX); - XCTAssertEqual(instructions[0].segment_override(), Source::CS); + XCTAssertEqual(instructions[0].data_segment(), Source::CS); test(instructions[1], DataSize::Byte, Operation::OR, Source::Immediate, Source::eAX, 0x9); test(instructions[2], DataSize::DWord, Operation::ADD, Source::Immediate, ScaleIndexBase(Source::eDI), 0x9f683aa9, -0x42); test(instructions[3], Operation::JP, 0, -30); @@ -421,7 +421,7 @@ decode(const std::initializer_list &stream, bool set_32_bit = false) { // stos BYTE PTR es:[edi],al // pusha test(instructions[4], DataSize::Byte, Operation::MOV, Source::Immediate, Source::AH, 0xc1); - XCTAssertEqual(instructions[4].segment_override(), Source::DS); + XCTAssertEqual(instructions[4].data_segment(), Source::DS); test(instructions[5], DataSize::Word, Operation::POP, Source::None, Source::DS); test(instructions[6], DataSize::Byte, Operation::STOS); test(instructions[7], Operation::PUSHA); @@ -464,7 +464,7 @@ decode(const std::initializer_list &stream, bool set_32_bit = false) { test(instructions[21], DataSize::Byte, Operation::XOR, Source::Immediate, Source::eAX, 0x45); test(instructions[22], DataSize::DWord, Operation::LDS, ScaleIndexBase(Source::eCX), Source::eDX); test(instructions[23], DataSize::Byte, Operation::MOV, Source::eAX, Source::DirectAddress, 0xe4dba6d3); - XCTAssertEqual(instructions[23].segment_override(), Source::DS); + XCTAssertEqual(instructions[23].data_segment(), Source::DS); // pop ds // movs DWORD PTR es:[edi],DWORD PTR ds:[esi]