dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

InvalidProgramException error message is unhelpful. #101321

Open masonwheeler opened 5 months ago

masonwheeler commented 5 months ago

Description

Sub-issue of #101298.

There is an error in this code, but the error message that gets raised in response does nothing to help with debugging the problem.

Reproduction Steps

using System.Reflection.Emit;
using System.Reflection;

namespace InvalidProgram
{
    internal class Program
    {
        static void Main(string[] args)
        {
            try
            {
                var type = BuildType();
                var method = type.GetMethod("Invalid")!;
                var result = method.Invoke(null, []);
                Console.WriteLine(result);
            } catch (Exception ex)
            {
                Console.WriteLine(ex.ToString());
            }

        }

        private static Type BuildType()
        {
            AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new("MyAssembly"), AssemblyBuilderAccess.RunAndCollect);
            ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("MyAssembly");
            var typeBuilder = moduleBuilder.DefineType("MyType", TypeAttributes.Public, typeof(ValueType));
            var extractor = typeBuilder.DefineMethod("Invalid", MethodAttributes.Public | MethodAttributes.Static, typeof(int), []);
            var il = extractor.GetILGenerator();
            il.Emit(OpCodes.Ldc_I4_1);
            il.Emit(OpCodes.Ldc_I4_1);
            il.Emit(OpCodes.Ret);

            return typeBuilder.CreateType();
        }

    }
}

Expected behavior

This will error out on the call to method.Invoke. A useful error message giving some clue as to the nature of the problem that would meaningfully aid in debugging it is expected.

Actual behavior

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.InvalidProgramException: Common Language Runtime detected an invalid program.
   at MyType.Invalid()
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at InvalidProgram.Program.Main(String[] args) in C:\Users\mason\source\repos\Repro2\InvalidProgram\Program.cs:line 14

This tells me that I have "an invalid program." OK, what am I supposed to do with that knowledge? Is the problem in the IL? In the metadata? Is it a problem with the method I invoked? With the type the method is on? With another method that this method is calling? (Yes, this particular example doesn't have any of those, but it easily could.)

The problem is an unbalanced CIL evaluation stack on the method MyType.Invalid. It would be nice if the error message said so.

Regression?

No response

Known Workarounds

No response

Configuration

.NET Core 8, Windows 10, x64.

Other information

No response

jkotas commented 5 months ago

What does ILVerify report for this code?

As discussed in https://github.com/dotnet/runtime/issues/63198 that you have filled a few years ago, our solution to help compiler writers to validate their output is ILVerify tool. It is by design that the runtime is not robust against invalid IL and metadata and that it can produce hard to diagnose crashes.

masonwheeler commented 5 months ago

@jkotas Oh wow, I had forgotten about that issue!

What does ILVerify report for this code?

Not sure. This is actually purely in-process refemit code, on a completely different project unrelated to my compiler work.

And my point from the other issue remains valid: I'm not asking for IL verification that you don't want to provide. What I'm asking for is, when the code that is already there, that you are providing, raises an error, it should explain itself clearly.

jkotas commented 5 months ago

This is where the exception is thrown in this case: https://github.com/dotnet/runtime/blob/907eff84ef204a2d71c10e7cd726b76951b051bd/src/coreclr/jit/importer.cpp#L11306 . Imbalanced IL stack is just one of the possible ways how the JIT can end up in this spot. Providing good actionable error message about imbalanced stack would require passing around more information in the JIT and doing more checks upfront, ie replicating what IL verify does.

masonwheeler commented 5 months ago

You make a decent point here. But, three things.

1) There's an error message there that's an order of magnitude better than the one that gets surfaced in the exception. Even if it isn't perfectly specific, it tells me that there's something wrong with a stack and the end of a BB, which isn't too difficult to interpret as "basic block" if you know a bit of compiler theory. (Or to look up on Google or StackOverflow if you don't.) This points you in the right direction far better than the information-less generic message we're currently getting. 2) Saying "just use ILVerify" is unhelpful for in-process refemit generation that isn't being saved to a file that can be verified. 3) Saying "just use ILVerify" is unhelpful when saving to files from refemit is not even supported in Core yet. (Yes, I've seen .NET 9 preview 3. Yes, I've downloaded it and am playing around with it. The same cannot be said of the majority of devs running into errors like this, though.)

jkotas commented 5 months ago

Right, we have been working towards fixing 2 and 3.

masonwheeler commented 5 months ago

2 and 3?

Saving refemit to a file will help to fix 3, and that's pretty cool, but what have you been working on to get ILVerify-type diagnostics in-process? I haven't heard anything about that.

jkotas commented 5 months ago

Once 3 is implemented and you hit a bad IL problem with the in-proc generation, you can write a small helper that reruns it and persist to assembly. We have a helper like that for RegEx in this repo: https://github.com/dotnet/runtime/blob/907eff84ef204a2d71c10e7cd726b76951b051bd/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexAssemblyCompiler.cs#L39-L40

masonwheeler commented 5 months ago

That looks pretty useful. Unfortunately, I just tried to do that to debug an InvalidProgramException that currently has me stumped, and tripped over the problem @buyaa-n mentioned here, that remains unfixed in the latest preview.

As the link you posted above shows, the JIT does have useful information. Is there any way to get access to it before it's thrown away?

jkotas commented 5 months ago

As https://github.com/dotnet/runtime/issues/101321#issuecomment-2067510049 shows, the JIT does have useful information. Is there any way to get access to it before it's thrown away?

Build your own debug flavor of the JIT and enable JIT logging. You need to be familiar with JIT internals to interpret what's going on. https://github.com/dotnet/runtime/issues/63198#issuecomment-1002914009

masonwheeler commented 5 months ago

This just keeps getting better. While trying to track down the source of this difficult problem by commenting out various parts of my code generation to see what causes it to stop happening, I managed to get a different error instead: ExecutionEngineException. According to official documentation, this is impossible:

ExecutionEngineException previously indicated an unspecified fatal error in the runtime. The runtime no longer raises this exception so this type is obsolete.

jkotas commented 5 months ago

The documentation is correct - runtime does not raise this exception. Visual Studio displays fatal crashes and fail fasts as ExecutionEngineException for some reason: https://github.com/dotnet/runtime/issues/63244#issuecomment-1374158706

masonwheeler commented 5 months ago

@jkotas This is driving me nuts! After correcting several flaws along the way, I'm now getting another seemingly-impossible error. Any insights into what in the world could cause dynamic invocation of a method generated via refemit to throw System.EntryPointNotFoundException, of all things?!?

jkotas commented 5 months ago

It is likely a type mismatch. This exception means that you are trying to call a method that is not implemented by the target.