CodeMutatingVisitor changes

Feb 13, 2011 at 5:07 PM

I was using CodeMutatingVisitor to walk an assembly to scan for static method calls and replace them. Basically the flow went like this:

var testAssembly = host.LoadUnitFrom("somePath") as IAssembly;
var mutableAssembly = Decompiler.GetCodeModelFromMetadataModel(host, testAssembly, null); 

var registrar = new StaticMethodCallRegistrar(host);
registrar.Visit(mutableAssembly);

where the 'registrar' looked like this:

    public class StaticMethodCallRegistrar : CodeMutatingVisitor
    {
        public StaticMethodCallRegistrar(IMetadataHost host) : base(host)
        {
        }

        public override IExpression Visit(MethodCall methodCall)
        {
            if (methodCall.IsStaticCall)
            {
                // Do something
            }

            return base.Visit(methodCall);
        }
    }

This code no longer works with recent changes to CodeModel. I'm wondering by what method I should achieve this traversal now.

For a little bit of background, I'm basically writing a post-compiler that does method replacement (for now just on static methods). The workflow is:

1 - Scan assembly for static method calls (shown above)

2 - Replace static methods identified with calls to different static methods (achieved by a second CodeMutatingVisitor that actually changes the value methodCall.MethodToCall)

I've actually always felt I was going about this the wrong way, but for quite some time it has worked nicely. I wasn't able to understand much from herman's svn comments or the diff for the latest changes to Mutator.cs in MutableCodeModel.

(P.S. Though it was frustrating at first to try to understand how to use the mutable CodeModel, now that I've become more familiar with it and developed some utilities of my own to easy in it's usage, I've really enjoyed working with this library. It is incredibly powerful, and I appreciate the breadth and depth of the entire CCI library!)

Coordinator
Feb 13, 2011 at 5:31 PM

I find it a bit surprsing that this code no longer works. Probably I've introduced a bug. If you can package up a small repro that works with an earlier version and doesn't work with the current version I'll be glad to investigate and fix the problem.

Your scenario sounds like one that does not need the use of a mutating visitor (which is actually more of a rewriter). Using the base traverser would be more efficient since it does not rewrite the tree (and hence does far less writing to memory). On your second pass, simply cast down the IMethodCall to a MethodCall (the cast should be safe since Decompiler uses mutable MethodCall objects to implement IMethodCall) and modify the method call so that the MethodToCall is the one you desire.

Feb 13, 2011 at 9:33 PM

I have uploaded a solution that reproduces my issue to the 'Issues' section. I will probably be refactoring my code per your suggestions, but in case a bug was introduced, perhaps my repro will help you find it. Thanks for your help!

Coordinator
Feb 14, 2011 at 12:43 AM

The underlying problem was that the Decompiler does not use a SourceMethodBody from the Mutable code model, but a custom source method body that knows how to decompile and that does not need to compile (since it still has the original IL available).

The CodeMutatingVisitor in the old version (which I did not write) dealt with this in an obscure manner by always duplicating any ISourceMethodBody object as a new SourceMethodBody object. When fixing this performance bug I had it in mind that the mutating visitor should only concern itself with objects from the mutable code model and I was blinded to this issue by the obscurity of the code. Since the decompiler produces a model that is entirely made up from mutable metadata/codemodel nodes, except for source method bodies, this is too much of a trap to leave around. Consequently I have departed from the "ignore unless mutable, as in Microsoft.Cci.MutableCodeModel" rule in this particular situation. I can't say that I'm happy about the inconsistency, but at the moment it seems like the least bad option.

Incidentally, the change had also weakened an internal test case. Fixing it led to uncovering another quite important bug, so this has been a very worthwhile investigation. Thanks for taking the trouble to report it and providing a convient repro case.

Feb 14, 2011 at 3:47 AM

I hope they're paying you the big bucks! This is impressive.

Coordinator
Feb 14, 2011 at 5:50 AM

Thanks. Upon further reflection, however, I think that my advice to simply use a traverser and rewrite the method call will not work until I get rid of this special case and force the decompiler to produce true Microsoft.Cci.MutableCodeModel.MethodBody instances. The behavior of the ILToCodeModel method body, which is to just render up the original IL with first recompiling the decompiled Block, breaks down as soon as any of this child nodes of the Block are altered. It seems that the latter kind of method body will have to fall by the wayside, which is probably going to be for the best.

Coordinator
Feb 14, 2011 at 9:48 PM

ILToCodeModel.SourceMethodBody is now a subclass of MutableCodeModel.MethodBody. As a result, the Decompiler now produces a fully mutable object model and making point mutations to parts of a decompiled method body should now work.