ILVerify falsely reports a field being modified

[masonwheeler]

Description

ILVerify is usually pretty good at finding invalid IL in your code. But I've stumbled across something that's either a false positive or a bug in Roslyn's C# codgen, and I'm guessing it's the former, because the error message does not match what's in either the C# code or the decompiled CIL. (If this is actually a Roslyn problem, let me know and I'll refile this bug on the Roslyn repo.)

Reproduction Steps

Build the following, then examine the resulting DLL in ILVerify

using System.Reflection.Emit;

namespace CorruptRoslynCodegen
{
	internal class Program
	{
		static void Main(string[] args)
		{
			Console.WriteLine("Hello, World!");
		}

		protected static bool IsStobj(OpCode code)
		{
			return OpCodes.Stobj.Value == code.Value;
		}
	}
}

Expected behavior

As the flagship .NET language, it's expected that the official C# compiler does not produce invalid IL.

It is also expected that official IL verification tools do not report invalid IL where none exists.

Actual behavior

[IL]: Error [InitOnly]: [C:...\CorruptRoslynCodegen.dll : CorruptRoslynCodegen.Program::IsStobj([S.P.CoreLib]System.Reflection.Emit.OpCode)][offset 0x00000001] Cannot change initonly field outside its .ctor.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 9, most recent daily build as of 6/3/2024
Windows 10, x64

Other information

No response

[dotnet-policy-service]

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

[MichalStrehovsky]

The message could be better (it matches peverify.exe per dotnet/corert#4911 though), but the gist is that ILVerify checks whether the input is verifiable and ldsflda with an initonly field is documented as not verifiable in the ECMA-335 spec. We track adding a weaker mode to ILVerify that checks for valid IL in #37391.

Roslyn generates ldsflda because the OpCode struct is marked readonly and therefore this doesn't violate C# type safety rules. But the code is unverifiable and ILVerify will warn.

[masonwheeler]

So basically, the ldsflda sets things up to that further down the line, something else could violate type safety, but nothing is actually doing so?

[jkotas]

ILVerify implements .NET Framework 4.0 set of verification rules. These rules were not officially amended for the new constructs such as readonly structs and methods in this case. We would be happy to merge changes to amend the verification rules for the new constructs such as this one.

[masonwheeler]

These rules were not officially amended for the new constructs such as readonly structs and methods in this case.

Are those rules unofficially described anywhere besides compiler source code?

[jkotas]

The theoretical verification rules are sometimes discussed in the C# design notes: https://github.com/search?q=repo%3Adotnet%2Fcsharplang%20verification . I do not see the discussion for this specific case.

I think the rules for this specific case would be:

• Introduce concept of readonly managed pointer (different from the existing controlled-mutability managed pointer)
• Reads from readonly managed pointer, calling readonly methods, conversion of regular managed pointer to readonly managed pointer are verifiable
• Writes to readonly managed pointer, conversion of readonly managed pointer to regular managed pointer are non-verifiable

[reflectronic]

https://github.com/dotnet/designs/blob/7e185087c28a3ae0d3b8b12c81d2ce2a49f88289/proposed/verifiable-ref-readonly.md ?

See #57444

[jkotas]

Resolving as duplicate of #57444