Invalid IL code - cgt.un when comparing null with void*

[thargy]

We recently had the following bug report: dotnet/Silk.NET#389 complaining about an invalid IL code. On further investigation, we saw a very similar situation reported on your Gitter.

Current Behavior

The following is the offending IL (see IL_002-IL_008):

  .method private hidebysig instance void
    AssertNotCreated() cil managed
  {
    .maxstack 2
    .locals init (
      [0] bool V_0
    )

    // [88 9 - 88 10]
    IL_0000: nop

    // [89 13 - 89 34]
    IL_0001: ldarg.0      // this
    IL_0002: ldfld        void* Silk.NET.SDL.SdlContext::_ctx
    IL_0007: ldnull
    IL_0008: cgt.un
    IL_000a: stloc.0      // V_0

    IL_000b: ldloc.0      // V_0
    IL_000c: brfalse.s    IL_001a

    // [90 13 - 90 14]
    IL_000e: nop

    // [91 17 - 91 81]
    IL_000f: ldstr        "Context created already."
    IL_0014: newobj       instance void [System.Runtime]System.InvalidOperationException::.ctor(string)
    IL_0019: throw

    // [93 9 - 93 10]
    IL_001a: ret

  } // end of method SdlContext::AssertNotCreated

Which is similar to the Gitter report, in that the cgt.un is used for a null check against a void* variable:

  // Code size       135 (0x87)
  .maxstack  8
  .locals init (           void* V_4,
  IL_0071:  ldloc.s    V_4
  IL_0073:  ldnull
  IL_0074:  cgt.un
  IL_0076:  br.s       IL_0079

Note: both examples are comparing a void* with null; as such a minimal repo might be:

unsafe
{
	  void* a = null;
	  if (a is null)
	  {
		  // Do something
	  }
}

Expected Behavior

This should not be considered invalid IL as this is valid in Roslyn.

ECMA-355 (pg. 371) states (emphasis added):

The ldnull pushes a null reference (type O) on the stack. This is used to initialize locations before
they become live or when they become dead.

[Rationale: It might be thought that ldnull is redundant: why not use ldc.i4.0 or ldc.i8.0 instead?
The answer is that ldnull provides a size-agnostic null – analogous to an ldc.i instruction, which
does not exist. However, even if CIL were to include an ldc.i instruction it would still benefit
verification algorithms to retain the ldnull instruction because it makes type tracking easier. end
rationale]

Indicating that a ldnull should be considered as analogous to the appropriate ldc.i4.0 or ldc.i8.0 instruction.

Further, pg.303 explicitly states:

cgt.un is allowed and verifiable on ObjectRefs (O). This is commonly used when
comparing an ObjectRef with null (there is no “compare-not-equal” instruction, which
would otherwise be a more obvious solution)

As such, the issue is most likely due to the first comparand being of type void* which is of no type?

[john-h-k]

As such, the issue is most likely due to the first comparand being of type void* which is of no type?

In IL, void* is native int on the stack

[borgdylan]

For equality comparisons, teh right op-code would be ceq. Roslyn may end up optimizing the case of an equality checkinside an if statement with the op-code bne. If one were to use cgt.un, there would need to be another copy of the operands to pass on to clt.un since !(a == b) is the same as a < b || a > b.

[john-h-k]

Roslyn may end up optimizing the case of an equality checkinside an if statement with the op-code bne

Yes, but it doesn't have to. cgt.un is perfectly valid for null checks

If one were to use cgt.un, there would need to be another copy of the operands to pass on to clt.un since !(a == b) is the same as a < b || a > b.

It's an unsigned check, and nothing can be less than 0 there, so the clt.un is unnecessary.

!(a == b) is the same as a < b || a > b.

I assume you mean && not ||?

[borgdylan]

It is || as not equal is the same as less than or greater than. Two numbers can never be less than and greater than at the same time.

[john-h-k]

It is || as not equal is the same as less than or greater than. Two numbers can never be less than and greater than at the same time.

Oh yeah, i am silly