Reflecting Code Is Not A Perfect Science

If you are a .NET developer then you are probably familiar with Reflector. For most developers it is an indispensible tool in their programming toolbox because it can often shed light on problems that you may be experiencing in your code. One thing that Reflector can do is to display the IL of a given class or method, and this can be useful, but if you are anything like me, then you don’t really want to stare at IL, you would much rather see a reflected version in the language of my choice. This can often expose issues because you can see all of the compiler generated types and have all of the syntactic sugar stripped away.

All this time though, I knew that there would be some very interesting translations and manipulations that occurred, but I didn’t really consider that the code which reflector spit out might not actually execute in the same way as the code that I was compiling. Well, today I found just that case (in the latest version of Reflector) while checking out this blog post.

The Code

Now I know that this has not always been the case, because I have looked at this reflected code a million times before in order to try and figure out what is going on, but here is the code which I punched into the C# editor:

var actions = new List<Action>();
IEnumerable<int> values = new List<int> {1, 2, 3};
foreach (int value in values)
{
    actions.Add(() => Console.WriteLine(value));
}

foreach (Action action in actions)
{
    action();
}

If you’ve worked with lambdas before then you probably know the exact problem with this code, you’ve seen it a million times. The issue is that the closure wraps the variable and not the value, so when we execute our actions, the value from the foreach loop has been set to “3”, which is the last item in the list. So we just end up writing out 3, 3, 3 to the console. Just be aware that this code is broken, and that the variable would need to be assigned to a variable inside the scope of the loop in order to fix the problem.

The Reflection

But the important note is not so much that this is a problem (which it is), but that this is the code which is reflected:

List<Action> actions = new List<Action>();
List<int> <>g__initLocal0 = new List<int>();
<>g__initLocal0.Add(1);
<>g__initLocal0.Add(2);
<>g__initLocal0.Add(3);
IEnumerable<int> values = <>g__initLocal0;

using (IEnumerator<int> CS$5$0000 = values.GetEnumerator())
{
    while (CS$5$0000.MoveNext())
    {
        int value = CS$5$0000.Current;
        actions.Add(delegate {
            Console.WriteLine(value);
        });
    }
}

foreach (Action action in actions)
{
    action();
}

Notice anything wrong with this code? Yup, you guessed it, if we replace the compiler generated names we get this:

var actions = new List<Action>();
var localList = new List<int>();
localList.Add(1);
localList.Add(2);
localList.Add(3);
IEnumerable<int> values = localList;
using (IEnumerator<int> enumerator = values.GetEnumerator())
{
    while (enumerator.MoveNext())
    {
        int value = enumerator.Current;
        actions.Add(delegate { Console.WriteLine(value); });
    }
}
foreach (Action action in actions)
{
    action();
}

Summary

The problem here is that there is no problem. This code compiles and runs correct and prints 1, 2, 3 while the code above just prints 3, 3, 3. Looks like Reflector is getting a bit too smart in its optimizations! Now I know that reflecting code is an extremely difficult task, and I also realize that the problems here have to do with using the IEnumerable<int> in conjunction with the closure. I just wanted to put this out there to show people that you can’t always trust 100% of what is happening in reflected code because there are definitely edge cases where you are going to run into trouble.

Be Sociable, Share!

7 comments

  1. Is there a way to make Reflector act dumber, and show the actual compiler-generated enumerator types (short of viewing the raw IL, which is often enough to make your eyes bleed…)?

  2. @Jon I haven’t dug too deep, but at first glance there doesn’t appear to be any way to do that.

  3. If Reflector was opensource and you had an "open source mentality" (no offense), this would be just a bug which you could report and (if you had time and interest) fix it yourself.

    So yes, reflecting code can be a perfect science.

  4. Reflector does not claim to be a decompiler from which you can get compilable code. The primary aim of Reflector is to produce readable code.

    Decompiling is difficult, because decompiling means raising the abstraction level from MSIL to C#. It was fairly easy with C# 1.0, and more difficult with newer versions of the language since it becomes more and more abstract.

    There is a balance between raising the abstraction level and producing accurate code.

    So I think your expectations guys are not in line with the product. Reflector produces very readable code and it does not necessarily has to compile.

    That said, I think a good option would be to be able to disable some levels of abstraction. Sometimes, I would like to decompile to C# 1.0.

    And I suggest Antimonio to build its own Reflector and distribute it open source ;).

    -gael

  5. @antimonio No offense taken, and I agree that it would be nice if Reflector was open source.

    @Gael Hello, thanks for the comment. I love PostSharp :-) Anyways, I think you might have taken the post in the wrong way. I was merely showing an interesting edge case and trying to highlight how it is possible, because of the way that IL has to be interpreted in order to reflect it, that code which is reflected through Reflector might not be functionally equivalent to what went in.

    I also fully understand the tradeoffs between producing readable code and accurate code, and the way that the current version of reflector renders lambdas as anonymous functions is much easier to read than when it was showing the compiler generated classes. Unfortunately for some of us, the way we use reflector, seeing the compiler generated classes would much more useful.

  6. Nice find. That scope/lambda use case has bitten me before. I’m glad I hadn’t reflected the code to figure the problem out at the time, it would’ve just confused me more!

  7. So security through obscurity *is* a viable option in certain cases? hehe

Leave a comment