From ebc4dc1147efe9daee48996efef2fab44ebcd1ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A4ll=C3=A9n?= Date: Wed, 18 Sep 2024 13:51:13 +0200 Subject: [PATCH] Refactor: remove redundant SPARC IndexedMemoryOperand Keeping a single memory operand class simplifies the SPARC rewriter. --- src/Arch/Sparc/MemoryOperand.cs | 49 ++++++++++--------- src/Arch/Sparc/SparcDisassembler.cs | 6 +-- src/Arch/Sparc/SparcInstructionComparer.cs | 19 ++++--- src/Arch/Sparc/SparcRewriter.cs | 20 ++++---- src/Arch/Telink/TC32Architecture.cs | 2 +- src/Arch/X86/X86ProcessorArchitecture.cs | 2 +- src/Core/Machine/InstructionComparer.cs | 8 +-- src/Gui/TextViewing/DisassemblyFormatter.cs | 10 ++-- .../AvaloniaUI/Controls/TextSpan.cs | 2 +- .../ViewModels/AvaloniaTextSpanFactory.cs | 4 +- subjects/regression.log | 14 +++--- 11 files changed, 70 insertions(+), 66 deletions(-) diff --git a/src/Arch/Sparc/MemoryOperand.cs b/src/Arch/Sparc/MemoryOperand.cs index f663493bfb..22f8618c70 100644 --- a/src/Arch/Sparc/MemoryOperand.cs +++ b/src/Arch/Sparc/MemoryOperand.cs @@ -24,6 +24,7 @@ using Reko.Core.Types; using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Text; @@ -31,41 +32,45 @@ namespace Reko.Arch.Sparc { public class MemoryOperand : AbstractMachineOperand { - public readonly RegisterStorage Base; - public readonly Constant Offset; - - public MemoryOperand(RegisterStorage b, Constant offset, PrimitiveType width) : base(width) + private MemoryOperand(RegisterStorage b, Constant? offset, RegisterStorage? index, PrimitiveType width) : base(width) { this.Base = b; this.Offset = offset; + this.Index = index; } - protected override void DoRender(MachineInstructionRenderer renderer, MachineInstructionRendererOptions options) + public static MemoryOperand Indirect(RegisterStorage baseRg, Constant offset, PrimitiveType dt) { - renderer.WriteFormat("[%{0}", Base.Name); - if (!Offset.IsNegative) - { - renderer.WriteString("+"); - } - renderer.WriteString(Offset.ToInt16().ToString()); - renderer.WriteString("]"); + return new MemoryOperand(baseRg, offset, null, dt); } - } - - public class IndexedMemoryOperand : AbstractMachineOperand - { - public readonly RegisterStorage Base; - public readonly RegisterStorage Index; - public IndexedMemoryOperand(RegisterStorage r1, RegisterStorage r2, PrimitiveType width) : base(width) + public static MemoryOperand Indexed(RegisterStorage baseReg, RegisterStorage index, PrimitiveType dt) { - this.Base = r1; - this.Index = r2; + return new MemoryOperand(baseReg, null, index, dt); } + public RegisterStorage Base { get; } + public Constant? Offset { get; } + public RegisterStorage? Index { get; } + + protected override void DoRender(MachineInstructionRenderer renderer, MachineInstructionRendererOptions options) { - renderer.WriteFormat("[%{0}+%{1}]", Base, Index); + renderer.WriteFormat("[%{0}", Base.Name); + if (Offset is not null) + { + if (!Offset.IsNegative) + { + renderer.WriteString("+"); + } + renderer.WriteString(Offset.ToInt16().ToString()); + } + else + { + Debug.Assert(Index is not null); + renderer.WriteFormat("+%{0}", Index); + } + renderer.WriteString("]"); } } } diff --git a/src/Arch/Sparc/SparcDisassembler.cs b/src/Arch/Sparc/SparcDisassembler.cs index 0d74838583..45c082bfc4 100644 --- a/src/Arch/Sparc/SparcDisassembler.cs +++ b/src/Arch/Sparc/SparcDisassembler.cs @@ -366,7 +366,7 @@ private static MachineOperand GetAlternateSpaceOperand(Registers registers, uint RegisterStorage b = registers.GetRegister(wInstr >> 14); RegisterStorage idx = registers.GetRegister(wInstr); var asi = (wInstr >> 4) & 0xFF; - return new MemoryOperand(b, Constant.Int32((int) asi), type); + return MemoryOperand.Indirect(b, Constant.Int32((int) asi), type); } private static MachineOperand GetMemoryOperand(Registers registers, uint wInstr, PrimitiveType type) @@ -374,12 +374,12 @@ private static MachineOperand GetMemoryOperand(Registers registers, uint wInstr, RegisterStorage b = registers.GetRegister(wInstr >> 14); if ((wInstr & (1 << 13)) != 0) { - return new MemoryOperand(b, Constant.Int32(SignExtend(wInstr, 13)), type); + return MemoryOperand.Indirect(b, Constant.Int32(SignExtend(wInstr, 13)), type); } else { RegisterStorage idx = registers.GetRegister(wInstr); - return new IndexedMemoryOperand(b, idx, type); + return MemoryOperand.Indexed(b, idx, type); } } diff --git a/src/Arch/Sparc/SparcInstructionComparer.cs b/src/Arch/Sparc/SparcInstructionComparer.cs index 86eed5ad4a..9ee9d345cd 100644 --- a/src/Arch/Sparc/SparcInstructionComparer.cs +++ b/src/Arch/Sparc/SparcInstructionComparer.cs @@ -61,12 +61,14 @@ private bool CompareOperands(MachineOperand a, MachineOperand b) var mB = (MemoryOperand) b; if (!CompareRegisters(mA.Base, mB.Base)) return false; - return CompareValues(mA.Offset, mB.Offset); - case IndexedMemoryOperand xA: - var xB = (IndexedMemoryOperand) b; - if (!CompareRegisters(xA.Base, xB.Base)) - return false; - return CompareRegisters(xA.Index, xB.Index); + if (mA.Offset is not null) + { + return CompareValues(mA.Offset, mB.Offset); + } + else + { + return CompareRegisters(mA.Index, mB.Index); + } } throw new NotImplementedException(); } @@ -95,10 +97,7 @@ private int GetOperandHash(MachineOperand op) case MemoryOperand m: var h = GetRegisterHash(m.Base); h = h ^ 29 * GetConstantHash(m.Offset); - return h; - case IndexedMemoryOperand x: - h = GetRegisterHash(x.Base); - h = h ^ 59 * GetRegisterHash(x.Index); + h = h ^ 59 * GetRegisterHash(m.Index); return h; } throw new NotImplementedException(); diff --git a/src/Arch/Sparc/SparcRewriter.cs b/src/Arch/Sparc/SparcRewriter.cs index a8e1b954d3..2a3a3a514d 100644 --- a/src/Arch/Sparc/SparcRewriter.cs +++ b/src/Arch/Sparc/SparcRewriter.cs @@ -30,6 +30,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; namespace Reko.Arch.Sparc { @@ -358,31 +359,28 @@ private Expression RewriteMemOp(MachineOperand op, DataType size) { Expression? baseReg; Expression? offset; - if (op is MemoryOperand mem) + var mem = (MemoryOperand) op; + if (mem.Offset is not null) { baseReg = mem.Base == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Base); offset = mem.Offset.IsIntegerZero ? null : mem.Offset; } else { - if (op is IndexedMemoryOperand i) - { - baseReg = i.Base == arch.Registers.g0 ? null : binder.EnsureRegister(i.Base); - offset = i.Index == arch.Registers.g0 ? null : binder.EnsureRegister(i.Index); - } - else - throw new NotImplementedException(string.Format("Unknown memory operand {0} ({1})", op, op.GetType().Name)); + Debug.Assert(mem.Index is not null); + baseReg = mem.Base == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Base); + offset = mem.Index == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Index); } return m.Mem(size, SimplifySum(baseReg, offset)); } private Expression SimplifySum(Expression? srcLeft, Expression? srcRight) { - if (srcLeft == null && srcRight == null) + if (srcLeft is null && srcRight is null) return Constant.Zero(PrimitiveType.Ptr32); - else if (srcLeft == null) + else if (srcLeft is null) return srcRight!; - else if (srcRight == null) + else if (srcRight is null) return srcLeft; else return m.IAdd(srcLeft, srcRight); diff --git a/src/Arch/Telink/TC32Architecture.cs b/src/Arch/Telink/TC32Architecture.cs index 7b8f2590ee..43df3d5718 100644 --- a/src/Arch/Telink/TC32Architecture.cs +++ b/src/Arch/Telink/TC32Architecture.cs @@ -43,7 +43,7 @@ public TC32Architecture(IServiceProvider services, string archId, Dictionary options) diff --git a/src/Core/Machine/InstructionComparer.cs b/src/Core/Machine/InstructionComparer.cs index d0fd5854c8..e63dbc4235 100644 --- a/src/Core/Machine/InstructionComparer.cs +++ b/src/Core/Machine/InstructionComparer.cs @@ -74,16 +74,16 @@ public virtual bool Equals(MachineInstruction? x, MachineInstruction? y) return CompareOperands(x, y); } - public int GetConstantHash(Constant c) + public int GetConstantHash(Constant? c) { - if ((norm & Normalize.Constants) != 0) + if ((norm & Normalize.Constants) != 0 || c is null) return 0; return c.GetValue().GetHashCode(); } - public int GetRegisterHash(RegisterStorage r) + public int GetRegisterHash(RegisterStorage? r) { - if ((norm & Normalize.Registers) != 0) + if ((norm & Normalize.Registers) != 0 || r is null) return 0; return r.Number.GetHashCode(); } diff --git a/src/Gui/TextViewing/DisassemblyFormatter.cs b/src/Gui/TextViewing/DisassemblyFormatter.cs index b033946561..b19889fca1 100644 --- a/src/Gui/TextViewing/DisassemblyFormatter.cs +++ b/src/Gui/TextViewing/DisassemblyFormatter.cs @@ -53,6 +53,7 @@ public DisassemblyFormatter( this.arch = arch; this.instr = instr; this.line = line; + this.addrInstr = default!; this.annotations = new List(); this.mnemonicStyle = Gui.Services.UiStyles.DisassemblerOpcode; } @@ -130,7 +131,7 @@ public void WriteUInt32(uint n) sb.Append(n); } - public void WriteString(string s) + public void WriteString(string? s) { sb.Append(s); } @@ -140,9 +141,12 @@ public void WriteFormat(string fmt, params object[] parms) sb.AppendFormat(fmt, parms); } - public void AddAnnotation(string annotation) + public void AddAnnotation(string? annotation) { - this.annotations.Add(annotation); + if (annotation is not null) + { + this.annotations.Add(annotation); + } } public void NewLine() diff --git a/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs b/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs index fbb38bbfc1..283d3fca84 100644 --- a/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs +++ b/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs @@ -38,7 +38,7 @@ protected TextSpan() } public abstract string GetText(); - public string Style { get; set; } + public string? Style { get; set; } public object? tag; public object? Tag { diff --git a/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs b/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs index c2b069e9e1..9a05b0ea97 100644 --- a/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs +++ b/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs @@ -114,7 +114,7 @@ public class InertTextSpan : TextSpan { private readonly string text; - public InertTextSpan(string text, string style) + public InertTextSpan(string text, string? style) { this.text = text; base.Style = style; @@ -169,7 +169,7 @@ public class MemoryTextSpan : TextSpan public Address Address { get; private set; } - public MemoryTextSpan(string text, string style) + public MemoryTextSpan(string text, string? style) { this.text = text; base.Style = style; diff --git a/subjects/regression.log b/subjects/regression.log index e7fe71cdd2..c65232e5dd 100644 --- a/subjects/regression.log +++ b/subjects/regression.log @@ -497,7 +497,7 @@ fn000ECBDB: error: An internal error occurred while building the expressions of at Reko.Analysis.Coalescer.Worker.CoalesceStatements(SsaIdentifier sid, Expression defExpr, Statement def, Statement use) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 152 at Reko.Analysis.Coalescer.Worker.Process(Block block) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 208 at Reko.Analysis.Coalescer.Worker.Transform() in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 233 - at Reko.Analysis.Coalescer.Transform(SsaState ssa) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 57 + at Reko.Analysis.Coalescer.Transform(SsaState ssa) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 58 at Reko.Analysis.DataFlowAnalysis.BuildExpressionTrees(IReadOnlyCollection`1 ssts) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\DataFlowAnalysis.cs:line 247 fn000F4A65: error: An internal error occurred while building the expressions of fn000F4A65 Argument size mismatch between real64 and word32. @@ -507,7 +507,7 @@ fn000F4A65: error: An internal error occurred while building the expressions of at Reko.Analysis.Coalescer.Worker.CoalesceStatements(SsaIdentifier sid, Expression defExpr, Statement def, Statement use) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 152 at Reko.Analysis.Coalescer.Worker.Process(Block block) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 208 at Reko.Analysis.Coalescer.Worker.Transform() in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 233 - at Reko.Analysis.Coalescer.Transform(SsaState ssa) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 57 + at Reko.Analysis.Coalescer.Transform(SsaState ssa) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\Coalescer.cs:line 58 at Reko.Analysis.DataFlowAnalysis.BuildExpressionTrees(IReadOnlyCollection`1 ssts) in c:\dev\uxmal\reko\master\src\Decompiler\Analysis\DataFlowAnalysis.cs:line 247 fn00116E5E: error: An internal error occurred while building the expressions of fn00116E5E The given key 'l00117142' was not present in the dictionary. @@ -1339,9 +1339,7 @@ fn0001B1D0: warning: Structure analysis stopped making progress, quitting. Pleas fn0001B1F8: warning: Structure analysis stopped making progress, quitting. Please report this issue at https://github.com/uxmal/reko fn0001B2AC: error: An error occurred while rewriting procedure to high-level language. Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index') - at System.Collections.Generic.List`1.get_Item(Int32 index) - at Reko.Core.Block.get_ThenBlock() in c:\dev\uxmal\reko\master\src\Core\Block.cs:line 92 - at Reko.Structure.CompoundConditionCoalescer.MaybeCoalesce(Block block) in c:\dev\uxmal\reko\master\src\Decompiler\Structure\CompoundConditionCoalescer.cs:line 96 + at Reko.Structure.CompoundConditionCoalescer.MaybeCoalesce(Block block) in c:\dev\uxmal\reko\master\src\Decompiler\Structure\CompoundConditionCoalescer.cs:line 95 at Reko.Structure.CompoundConditionCoalescer.Transform() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\CompoundConditionCoalescer.cs:line 62 at Reko.Structure.StructureAnalysis.Structure() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\StructureAnalysis.cs:line 78 at Reko.Decompiler.StructureProgram() in c:\dev\uxmal\reko\master\src\Decompiler\Decompiler.cs:line 592 @@ -2478,7 +2476,7 @@ fn00008EC2: error: An error occurred while rewriting procedure to high-level lan The given key 'l00009735' was not present in the dictionary. at System.Collections.Generic.Dictionary`2.get_Item(TKey key) at Reko.Core.Graphs.DominatorGraph`1.BuildDominanceFrontiers(DirectedGraph`1 graph, Dictionary`2 idoms) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 193 - at Reko.Core.Graphs.DominatorGraph`1..ctor(DirectedGraph`1 graph, T entryNode) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 46 + at Reko.Core.Graphs.DominatorGraph`1..ctor(DirectedGraph`1 graph, T entryNode) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 47 at Reko.Structure.StructureAnalysis.Execute() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\StructureAnalysis.cs:line 147 at Reko.Structure.StructureAnalysis.Structure() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\StructureAnalysis.cs:line 80 at Reko.Decompiler.StructureProgram() in c:\dev\uxmal\reko\master\src\Decompiler\Decompiler.cs:line 592 @@ -2486,7 +2484,7 @@ fn00009746: error: An error occurred while rewriting procedure to high-level lan The given key 'l0000985B' was not present in the dictionary. at System.Collections.Generic.Dictionary`2.get_Item(TKey key) at Reko.Core.Graphs.DominatorGraph`1.BuildDominanceFrontiers(DirectedGraph`1 graph, Dictionary`2 idoms) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 193 - at Reko.Core.Graphs.DominatorGraph`1..ctor(DirectedGraph`1 graph, T entryNode) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 46 + at Reko.Core.Graphs.DominatorGraph`1..ctor(DirectedGraph`1 graph, T entryNode) in c:\dev\uxmal\reko\master\src\Core\Graphs\DominatorGraph.cs:line 47 at Reko.Structure.StructureAnalysis.Execute() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\StructureAnalysis.cs:line 147 at Reko.Structure.StructureAnalysis.Structure() in c:\dev\uxmal\reko\master\src\Decompiler\Structure\StructureAnalysis.cs:line 80 at Reko.Decompiler.StructureProgram() in c:\dev\uxmal\reko\master\src\Decompiler\Decompiler.cs:line 592 @@ -2588,4 +2586,4 @@ PE Debug type 14 not supported yet. === PE\x86\VCExeSample\VCExeSample Signature of 'Microsoft Visual C++ 8' detected. -Decompiled 93 binaries in 41.59 seconds. +Decompiled 93 binaries in 61.22 seconds.