Coalesce expression causing an issue in return statements in Fody

Question

Consider the following snippet of code:

private string GetFieldValueAsString(string nonAliasedEntityName = null, string nonAliasedName = null)
{
    return nonAliasedEntityName ?? nonAliasedName;   // simplified code of course!
}

It is compiled to the following IL code:

.method private hidebysig 
    instance string GetFieldValueAsString (
        [opt] string nonAliasedEntityName,
        [opt] string nonAliasedName
    ) cil managed 
{
    .param [1] = nullref
    .param [2] = nullref
    // Method begins at RVA 0x2cb6f
    // Code size 7 (0x7)
    .maxstack 8

    IL_0000: ldarg.1
    IL_0001: dup
    IL_0002: brtrue.s IL_0006

    IL_0004: pop
    IL_0005: ldarg.2

    IL_0006: ret
} // end of method MessageBuilder::GetFieldValueAsString

If I apply 'fix the returns' in Fody, I get the following IL code:

.method private hidebysig 
    instance string GetFieldValueAsString (
        [opt] string nonAliasedEntityName,
        [opt] string nonAliasedName
    ) cil managed 
{
    .param [1] = nullref
    .param [2] = nullref
    // Method begins at RVA 0x2cb70
    // Code size 15 (0xf)
    .maxstack 3
    .locals (
        [0] string $returnVariable
    )

    IL_0000: ldarg.1
    IL_0001: dup
    IL_0002: dup
    IL_0003: stloc.0
    IL_0004: brtrue.s IL_000b

    IL_0006: pop
    IL_0007: ldarg.2
    IL_0008: stloc.0
    IL_0009: br.s IL_000b

    IL_000b: nop
    IL_000c: nop
    IL_000d: ldloc.0
    IL_000e: ret
} // end of method MessageBuilder::GetFieldValueAsString

It gives me the following error when decompiling in ILSpy, and fails to run:

ICSharpCode.Decompiler.DecompilerException: Error decompiling System.String LinkDev.Notifications.Steps.MessageBuilder::GetFieldValueAsString(System.String,System.String)
 ---> System.Exception: Inconsistent stack size at IL_09
   at ICSharpCode.Decompiler.ILAst.ILAstBuilder.StackAnalysis(MethodDefinition methodDef)
   at ICSharpCode.Decompiler.ILAst.ILAstBuilder.Build(MethodDefinition methodDef, Boolean optimize, DecompilerContext context)
   at ICSharpCode.Decompiler.Ast.AstMethodBodyBuilder.CreateMethodBody(IEnumerable`1 parameters)
   at ICSharpCode.Decompiler.Ast.AstMethodBodyBuilder.CreateMethodBody(MethodDefinition methodDef, DecompilerContext context, IEnumerable`1 parameters)
   --- End of inner exception stack trace ---
   at ICSharpCode.Decompiler.Ast.AstMethodBodyBuilder.CreateMethodBody(MethodDefinition methodDef, DecompilerContext context, IEnumerable`1 parameters)
   at ICSharpCode.Decompiler.Ast.AstBuilder.CreateMethod(MethodDefinition methodDef)
   at ICSharpCode.Decompiler.Ast.AstBuilder.AddMethod(MethodDefinition method)
   at ICSharpCode.ILSpy.CSharpLanguage.DecompileMethod(MethodDefinition method, ITextOutput output, DecompilationOptions options)
   at ICSharpCode.ILSpy.TextView.DecompilerTextView.DecompileNodes(DecompilationContext context, ITextOutput textOutput)
   at ICSharpCode.ILSpy.TextView.DecompilerTextView.<>c__DisplayClass31_0.<DecompileAsync>b__0()

I traced the stack size, and it seems to hold. Any idea what could be causing this issue?

Update 1:

I added a temporary quick fix for the issue:

Instruction doubleDupInstruction = null;

for (var index = 0; index < instructions.Count; index++)
{
    var instruction = instructions[index];

    if (instruction.OpCode == OpCodes.Dup && instructions[index + 1].OpCode == OpCodes.Dup)
    {
        doubleDupInstruction = instructions[index + 1];
    }

    if (instruction.OpCode == OpCodes.Pop && doubleDupInstruction != null)
    {
        var extraPopInstruction = instructions[index];
        ilProcessor.Remove(extraPopInstruction);
        ilProcessor.InsertAfter(doubleDupInstruction, extraPopInstruction);
        doubleDupInstruction = null;
    }
}

So far it works in a decent sized program. I will keep monitoring it, and will update if anything changes. It would be much better if I could find the source of the issue in the 'return fixer'.

Answer 1

I think you have an inconsistent stack size when arriving at IL_000b depending on coming from IL0004 or IL0009.

Here is my analysis, the value on the right is the stack size after executing the respective line.

                            [0]
IL_0000: ldarg.1            [1]
IL_0001: dup                [2]
IL_0002: dup                [3]
IL_0003: stloc.0            [2]
IL_0004: brtrue.s IL_000b   [1]---
                                 |
IL_0006: pop                [0]  |
IL_0007: ldarg.2            [1]  |
IL_0008: stloc.0            [0]  |
IL_0009: br.s IL_000b       [0]  |
                                 v
IL_000b: nop                [?] arriving here is inconsistent
IL_000c: nop
IL_000d: ldloc.0
IL_000e: ret

Maybe you can run PEVerify against this method and see whats the output there.