ioncodes / dnpatch

.NET Patcher library using dnlib
MIT License
313 stars 48 forks source link

Fails to patch methods with try/catch blocks #39

Closed DenJur closed 6 years ago

DenJur commented 7 years ago

Patching fails if target method contains try/catch block. I have attached a test library and here is the reproduction code:

            var patcher = new dnpatch.Patcher("data/Test.dll");
            var target = new Target
            {
                Namespace = "Test",
                Class = "TestClass",
                Method = "TestMethod",
            };
            patcher.WriteReturnBody(target,true);
            patcher.Save("TestNew.dll");

Test.zip

Throws dnlib.DotNet.Writer.ModuleWriterException exception {"Found some other method's instruction or a removed instruction. You probably removed an instruction that is the target of a branch instruction or an instruction that's the first/last instruction in an exception handler."}

I have not gotten to test with v1 so not sure if it is the same there.

DenJur commented 7 years ago

Can confirm same happening in v1. ----------Edit---------- Problem is because Body.ExceptionHandlers is not cleared. Definitely should be cleared for full overwrites. Also should consider interface for building exception handlers(maybe similar to ids I proposed for jump codes). ----------Edit---------- Fix for this case #40

gagmeng commented 7 years ago

any progress on this issue ? I have encountered this issue too.

gagmeng commented 7 years ago
public  void PatchAndClear(Target target)
{
    string[] nestedClasses = { };
    if (target.NestedClasses != null)
    {
        nestedClasses = target.NestedClasses;
    }
    else if (target.NestedClass != null)
    {
        nestedClasses = new[] { target.NestedClass };
    }
    var type = FindType(target.Namespace + "." + target.Class, nestedClasses);
    var method = FindMethod(type, target.Method, target.Parameters, target.ReturnType);
    var instructions = method.Body.Instructions;
    instructions.Clear();
    if (target.Instructions != null)
    {
        for (int i = 0; i < target.Instructions.Length; i++)
        {
            instructions.Insert(i, target.Instructions[i]);
        }
    }
    else
    {
        instructions.Insert(0, target.Instruction);
    }
}

Edit as below, the issuse closed.

public  void PatchAndClear(Target target)
{
    string[] nestedClasses = { };
    if (target.NestedClasses != null)
    {
        nestedClasses = target.NestedClasses;
    }
    else if (target.NestedClass != null)
    {
        nestedClasses = new[] { target.NestedClass };
    }
    var type = FindType(target.Namespace + "." + target.Class, nestedClasses);
    var method = FindMethod(type, target.Method, target.Parameters, target.ReturnType);
    var instructions = method.Body.Instructions;
    instructions.Clear();
    method.Body.ExceptionHandlers.Clear();  //Added
    if (target.Instructions != null)
    {
        for (int i = 0; i < target.Instructions.Length; i++)
        {
            instructions.Insert(i, target.Instructions[i]);
        }
    }
    else
    {
        instructions.Insert(0, target.Instruction);
    }
}
ioncodes commented 6 years ago

I will fix this for the v1, but I don't plan to fix it for the master branch.

ioncodes commented 6 years ago

This fix adds the possibility to create a completely new body and/or just wipe the exception handlers.