ISIL -> CIL Decompiler #426
Conversation
…d instead if IOperand, IL is now non nested (makes analysis easier) and refactoring, a lot...
|
Hello, thank you for the pull request! This resolves #223 |
ds5678
left a comment
ds5678
left a comment
There was a problem hiding this comment.
I gave it a first pass. Thank you for working on this.
| public override string Description => | ||
| "Core Cpp2IL plugin containing built-in instruction sets, binaries, and other core functionality."; |
There was a problem hiding this comment.
This change is unnecessary.
There was a problem hiding this comment.
formatter did that
There was a problem hiding this comment.
You should avoid using automatic formatters on existing files.
| <ProjectReference Include="..\Cpp2IL.Decompiler\Cpp2IL.Decompiler.csproj"/> | ||
|
|
There was a problem hiding this comment.
Sam would need to weigh in on this, but I feel like everything should be in one project.
| Logger.VerboseNewline($"Core plugin loaded in {elapsed.Ticks} ticks ({elapsed.TotalMilliseconds}ms)", | ||
| "Core Plugin"); |
There was a problem hiding this comment.
This change is unnecessary.
There was a problem hiding this comment.
formatter did that
| public abstract List<Instruction> GetDecompilerIlFromMethod(MethodAnalysisContext context, out List<object> ilParams); | ||
|
|
There was a problem hiding this comment.
Why decompilation is specific to each instruction set if you're using ISIL?
|
|
||
| namespace Cpp2IL.Core.OutputFormats; | ||
|
|
||
| public class IlOutputFormat : AsmResolverDllOutputFormat |
There was a problem hiding this comment.
AsmResolverDllOutputFormatIlRecovery should be modified instead of creating a new output format.
| /// <summary> | ||
| /// Provides additional context for the decompiler. | ||
| /// </summary> | ||
| public interface IContext |
| using AsmResolver.DotNet; | ||
| using AsmResolver.DotNet.Signatures; |
There was a problem hiding this comment.
Are these used at all in this file? I was confused to discover that OpCode is not an AsmResolver type. Please use the Sort and remove unnecessary usings tool in your IDE.
| /// <summary> | ||
| /// Type of the variable. | ||
| /// </summary> | ||
| public TypeDefinition? Type = type; |
There was a problem hiding this comment.
I'm surprised this isn't TypeSignature.
| /// <summary> | ||
| /// All opcodes. If changing this, also update <see cref="Instruction"/>. | ||
| /// </summary> | ||
| public enum OpCode // There is some weird stuff in doc comments because i can't use <, >, & (idk why & doesn't work) |
There was a problem hiding this comment.
This feels redundant with ISIL.
There was a problem hiding this comment.
i didn't use ISIL because compare sets flags, and those flags could be platform specific, and those could be used in non branch instructions so that would be annoying, and ISIL is just weird in many ways and if i changed it i would need to change it in so many places
There was a problem hiding this comment.
i didn't use ISIL
So why is your pull request titled ISIL -> CIL if you don't use ISIL?
| /// <summary> | ||
| /// Memory operand in the format of [base+addend+index*scale]. | ||
| /// </summary> | ||
| public struct MemoryAddress(object? baseRegister = null, object? indexRegister = null, long addend = 0, int scale = 0) |
There was a problem hiding this comment.
Are baseRegister and indexRegister always Register or null? If so, they should be typed as Register? to avoid boxing.
3 participants
CIL is very bad right now, but control flow graph seems to pretty good. to get those graphs uncomment that thing in IlOutputFormat.cs FillMethodBody