Possible codegen issue with unbox instruction for nullable types

[LEI-Hongfaan]

Description

I encountered a bug when using the "unbox valuetype System.Nullable`1<!!0>" instruction. The bug causes the unboxed value to be incorrect.

However, the bug is not always reproducible. It depends on some details of the code, such as the way of asserting the value and whether the AggressiveOptimization attribute is enabled or not.

I suspect that the bug is related to some optimization mechanism in the code generation process, but I'm not sure how to debug it further.

Here are the codes :

.method public hidebysig static valuetype System.Nullable`1<!!T>& 
  UnboxNullable<valuetype .ctor (System.ValueType) T>(object boxed) cil managed {
  .maxstack 8
  ldarg.0
  unbox valuetype System.Nullable`1<!!0>
  ret
}

or equivalently using InlineIL.Fody

public static ref T? UnboxNullable<T>(object? boxed) where T : struct {
  Emit.Ldarg(nameof(boxed));
  Emit.Unbox(typeof(T?));
  Emit.Ret();
  throw null!;
}

Reproduction Steps

To reproduce the bug, I used the following code:

[Test()]
// [MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static void UnboxNullableTest() {
  var boxed = (object?)(int?)42;
  ref var v = ref Unsafe.UnboxNullable<int>(boxed);
  Assert.IsTrue(42 == v);
  Assert.AreEqual(42, v);
}

Expected behavior

The test UnboxNullableTest should pass.

Actual behavior

Assert failure.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 7.0.5
x64

Other information

ECMA-335 2012 p.431

III.4.32 unbo x – convert boxed value type to its raw form

The unbox instruction converts obj (of type O), the boxed representation of a value type, to
valueTypePtr (a controlled-mutability managed pointer (§III.1.8.1.2.2), type &), its unboxed
form. valuetype is a metadata token (a typeref, typedef or typespec). The type of valuetype
contained within obj must be verifier-assignable-to valuetype.

Correctness:
Correct CIL ensures that valueType is a typeref, typedef or typespec metadata token for some
boxable value type, and that obj is always an object reference (i.e., of type O). If valuetype is the
type Nullable<T>, the boxed instance shall be of type T.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

I encountered a bug when using the "unbox valuetype System.Nullable`1<!!0>" instruction. The bug causes the unboxed value to be incorrect.

However, the bug is not always reproducible. It depends on some details of the code, such as the way of asserting the value and whether the AggressiveOptimization attribute is enabled or not.

I suspect that the bug is related to some optimization mechanism in the code generation process, but I'm not sure how to debug it further.

Here are the codes :

.method public hidebysig static valuetype System.Nullable`1<!!T>& 
  UnboxNullable<valuetype .ctor (System.ValueType) T>(object boxed) cil managed {
  .maxstack 8
  ldarg.0
  unbox valuetype System.Nullable`1<!!0>
  ret
}

or equivalently using InlineIL.Fody

public static ref T? UnboxNullable<T>(object? boxed) where T : struct {
  Emit.Ldarg(nameof(boxed));
  Emit.Unbox(typeof(T?));
  Emit.Ret();
  throw null!;
}

Reproduction Steps

To reproduce the bug, I used the following code:

[Test()]
// [MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static void UnboxNullableTest() {
  var boxed = (object?)(int?)42;
  ref var v = ref Unsafe.UnboxNullable<int>(boxed);
  Assert.IsTrue(42 == v);
  Assert.AreEqual(42, v);
}

Expected behavior

The test UnboxNullableTest should pass.

Actual behavior

Assert failure.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 7.0.5
x64

Other information

ECMA-335 2012 p.431

III.4.32 unbo x – convert boxed value type to its raw form

The unbox instruction converts obj (of type O), the boxed representation of a value type, to
valueTypePtr (a controlled-mutability managed pointer (§III.1.8.1.2.2), type &), its unboxed
form. valuetype is a metadata token (a typeref, typedef or typespec). The type of valuetype
contained within obj must be verifier-assignable-to valuetype.

Correctness:
Correct CIL ensures that valueType is a typeref, typedef or typespec metadata token for some
boxable value type, and that obj is always an object reference (i.e., of type O). If valuetype is the
type Nullable<T>, the boxed instance shall be of type T.

Author:	LEI-Hongfaan
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[EgorBo]

I don't see how the highlighted ECMA paragraph proves it's not correct. It doesn't say anything about boxing an integer and unboxing it as nullable (via unbox, not unbox.any)

You boxed a 4-byte integer and obtained an address of the boxed contet as a 8-byte struct

[LEI-Hongfaan]

@EgorBo As I understand it, first, boxing an int and then unboxing it to an int? is correct CIL. Second, boxing an int 42 and an int? 42 will result in the same object on the heap - a boxed int 42. Finally, unbox returns a pointer, but it is essentially a conversion instruction that can involve data copying, typically (including CoreCLR) implemented as returning a pointer to an int? struct copied (back) to the stack.

[EgorBo]

As I understand it, first, boxing an int and then unboxing it to an int? is correct CIL.

Yes, via unbox.any

but it is essentially a conversion instruction that can involve data copying, typically (including CoreCLR) implemented as returning a pointer to an int? struct copied (back) to the stack.

Do you mean a pointer to the stack of UnboxNullable in your case? Because it's where it's defined.

[EgorBo]

Although, the ECMA says:

🤔

Does it say that unbox should allocate a new object and return a byref to its content?

cc @jkotas

[LEI-Hongfaan]

Yes, via unbox.any

Via unbox

Do you mean a pointer to the stack of UnboxNullable in your case? Because it's where it's defined.

Oh, I think I see where the problem is. Unless there is some mechanism that ensures UnboxNullable is inlined or supported as an intrinsic, it seems that this form of the library function won't work, because the copied struct's lifetime is implicitly and naturally expected to cover at least the scope of the unbox instruction, and when the library function returns, that struct can be destroyed. Although theoretically the implementation could make a real copy on the GC heap, that would not be efficient, nor required by ECMA-335.

[LEI-Hongfaan]

@EgorBo Do you think copying to the GC heap is a requirement of ECMA-335? I just thought that was a possible way to do it.

[jkotas]

RyuJIT implements unbox for Nullable<T> by returning a byref to a temp. It does not work when the byref is returned from the current method, or when the same temp is used more than once (while the previous byref is still in use). It has been like that since .NET Framework 2.0.

We can either document the current behavior in the ECMA augments or change the codegen to allocate the storage on GC heap as the ECMA spec suggests. If we choose to do the later, we need to do analysis of affected places - changing the stack allocation to GC heap allocation can be a significant regression.

I have a mild preference for documenting the current behavior by saying that unbox is not supported for Nullable<T> and that the type used with unbox instruction must satisfy struct C# constraint.