Why is this code failing to verify?

228 views Asked by At

I have a compiler that builds and runs correctly, but PEVerify is calling it unverifiable at a certain point. After looking carefully at the error, the corresponding source code, and the ILDasm output for the point in question, I can't find the problem, to the point where I would suspect a bug in PEVerify, except that the .NET and Mono versions report the same error in the same place.

The problematic method reads as follows:

internal static bool InAsyncMethod(Expression value)
{
    INodeWithBody ancestor = value.GetAncestor<BlockExpression>() ?? (INodeWithBody) value.GetAncestor<Method>();
    return ContextAnnotations.IsAsync(ancestor);
}

The error is reported as:

[IL]: Error: [D:\SDL-1.3.0-4423\boo\build\Boo.Lang.Compiler.dll : Boo.Lang.Compiler.TypeSystem.AsyncHelper::InAsyncMethod][offset 0x00000011][found ref 'Boo.Lang.Compiler.Ast.Node'][expected ref Boo.Lang.Compiler.Ast.INodeWithBody'] Unexpected type on the stack.

Offsest 0x11 corresponds to the second half of the ?? expression. From ILDasm:

.method assembly hidebysig static bool  InAsyncMethod(class Boo.Lang.Compiler.Ast.Expression 'value') cil managed
{
  // Code size       29 (0x1d)
  .maxstack  2
  .locals init ([0] class Boo.Lang.Compiler.Ast.INodeWithBody ancestor,
           [1] bool CS$1$0000)
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  callvirt   instance !!0 Boo.Lang.Compiler.Ast.Node::GetAncestor<class Boo.Lang.Compiler.Ast.BlockExpression>()
  IL_0007:  dup
  IL_0008:  brtrue.s   IL_0011
  IL_000a:  pop
  IL_000b:  ldarg.0
  IL_000c:  callvirt   instance !!0 Boo.Lang.Compiler.Ast.Node::GetAncestor<class Boo.Lang.Compiler.Ast.Method>()
  IL_0011:  stloc.0
  IL_0012:  ldloc.0
  IL_0013:  call       bool Boo.Lang.Compiler.Steps.ContextAnnotations::IsAsync(class Boo.Lang.Compiler.Ast.INodeWithBody)
  IL_0018:  stloc.1
  IL_0019:  br.s       IL_001b
  IL_001b:  ldloc.1
  IL_001c:  ret
} // end of method AsyncHelper::InAsyncMethod

The Boo.Lang.Compiler.Ast.Node class is the base class for all AST nodes. BlockExpression and Method are the node classes for lambdas and methods, respectively, both of which implement the INodeWithBody interface. Everything appears correct, both in the C# (which would not build if there were type problems) and in the IL (where it says at 000c that the return type of the GetAncestor<Method> call is !!0, the first type parameter on the method call.)

What is causing PEVerify to think it's dealing with a value of type Node here when it clearly has a value of type Method? And is there any way to fix it?

2

There are 2 answers

3
AudioBubble On BEST ANSWER

What is causing PEVerify to think it's dealing with a value of type Node here when it clearly has a value of type Method?

As pointed out by Stephane Delcroix as well, there are two code paths reaching to IL_0011, as it is the target of a branch. It clearly does not necessarily have a value of type Method.

GetAncestor<TAncestor> returns TAncestor. You've got either GetAncestor<BlockExpression>'s result, or GetAncestor<Method>'s result, so either BlockExpression or Method. Both implement INodeWithBody, so logically, the code is still fine.

Unfortunately, "either BlockExpression or Method" is too much for verification. This gets simplified to Node (the common base), which does not implement INodeWithBody. See ECMA-335 §III.1.8.1.3:

The merged type, U, shall be computed as follows (recall that S := T is the compatibility function defined in §III.1.8.1.2.2):

  1. if S := T then U=S

  2. Otherwise, if T := S then U=T

  3. Otherwise, if S and T are both object types, then let V be the closest common supertype of S and T then U=V.

  4. Otherwise, the merge shall fail.

If you check what the C# compiler does, you'll see that it emits a stloc.0/ldloc.0 combination into a local of type INodeWithBody prior to the dup. This makes everything work, because the common type of INodeWithBody and Method is then INodeWithBody.

1
Stephane Delcroix On

It's unclear if it fails on 0x11 due to the first or the right part of the ?? as 0x08 is branching to there.

I'm leaning toward thinking GetAncestor<> returns a Node and the left part of the ?? is missing an explicit cast.