[mono-cecil] Branch offsets will fail if target instruction is changed?

[Jonathan Piché]

Hi,

I noticed that most of the time, Cecil correctly updates branch offset 
targets when weaving in new IL instructions. For instance, adding a simple 
"ldstr" followed by a call to Console.WriteLine will correctly offset the 
branches below to the new instruction offsets.


However, Cecil doesn't seem to correctly update offsets if the target 
instruction is changed.

Consider a brtrue_s that points to the final instruction of a method. In 
order to wrap the method in a try/finally, I am adding a new final ret 
instruction, and then changing all previous returns into new leave 
instructions to the new final instruction (else you branch out of a try 
block, which is invalid).

In the above case, the branch offset will remain what it was originally: no 
correction is made. Of course, the original instruction is no longer in the 
list of instructions; it was replaced with a new one.


I tried with a direct assignation (Instructions[index] = newInstruction) 
and also with (ilProcessor.Replace(Instructions[index], newInstruction). 
Both will create the bug.

The workaround is to change the instruction to ensure that the object 
reference is the same. so: Instructions[index].OpCode = 
newInstruction.OpCode; Instructions[index].Operand = newInstruction.Operand;


My question is: Shouldn't this be what .Replace does? Is there another way 
to deal with this behavior?


Thanks!


Note: I included a text file with my weaving code and the problematic 
method. The problematic branch is the first one (brtrue.s IL_009b on my 
machine).

-- 
-- 
--
mono-cecil
--- 
You received this message because you are subscribed to the Google Groups 
"mono-cecil" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to mono-cecil+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

// THE TEST METHOD
public void Test()
{
    AssemblyDefinition targetAssembly = 
AssemblyDefinition.ReadAssembly(m_TestAssembly, new ReaderParameters { 
ReadSymbols = true });
    MethodDefinition methodToWeave = 
targetAssembly.MainModule.GetType("InTest.UT.WeaverTemplates", 
"Templates").Methods.Single(m => m.Name == "ProblematicBranching").Resolve();

    string className = 
methodToWeave.DeclaringType.FullName.Replace(methodToWeave.DeclaringType.Namespace
 + ".", "");
    string methodName = className + "." + methodToWeave.Name;

    MethodReference callWriteLine = 
targetAssembly.MainModule.Import(typeof(Console).GetMethod("WriteLine", new 
Type[] { typeof(string) }));
    Instruction first = methodToWeave.Body.Instructions.First();

    var il = methodToWeave.Body.GetILProcessor();
    il.InsertBefore(first, il.Create(OpCodes.Ldstr, "--> " + methodName));
    il.InsertBefore(first, il.Create(OpCodes.Call, callWriteLine));

    var originalReturn = il.Body.Instructions.Last();           // original 
return
    var finallyStart = Instruction.Create(OpCodes.Nop);         // first 
instruction in finally block (and last try)
    il.Append(finallyStart);
    var finallyEnd = Instruction.Create(OpCodes.Endfinally);    // last 
instruction in finally block
    il.Append(finallyEnd);
    var finalReturn = Instruction.Create(OpCodes.Ret);          // the final 
return
    il.Append(finalReturn);

    // Find all returns and change them to leaves to the final return
    for (var index = 0; index < il.Body.Instructions.Count - 1; index++) {
        var instruction = il.Body.Instructions[index];
        if (instruction.OpCode == OpCodes.Ret) {
            // Change return into leave

            // Bug Method 1:
            var leaveInstruction = il.Create(OpCodes.Leave, finalReturn);
            il.Replace(il.Body.Instructions[index], leaveInstruction);

            // Bug Method 2:
            //var leaveInstruction = il.Create(OpCodes.Leave, finalReturn);
            //il.Body.Instructions[index] = leaveInstruction;

            // Workaround method:
            //instruction.OpCode = OpCodes.Leave;
            //instruction.Operand = finalReturn;
        }
    }

    Instruction ldArg1 = il.Create(OpCodes.Ldstr, " << " + methodName);
    Instruction callOnExit = il.Create(OpCodes.Call, callWriteLine);
    il.InsertBefore(finallyEnd, ldArg1);
    il.InsertBefore(finallyEnd, callOnExit);

    var handler = new ExceptionHandler(ExceptionHandlerType.Finally) {
        TryStart = first,
        TryEnd = finallyStart,
        HandlerStart = finallyStart,
        HandlerEnd = finalReturn
    };

    il.Body.ExceptionHandlers.Add(handler);
    il.Body.InitLocals = true;

    targetAssembly.Write(m_TestAssembly, new WriterParameters() { WriteSymbols 
= true });
}
    
    
// THE METHOD WE WEAVE INTO
void ProblematicBranching(string p_Path, Dictionary<string, string> p_Changes, 
bool p_ErasePatchList)
{
    string path = p_Path + "\\";
    string patchFileList = path + "PatchFileList.txt";
    if (File.Exists(patchFileList)) {
        using (TextReader tr = new StreamReader(patchFileList)) {
            // Read a filename.
            string filename;
            while ((filename = tr.ReadLine()) != null) {
                filename = filename.Trim();
                if (filename.Length != 0) {
                    // Patch the corresponding file.
                    ProblematicBranching(path + filename, p_Changes, true);
                }
            }
        }
        // Erase the list
        if (p_ErasePatchList) {
            File.Delete(patchFileList);
        }
    }
}