BadImageFormatException on calling generic local function

[udoliess]

Version Used:
current Visual Studio 2017 15.8.0
tested with:

• Console Application
• .NET Framework 4.7.1.
• Any CPU

Steps to Reproduce:
Compile and run following code:

using System.Linq;
class Program
{
	static void Main(string[] args)
	{
		var n = 0;
		do
		{
			void Foo<T>() => args.Select(a => (n, default(T)));
			Foo<float>();
		} while (false);
	}
}

Expected Behavior:
Compile and run without errors.

Actual Behavior:
Program compiles without error but throws BadImageFormatException when run.

[jaredpar]

CC @agocke for investigation

[codernator]

Here is another slightly different example of reproducing the bug as well as some counter examples that do not reproduce the bug.

/// <summary>
/// Demonstrate how a generic function passing a closure to of a deferred function
/// can create a BadImageFormatException. This includes an example of the exception,
/// and similar examples that do not create the exception.
/// </summary>
static void Main()
{
	// The data type and value is not important to this example.
	// Only the context in which this variable is used is important.
	var myClosure = 0;
	
	HereComesTheException(new object[0]);
	//NonGenericNoException(new object[0]);
	//GenericWorkAroundA(new object[0]);
	//GenericWorkAroundB(new object[0]);

	// This fucntion creates a BadImageFormatException. Note the
	// use of myClosure within a deferred function that is
	// inline to a foreach. As far as I could read the IL, there is no
	// reference to myClosure in the scope of this function.
	void HereComesTheException<T>(IEnumerable<T> things)
	{
		foreach (var thing in things.Where(tt => myClosure == 0)) {}
	}

	// This function doesn't create a BadImageFormatException...
	// The only difference from HereComesTheException is that
	// this function is not generic.
	void NonGenericNoException(IEnumerable<object> things)
	{
		foreach (var thing in things.Where(tt => myClosure == 0)) {}
	}

	// This function doesn't create a BadImageFormatException because the
	// outer closure is stored in a local variable that is clearly in scope 
	// when the deferred function is executed.
	void GenericWorkAroundA<T>(IEnumerable<T> things)
	{
		var inner = myClosure;
		foreach (var thing in things.Where(tt => inner == 0)) {}
	}
	
	// This function doesn't create a BadImageFormatException... 
	// Is this because declaring the Enumerator as a local variable forces
	// local context to be given to myClosure?
	void GenericWorkAroundB<T>(IEnumerable<T> things)
	{
		var filtered = things.Where(tt => myClosure == 0);
		foreach (var thing in filtered) {}
	}
}

[agocke]

This turns out to be an interesting bug that happens because we introduce a field of the delegate type for delegate caching, but we only do that after checking for captured variables (but not captured types).

The right fix here is track captured types as well as captured variables, but it's a change that will need to be applied carefully as it's likely to cause change in unrelated closures. Some of that change will probably make things more efficient, while some code may need more work to re-enable a valid optimization.

[codernator]

Why does GenericWorkAroundB<T> generate valid IL when the only difference to HereComesTheException<T> is that the iterator is first stored in a local variable before the foreach?

[agocke]

Ah, that's because one of the triggers for delegate caching is if the lambda is inside a loop. Note, not necessarily in the body of a loop, we just have a coarse check to see if the parent syntax is a loop, which it is. By lifting the expression into a local, you avoided the optimization.