What's wrong with my simple MSIL?

Question

I'm trying to generate the following class:

public class MyType
{
    public string MyMethod() { return "Hi"; }
}

My Emit code is as follows:

var assemblyBuilder = GetAssemblyBuilder("MyAssembly");
var moduleBuilder = assemblyBuilder.DefineDynamicModule("MyModule");
var typeBuilder = moduleBuilder.DefineType("MyType", TypeAttributes.Public);
var methodBuilder = typeBuilder.DefineMethod("MyMethod", MethodAttributes.Public, typeof(string), new Type[] { });
var ilBuilder = methodBuilder.GetILGenerator();
ilBuilder.Emit(OpCodes.Nop);
ilBuilder.Emit(OpCodes.Ldstr, "Hi");
ilBuilder.Emit(OpCodes.Stloc_0);
ilBuilder.Emit(OpCodes.Br_S);
ilBuilder.Emit(OpCodes.Ldloc_0);
ilBuilder.Emit(OpCodes.Ret);
var type = typeBuilder.CreateType();

but when I invoke MyMethod on an instance of MyType I get an InvalidProgramException: Common Language Runtime detected an invalid program.

I've tried changing the return type to void and using just EmitWriteLine and Emit(OpCodes.Ret), which runs fine, so it must be the IL I've written here.

Am I missing something obvious here? A clear explanation would be helpful as I'm just getting started with Emit.

Additional information from comments:

The "original" IL is taken from the IL-generation in LINQ-pad.

Answer 1

In addition to removing the OpCodes.Br_S as pointed out by @Kendall, you need to declare the local variable you're storing "Hi" to:

ilBuilder.DeclareLocal(typeof(string));
ilBuilder.Emit(OpCodes.Nop);
ilBuilder.Emit(OpCodes.Ldstr, "Hi");
ilBuilder.Emit(OpCodes.Stloc_0);
ilBuilder.Emit(OpCodes.Ldloc_0);
ilBuilder.Emit(OpCodes.Ret);

Additionally, I think the whole thing could be shortened to (this is what you get if you build your class with compiler optimizations on):

ilBuilder.Emit(OpCodes.Ldstr, "Hi");
ilBuilder.Emit(OpCodes.Ret);

(Just load the string, putting it on top of the evaluation stack, and then return the string).

If you wanted to replicate the version with compiler optimizations off, which I think is what you were originally trying to do:

Label returnLabel = ilBuilder.DefineLabel();

ilBuilder.DeclareLocal(typeof(string));
ilBuilder.Emit(OpCodes.Nop);
/* Load the string "HI" and put it on the evaluation stack. */
ilBuilder.Emit(OpCodes.Ldstr, "Hi");

/* Store "Hi" in the local variable we declared. */
ilBuilder.Emit(OpCodes.Stloc_0);

/* Jump to the return label, which is, err the next line anyway: */
ilBuilder.Emit(OpCodes.Br_S, returnLabel);

/* Mark "returnLabel" here so we can jump to it: */
ilBuilder.MarkLabel(returnLabel);    

/* Load the contents of our local variable, put it on the evaluation stack: */
ilBuilder.Emit(OpCodes.Ldloc_0);

/* Return "Hi", which is now back on top of the evaluation stack: */
ilBuilder.Emit(OpCodes.Ret);

You need to define a label to branch to (using ILGenerator.DefineLabel, use MarkLabel, and then pass that label to Emit when you emit OpCodes.Br_S.

As you can see though, the branching is kind of pointless with a method this short.

Answer 2

ilBuilder.Emit(OpCodes.Br_S);

Where were you intending to branch to? br.s requires a signed byte argument that gives the offset to jump to.