Skip to content

Commit 385f588

Browse files
Fix/disasm: more inline evaluators (LDS, LES, LEA, CALL FAR) (#2094)
* fix: Handle LEA: show effective address, not memory value ExpressionEvaluationService now computes and displays the effective address for LEA instructions instead of reading memory contents. Added AddAddressValue and address expression builders for LEA. Introduced UI tests to verify correct LEA operand evaluation and prevent memory value display. Refactored RegisterToExpression for clarity. * fix: Show memory values for LDS and CALL FAR PTR * refactor: compileValue returns uint not long * fix: address PR review comments - remove unused param, fix null operators, replace non-ASCII chars Agent-Logs-Url: https://github.com/OpenRakis/Spice86/sessions/39c41621-5b19-46ce-999c-061701146122 Co-authored-by: maximilien-noal <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: maximilien-noal <[email protected]>
1 parent b9a8384 commit 385f588

2 files changed

Lines changed: 237 additions & 29 deletions

File tree

src/Spice86/ViewModels/Services/ExpressionEvaluationService.cs

Lines changed: 74 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,32 @@ public ExpressionEvaluationService(State state, IMemory memory) {
4040
Register reg = instruction.GetOpRegister(i);
4141
string? expr = RegisterToExpression(reg);
4242
if (expr != null) {
43-
long value = _compiler.CompileValue(expr)();
43+
uint value = _compiler.CompileValue(expr)();
4444
if (segments.Count > 0) {
4545
AddSeparator(segments);
4646
}
4747
AddRegisterValue(segments, expr.ToUpperInvariant(), value, GetRegisterBitWidth(reg));
4848
}
4949
} else if (kind == OpKind.Memory) {
50-
string? expr = BuildMemoryExpression(instruction);
51-
if (expr != null) {
52-
long value = _compiler.CompileValue(expr)();
53-
if (segments.Count > 0) {
54-
AddSeparator(segments);
50+
bool isLea = instruction.Mnemonic == Mnemonic.Lea;
51+
if (isLea) {
52+
string? addressExpr = BuildAddressExpression(instruction);
53+
if (addressExpr != null) {
54+
uint value = _compiler.CompileValue(addressExpr)();
55+
if (segments.Count > 0) {
56+
AddSeparator(segments);
57+
}
58+
AddAddressValue(segments, value, GetRegisterBitWidth(instruction.GetOpRegister(0)));
59+
}
60+
} else {
61+
string? expr = BuildMemoryExpression(instruction);
62+
if (expr != null) {
63+
uint value = _compiler.CompileValue(expr)();
64+
if (segments.Count > 0) {
65+
AddSeparator(segments);
66+
}
67+
AddMemoryValue(segments, instruction, value, GetMemorySizeBitWidth(instruction.MemorySize));
5568
}
56-
AddMemoryValue(segments, instruction, value, GetMemorySizeBitWidth(instruction.MemorySize));
5769
}
5870
}
5971
// Immediate and branch operands are skipped (already visible in disassembly text)
@@ -72,6 +84,12 @@ private static void AddRegisterValue(List<FormattedTextToken> segments, string n
7284
segments.Add(new FormattedTextToken { Text = FormatHex(value, bitWidth), Kind = FormatterTextKind.Number });
7385
}
7486

87+
private static void AddAddressValue(List<FormattedTextToken> segments, long value, int bitWidth) {
88+
segments.Add(new FormattedTextToken { Text = "addr", Kind = FormatterTextKind.Keyword });
89+
segments.Add(new FormattedTextToken { Text = "=", Kind = FormatterTextKind.Punctuation });
90+
segments.Add(new FormattedTextToken { Text = FormatHex(value, bitWidth), Kind = FormatterTextKind.Number });
91+
}
92+
7593
private static void AddMemoryValue(List<FormattedTextToken> segments, Instruction instruction, long value, int bitWidth) {
7694
segments.Add(new FormattedTextToken { Text = "[", Kind = FormatterTextKind.Punctuation });
7795

@@ -112,14 +130,36 @@ private static void AddMemoryValue(List<FormattedTextToken> segments, Instructio
112130

113131
private static string? RegisterToExpression(Register register) {
114132
return register switch {
115-
Register.AL => "al", Register.CL => "cl", Register.DL => "dl", Register.BL => "bl",
116-
Register.AH => "ah", Register.CH => "ch", Register.DH => "dh", Register.BH => "bh",
117-
Register.AX => "ax", Register.CX => "cx", Register.DX => "dx", Register.BX => "bx",
118-
Register.SP => "sp", Register.BP => "bp", Register.SI => "si", Register.DI => "di",
119-
Register.EAX => "eax", Register.ECX => "ecx", Register.EDX => "edx", Register.EBX => "ebx",
120-
Register.ESP => "esp", Register.EBP => "ebp", Register.ESI => "esi", Register.EDI => "edi",
121-
Register.ES => "es", Register.CS => "cs", Register.SS => "ss", Register.DS => "ds",
122-
Register.FS => "fs", Register.GS => "gs",
133+
Register.AL => "al",
134+
Register.CL => "cl",
135+
Register.DL => "dl",
136+
Register.BL => "bl",
137+
Register.AH => "ah",
138+
Register.CH => "ch",
139+
Register.DH => "dh",
140+
Register.BH => "bh",
141+
Register.AX => "ax",
142+
Register.CX => "cx",
143+
Register.DX => "dx",
144+
Register.BX => "bx",
145+
Register.SP => "sp",
146+
Register.BP => "bp",
147+
Register.SI => "si",
148+
Register.DI => "di",
149+
Register.EAX => "eax",
150+
Register.ECX => "ecx",
151+
Register.EDX => "edx",
152+
Register.EBX => "ebx",
153+
Register.ESP => "esp",
154+
Register.EBP => "ebp",
155+
Register.ESI => "esi",
156+
Register.EDI => "edi",
157+
Register.ES => "es",
158+
Register.CS => "cs",
159+
Register.SS => "ss",
160+
Register.DS => "ds",
161+
Register.FS => "fs",
162+
Register.GS => "gs",
123163
_ => null
124164
};
125165
}
@@ -138,7 +178,7 @@ private static int GetRegisterBitWidth(Register register) {
138178
return size switch {
139179
MemorySize.UInt8 or MemorySize.Int8 => "byte",
140180
MemorySize.UInt16 or MemorySize.Int16 => "word",
141-
MemorySize.UInt32 or MemorySize.Int32 => "dword",
181+
MemorySize.UInt32 or MemorySize.Int32 or MemorySize.SegPtr16 => "dword",
142182
_ => null
143183
};
144184
}
@@ -147,7 +187,7 @@ private static int GetMemorySizeBitWidth(MemorySize size) {
147187
return size switch {
148188
MemorySize.UInt8 or MemorySize.Int8 => 8,
149189
MemorySize.UInt16 or MemorySize.Int16 => 16,
150-
MemorySize.UInt32 or MemorySize.Int32 => 32,
190+
MemorySize.UInt32 or MemorySize.Int32 or MemorySize.SegPtr16 => 32,
151191
_ => 16
152192
};
153193
}
@@ -158,7 +198,23 @@ private static int GetMemorySizeBitWidth(MemorySize size) {
158198
return null;
159199
}
160200

201+
string addressExpr = BuildAddressExpressionCore(instruction);
202+
161203
string? segment = RegisterToExpression(instruction.MemorySegment);
204+
return segment != null
205+
? $"{sizePrefix} ptr {segment}:[{addressExpr}]"
206+
: $"{sizePrefix} ptr [{addressExpr}]";
207+
}
208+
209+
/// <summary>
210+
/// Builds an arithmetic expression for the effective address (offset only, no memory dereference).
211+
/// Used by LEA to compute the address without reading memory.
212+
/// </summary>
213+
private static string? BuildAddressExpression(Instruction instruction) {
214+
return BuildAddressExpressionCore(instruction);
215+
}
216+
217+
private static string BuildAddressExpressionCore(Instruction instruction) {
162218

163219
List<string> addressParts = new();
164220

@@ -179,9 +235,7 @@ private static int GetMemorySizeBitWidth(MemorySize size) {
179235

180236
string addressExpr = string.Join(" + ", addressParts);
181237

182-
return segment != null
183-
? $"{sizePrefix} ptr {segment}:[{addressExpr}]"
184-
: $"{sizePrefix} ptr [{addressExpr}]";
238+
return addressExpr;
185239
}
186240

187241
private static string FormatHex(long value, int bitWidth) {

tests/Spice86.Tests/UI/ExpressionEvaluatorUITests.cs

Lines changed: 163 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void EvaluateOperands_MovAxBx_ShowsBothRegisterValues() {
6868
state.AX = 0x1234;
6969
state.BX = 0x5678;
7070

71-
// x86 16-bit encoding: mov ax, bx 89 D8
71+
// x86 16-bit encoding: mov ax, bx -> 89 D8
7272
byte[] machineCode = [0x89, 0xD8];
7373
SegmentedAddress address = new(0x1000, 0x0100);
7474
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
@@ -80,7 +80,8 @@ public void EvaluateOperands_MovAxBx_ShowsBothRegisterValues() {
8080

8181
// Assert
8282
evaluated.Should().NotBeNullOrEmpty();
83-
string text = SegmentsToText(evaluated!);
83+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
84+
string text = SegmentsToText(evaluatedTokens);
8485
text.Should().Contain("AX=0x1234");
8586
text.Should().Contain("BX=0x5678");
8687

@@ -107,7 +108,7 @@ public void EvaluateOperands_MovAxMemBx_ShowsMemoryValue() {
107108
// Write 0xABCD at physical address DS*16 + BX = 0x20000 + 0x50 = 0x20050
108109
memory.UInt16[0x20050] = 0xABCD;
109110

110-
// x86 16-bit encoding: mov ax, word ptr [bx] 8B 07
111+
// x86 16-bit encoding: mov ax, word ptr [bx] -> 8B 07
111112
byte[] machineCode = [0x8B, 0x07];
112113
SegmentedAddress address = new(0x1000, 0x0100);
113114
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
@@ -119,7 +120,8 @@ public void EvaluateOperands_MovAxMemBx_ShowsMemoryValue() {
119120

120121
// Assert
121122
evaluated.Should().NotBeNullOrEmpty();
122-
string text = SegmentsToText(evaluated!);
123+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
124+
string text = SegmentsToText(evaluatedTokens);
123125
text.Should().Contain("AX=0x0000");
124126
text.Should().Contain("0xABCD");
125127

@@ -130,7 +132,7 @@ public void EvaluateOperands_MovAxMemBx_ShowsMemoryValue() {
130132

131133
/// <summary>
132134
/// Verifies that immediates are NOT evaluated (they are already visible in disassembly text).
133-
/// ASM: mov ax, 0x1234 (opcode B8 34 12) only AX should appear in evaluation.
135+
/// ASM: mov ax, 0x1234 (opcode B8 34 12) - only AX should appear in evaluation.
134136
/// </summary>
135137
[AvaloniaFact]
136138
public void EvaluateOperands_MovAxImm_OnlyShowsDestRegister() {
@@ -140,7 +142,7 @@ public void EvaluateOperands_MovAxImm_OnlyShowsDestRegister() {
140142

141143
state.AX = 0x5555;
142144

143-
// x86 16-bit encoding: mov ax, 0x1234 B8 34 12
145+
// x86 16-bit encoding: mov ax, 0x1234 -> B8 34 12
144146
byte[] machineCode = [0xB8, 0x34, 0x12];
145147
SegmentedAddress address = new(0x1000, 0x0100);
146148
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
@@ -152,7 +154,8 @@ public void EvaluateOperands_MovAxImm_OnlyShowsDestRegister() {
152154

153155
// Assert
154156
evaluated.Should().NotBeNullOrEmpty();
155-
string text = SegmentsToText(evaluated!);
157+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
158+
string text = SegmentsToText(evaluatedTokens);
156159
text.Should().Contain("AX=0x5555");
157160
// The immediate value 0x1234 should NOT appear in evaluation
158161
text.Should().NotContain("=0x1234");
@@ -175,7 +178,7 @@ public void EvaluateOperands_MovAxMemBxDisp_ShowsMemoryValue() {
175178
// Write 0xBEEF at physical address DS*16 + BX + 0x10 = 0x20000 + 0x50 + 0x10 = 0x20060
176179
memory.UInt16[0x20060] = 0xBEEF;
177180

178-
// x86 16-bit encoding: mov ax, word ptr [bx+0x10] 8B 47 10
181+
// x86 16-bit encoding: mov ax, word ptr [bx+0x10] -> 8B 47 10
179182
byte[] machineCode = [0x8B, 0x47, 0x10];
180183
SegmentedAddress address = new(0x1000, 0x0100);
181184
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
@@ -187,11 +190,162 @@ public void EvaluateOperands_MovAxMemBxDisp_ShowsMemoryValue() {
187190

188191
// Assert
189192
evaluated.Should().NotBeNullOrEmpty();
190-
string text = SegmentsToText(evaluated!);
193+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
194+
string text = SegmentsToText(evaluatedTokens);
191195
text.Should().Contain("AX=0x0000");
192196
text.Should().Contain("0xBEEF");
193197

194198
// Verify displacement operator is syntax-colored
195199
evaluated.Should().Contain(s => s.Text == "+" && s.Kind == FormatterTextKind.Operator);
196200
}
201+
202+
/// <summary>
203+
/// Verifies that LEA computes the effective address instead of reading memory.
204+
/// ASM: lea ax, [bp-8] (opcode 8D 46 F8) - should show computed address, not memory contents.
205+
/// </summary>
206+
[AvaloniaFact]
207+
public void EvaluateOperands_LeaAxBpMinus8_ShowsEffectiveAddress() {
208+
// Arrange
209+
State state = CreateState();
210+
(Memory memory, _, _) = CreateMemory();
211+
212+
state.AX = 0x1111;
213+
state.BP = 0x0100;
214+
state.SS = 0x2000;
215+
216+
// Write decoy data at effective address - LEA must NOT read this
217+
uint physicalAddress = (uint)(state.SS * 16 + state.BP - 8);
218+
memory.UInt16[physicalAddress] = 0xDEAD;
219+
220+
// x86 16-bit encoding: lea ax, [bp-8] -> 8D 46 F8
221+
byte[] machineCode = [0x8D, 0x46, 0xF8];
222+
SegmentedAddress address = new(0x1000, 0x0100);
223+
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
224+
225+
ExpressionEvaluationService service = new(state, memory);
226+
227+
// Act
228+
List<FormattedTextToken>? evaluated = service.FormatOperandValues(line.InstructionInfo);
229+
230+
// Assert
231+
evaluated.Should().NotBeNullOrEmpty();
232+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
233+
string text = SegmentsToText(evaluatedTokens);
234+
235+
// LEA should show effective address = BP - 8 = 0x0100 - 8 = 0x00F8
236+
text.Should().Contain("0x00F8");
237+
// LEA must NOT show the memory contents 0xDEAD
238+
text.Should().NotContain("DEAD");
239+
// AX register value should still be shown
240+
text.Should().Contain("AX=0x1111");
241+
}
242+
243+
/// <summary>
244+
/// Verifies LEA with base+index computes the effective address.
245+
/// ASM: lea ax, [bx+si] (opcode 8D 00)
246+
/// </summary>
247+
[AvaloniaFact]
248+
public void EvaluateOperands_LeaAxBxSi_ShowsEffectiveAddress() {
249+
// Arrange
250+
State state = CreateState();
251+
(Memory memory, _, _) = CreateMemory();
252+
253+
state.AX = 0x0000;
254+
state.BX = 0x0030;
255+
state.SI = 0x0010;
256+
state.DS = 0x2000;
257+
258+
// x86 16-bit encoding: lea ax, [bx+si] -> 8D 00
259+
byte[] machineCode = [0x8D, 0x00];
260+
SegmentedAddress address = new(0x1000, 0x0100);
261+
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
262+
263+
ExpressionEvaluationService service = new(state, memory);
264+
265+
// Act
266+
List<FormattedTextToken>? evaluated = service.FormatOperandValues(line.InstructionInfo);
267+
268+
// Assert
269+
evaluated.Should().NotBeNullOrEmpty();
270+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
271+
string text = SegmentsToText(evaluatedTokens);
272+
// Effective address = BX + SI = 0x0030 + 0x0010 = 0x0040
273+
text.Should().Contain("0x0040");
274+
}
275+
276+
/// <summary>
277+
/// Verifies that LDS evaluates the far pointer memory operand (dword containing offset:segment).
278+
/// ASM: lds si, ss:[bp+0x10] (opcode C5 76 10) - should show the dword value at memory.
279+
/// </summary>
280+
[AvaloniaFact]
281+
public void EvaluateOperands_LdsSiBpPlus10_ShowsFarPointerValue() {
282+
// Arrange
283+
State state = CreateState();
284+
(Memory memory, _, _) = CreateMemory();
285+
286+
state.SI = 0x0000;
287+
state.BP = 0x0984;
288+
state.SS = 0x2F23;
289+
state.DS = 0x261F;
290+
291+
// Write a far pointer (offset:segment) at SS:BP+0x10
292+
uint physicalAddress = (uint)(state.SS * 16 + state.BP + 0x10);
293+
memory.UInt16[physicalAddress] = 0x1234; // offset
294+
memory.UInt16[physicalAddress + 2] = 0x5678; // segment
295+
296+
// x86 16-bit encoding: lds si, [bp+0x10] -> C5 76 10
297+
byte[] machineCode = [0xC5, 0x76, 0x10];
298+
SegmentedAddress address = new(0x1000, 0x0100);
299+
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
300+
301+
ExpressionEvaluationService service = new(state, memory);
302+
303+
// Act
304+
List<FormattedTextToken>? evaluated = service.FormatOperandValues(line.InstructionInfo);
305+
306+
// Assert
307+
evaluated.Should().NotBeNullOrEmpty();
308+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
309+
string text = SegmentsToText(evaluatedTokens);
310+
// Should show the dword value read from memory (segment:offset packed as dword)
311+
text.Should().Contain("0x56781234");
312+
// SI register value should also be shown
313+
text.Should().Contain("SI=0x0000");
314+
}
315+
316+
/// <summary>
317+
/// Verifies that CALL dword ptr [mem] evaluates the far pointer memory operand.
318+
/// ASM: call dword ptr ss:[bp-4] (opcode FF 5E FC) - should show the target address.
319+
/// </summary>
320+
[AvaloniaFact]
321+
public void EvaluateOperands_CallFarPtrBpMinus4_ShowsFarPointerValue() {
322+
// Arrange
323+
State state = CreateState();
324+
(Memory memory, _, _) = CreateMemory();
325+
326+
state.BP = 0x0984;
327+
state.SS = 0x2F23;
328+
329+
// Write a far pointer (offset:segment) at SS:BP-4
330+
uint physicalAddress = (uint)(state.SS * 16 + state.BP - 4);
331+
memory.UInt16[physicalAddress] = 0xABCD; // offset
332+
memory.UInt16[physicalAddress + 2] = 0x1234; // segment
333+
334+
// x86 16-bit encoding: call dword ptr [bp-4] -> FF 5E FC
335+
byte[] machineCode = [0xFF, 0x5E, 0xFC];
336+
SegmentedAddress address = new(0x1000, 0x0100);
337+
DebuggerLineViewModel line = CreateDebuggerLine(machineCode, address);
338+
339+
ExpressionEvaluationService service = new(state, memory);
340+
341+
// Act
342+
List<FormattedTextToken>? evaluated = service.FormatOperandValues(line.InstructionInfo);
343+
344+
// Assert
345+
evaluated.Should().NotBeNullOrEmpty();
346+
List<FormattedTextToken> evaluatedTokens = evaluated ?? [];
347+
string text = SegmentsToText(evaluatedTokens);
348+
// Should show the dword value read from memory
349+
text.Should().Contain("0x1234ABCD");
350+
}
197351
}

0 commit comments

Comments
 (0)