System.Linq.Expressions simulating Nullable.GetValueOrDefault should use GetUninitializedObject instead of Activator.CreateInstance

[eerhardt]

See the conversation here.

This code in SLE:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs

Lines 155 to 160 in 8a52f1e

	public override int Run(InterpretedFrame frame)
	{
	if (frame.Peek() == null)
	{
	frame.Pop();
	frame.Push(Activator.CreateInstance(_defaultValueType));

has the possibility of behaving incorrectly if/when dotnet/csharplang#99 is implemented in the future. If the value type has a parameterless constructor present in IL, Activator.CreateInstance would run that parameterless constructor. Which means the value you get back from Nullable<T>.GetValueOrDefault() could be different if run through this code than if you actually called Nullable<T>.GetValueOrDefault() on a "null" value.

We should instead change this to use GetUninitializedObject which wouldn't run the parameterless constructor. (And of course add a test for this.)

[ghost]

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

Issue Details

---

See the conversation here.

This code in SLE:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs

Lines 155 to 160 in 8a52f1e

	public override int Run(InterpretedFrame frame)
	{
	if (frame.Peek() == null)
	{
	frame.Pop();
	frame.Push(Activator.CreateInstance(_defaultValueType));

has the possibility of behaving incorrectly if/when dotnet/csharplang#99 is implemented in the future. If the value type has a parameterless constructor present in IL, Activator.CreateInstance would run that parameterless constructor. Which means the value you get back from Nullable<T>.GetValueOrDefault() could be different if run through this code than if you actually called Nullable<T>.GetValueOrDefault() on a "null" value.

We should instead change this to use GetUninitializedObject which wouldn't run the parameterless constructor. (And of course add a test for this.)

Author:	eerhardt
Assignees:	-
Labels:	area-System.Linq.Expressions
Milestone:	-

[GrabYourPitchforks]

See also the table I put in the description of #45397. It gives a few other scenarios where LINQ instantiates objects incorrectly.

[eerhardt]

If this call isn't changed to GetUninitializedObject, we need to revisit the suppression being added here:

https://github.com/dotnet/runtime/pull/47585/files/07c0fc14fbab9b092a2270083bd933cbee8b2248#r567222530

[eerhardt]

Closing in favor of #45397. There are a handful of scenarios that need fixing here, it doesn't make sense to track each scenario in a separate issue.