Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh ILVerify and file runtime issues as needed #69860

Closed
jcouv opened this issue Sep 8, 2023 · 1 comment · Fixed by #71544
Closed

Refresh ILVerify and file runtime issues as needed #69860

jcouv opened this issue Sep 8, 2023 · 1 comment · Fixed by #71544
Assignees
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Sep 8, 2023

Before filing issues on ILVerify, we should refresh our ILVerify bits. If those issues persist, then we should file runtime bugs.

Work-in-progress at https://github.com/dotnet/roslyn/compare/main...jcouv:roslyn:update-ilverify?expand=1


See Nullable_ImplicitConversion_Byte test

Here's a repro from the roslyn repo. It should contain all the necessary details. In short, ILVerify complains for seemingly legitimate IL that the compiler generates (also shown below).

        [Fact]
        public void TODO2()
        {
            string src = """
using System;

System.Console.Write(42);

partial class Program
{
    static void M()
    {
        M2(new byte[] { 1, 2, 3 });
    }
    public static void M2(ReadOnlySpan<byte> items) { }
}
""";
            var comp = CreateCompilation(new[] { src, s_collectionExtensions }, targetFramework: TargetFramework.Net80);
            comp.VerifyDiagnostics();

            var verifier = CompileAndVerify(comp, expectedOutput: IncludeExpectedOutput("[1, 2, 3],"), verify: Verification.FailsPEVerify);

            // ILVerify:
            // [M]: Cannot change initonly field outside its .ctor. { Offset = 0x0 }
            // [M]: Field is not visible. { Offset = 0x0 }
            // [M]: Unexpected type on the stack. { Offset = 0x6, Found = address of '[78cb4f30-abc1-41ca-b5d2-939830104c72]<PrivateImplementationDetails>+__StaticArrayInitTypeSize=3', Expected = Native Int }
            verifier.VerifyIL("Program.M", """
{
  // Code size       22 (0x16)
  .maxstack  2
  IL_0000:  ldsflda    "<PrivateImplementationDetails>.__StaticArrayInitTypeSize=3 <PrivateImplementationDetails>.039058C6F2C0CB492C533B0A4D14EF77CC0F78ABCCCED5287D84A1A2011CFB81"
  IL_0005:  ldc.i4.3
  IL_0006:  newobj     "System.ReadOnlySpan<byte>..ctor(void*, int)"
  IL_000b:  call       "MyCollection<byte> MyCollectionBuilder.Create<byte>(System.ReadOnlySpan<byte>)"
  IL_0010:  newobj     "MyCollection<byte>?..ctor(MyCollection<byte>)"
  IL_0015:  ret
}
""");
   }
@jcouv jcouv added this to the 17.9 milestone Sep 8, 2023
@jcouv jcouv self-assigned this Sep 8, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Sep 8, 2023
@jcouv jcouv added Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 8, 2023
@jcouv
Copy link
Member Author

jcouv commented Dec 15, 2023

The upgrade yield a bunch of failures. We suspect it is due to how assemblies are resolved or what kind of assemblies we're using (ref vs. real assemblies), but haven't yet identified the root cause.
Capturing some notes on investigation below:

Below are two examples of such failures. The IL hasn't changed and is fine, but something about our netstandard references seems to be tripping ILVerify up.

       [Fact]
       public void AttributeAndDefaultValueArguments_01()
       {
           var source = @"
sing System;
A]
ublic class A : Attribute
   public A(object a = default(A)) { }

A(1)]
lass C
   public static void Main()
   {
       typeof(C).GetCustomAttributes(false);
   }
";
           var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
           // ILVerify fails
           // [Main]: Unexpected type on the stack. { Offset = 0x5, Found = value '[netstandard]System.RuntimeTypeHandle', Expected = ref '[netstandard]System.RuntimeTypeHandle' }
           var verifier = CompileAndVerify(compilation);
           verifier.VerifyIL("C.Main", """
               {
                 // Code size       18 (0x12)
                 .maxstack  2
                 IL_0000:  ldtoken    "C"
                 IL_0005:  call       "System.Type System.Type.GetTypeFromHandle(System.RuntimeTypeHandle)"
                 IL_000a:  ldc.i4.0
                 IL_000b:  callvirt   "object[] System.Reflection.MemberInfo.GetCustomAttributes(bool)"
                 IL_0010:  pop
                 IL_0011:  ret
               }
               """);
       }
        [Fact]
        public void IsExpression_SwitchDispatch_String()
        {
            var source = """
class C
{
    public static void Main()
    {
        System.Console.Write(
              Test("0") == false
            & Test("1")
            & Test("2")
            & Test("3")
            & Test("4")
            & Test("5")
            & Test("6")
            & Test("7")
            & Test("8")
        );
    }  
    public static bool Test(string a)
    {
        return (a is "1" or "2" or "3" or "4" or "5" or "6" or "7" or "8");
    }
}
""";
            // ILVerify fails:
            // [Test]: Unexpected type on the stack. { Offset = 0x4, Found = ref 'string', Expected = ref '[netstandard]System.String' }
            var compilation = CompileAndVerify(source, expectedOutput: "True");
            compilation.VerifyIL("C.Test", """
{
  // Code size       73 (0x49)
  .maxstack  2
  .locals init (bool V_0,
                int V_1,
                char V_2)
  IL_0000:  ldarg.0
  IL_0001:  brfalse.s  IL_0045
  IL_0003:  ldarg.0
  IL_0004:  call       "int string.Length.get"
  IL_0009:  stloc.1
  IL_000a:  ldloc.1
  IL_000b:  ldc.i4.1
  IL_000c:  bne.un.s   IL_0045
  IL_000e:  ldarg.0
  IL_000f:  ldc.i4.0
  IL_0010:  call       "char string.this[int].get"
  IL_0015:  stloc.2
  IL_0016:  ldloc.2
  IL_0017:  ldc.i4.s   49
  IL_0019:  sub
  IL_001a:  switch    (
        IL_0041,
        IL_0041,
        IL_0041,
        IL_0041,
        IL_0041,
        IL_0041,
        IL_0041,
        IL_0041)
  IL_003f:  br.s       IL_0045
  IL_0041:  ldc.i4.1
  IL_0042:  stloc.0
  IL_0043:  br.s       IL_0047
  IL_0045:  ldc.i4.0
  IL_0046:  stloc.0
  IL_0047:  ldloc.0
  IL_0048:  ret
}
""");
        }

@jcouv jcouv added the 4 - In Review A fix for the issue is submitted for review. label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant