runtime: InvalidProgramException is extremely unhelpful

Description

The documentation for InvalidProgramException states right up front that:

Generally this [exception being thrown] indicates a bug in the compiler that generated the program.

Therefore the people most likely to see it are those working on compilers or similar code generation tools. (It’s certainly possible that other people could see it if untested compiler bugs are released into the wild, but hopefully the bulk of them will stay inside of the realm of compiler development!)

Exceptions, by design, are full of information meant to make debugging easier. The class describes the general type of problem, the message gives specifics, the stack trace shows you exactly where the error occurred, etc. So you’d expect an exception caused by a bad compiler, whose primary expected audience is compiler developers, to give them a decent clue as to what their compiler did wrong. Unfortunately, InvalidProgramException fails pretty hard at this.

Reproduction Steps

Modify Roslyn’s code generator to omit the leave opcode at the end of a try body.
Compile an otherwise-valid test case containing a try block.
Run the test case.

Expected behavior

The exception that comes up should tell you that there’s a try body with no leave instruction. The JIT knows which IL rule was violated, so it ought to be able to convey this information to the user.

Actual behavior

You get a generic error message that says nothing whatsoever about the nature of the problem. All you know is “something went wrong,” during the JITting of this method.

Other information

This is one specific example that I recently discovered can cause this error. A glance at the JIT source suggests there are plenty of others. Ideally, each rule violated that leads to this exception being thrown should return an informative error message explaining what went wrong.

About this issue

Original URL
State: open
Created 3 years ago
Reactions: 10
Comments: 35 (34 by maintainers)

Most upvoted comments

It’s running into the “A single protected block shall have exactly one handler associated with it” rule from the ECMA-335 spec.

There’s a try block that has a filter, fault, and finally handler:

  .try IL_000a to IL_0015 filter IL_0015 handler IL_0023 to IL_0032
  .try IL_000a to IL_0015 fault handler IL_0032 to IL_003d
  .try IL_000a to IL_0015 finally handler IL_003d to IL_0048

We would be happy to accept a pull request that adds this validation to ILVerify.

MichalStrehovsky on Jan 5, 2022

Could this just be additional information tacked onto the existing InvalidOperationException? For example, it could expose this as additional informaiton in the .Data property. That main message woudl still be localized as normal. This would just be additional info someone could poke at to learn more.

This kind of goes against the spirit of localization. The purpose of localization is to give even ground to people who don’t speak English.

(I didn’t think someone can be successful with computers without speaking English until I met my sister-in-law’s husband who is a firmware engineer at Nikon. I’m sure he does a lot of cool stuff but I can’t talk to him about any of it because he only speaks Japanese and I only know like 50 words in Japanese. There’s a lot of people like him in Japan. Turns out my view was biased by the fact that I speak a niche language and I definitely wouldn’t be able to succeed with that language without also learning English.)

MichalStrehovsky on Dec 30, 2021

However, I still maintain that code that throws an exception indicating there’s a problem that needs to be fixed has a responsibility to provide useful information about the problem so that it can be fixed.

Stop doing that. It’s annoying and counterproductive."

@masonwheeler as discussed, the user base size for these APIs is tiny and has generally been served fine with the current approach. Given finite resources to expend on the breadth of functionality that is in the runtime, this is going to be low on the list. If you are interested in working to improve things here, your help would be welcome. However, berating because you don’t like that this particular system hasn’t gotten the deep attention that would make it better for a tiny subset of consuming devs isn’t productive or useful for winning friends and changing minds.

It seems already like there’s a path foward to expose this information. It doesn’t appear to be particularly crazy code or some very onerous c++. At this point, absent anything new coming to light, the best way to get to the outcome you’re looking for would likely be to just contribute a PR. I do believe if it fit the classification i laid out above, it would likely be approved. That would quickly get you to the state where you got the info you wanted for any future work you’re doing in your particular compiler.

CyrusNajmabadi on Dec 30, 2021

If you’re building a compiler, you want to know the entire output of the compiler is valid, not just the parts that get executed/JITted. Sure, a more detailed InvalidProgramException could help, but it should not be the thing a compiler developer relies on because it’s annoying having to execute all the produced code to ensure it’s valid.

I think the solution to this problem is an external tool. JIT will by design only look at code that is getting executed. One could force the JIT to compile everything, but even then one might have trouble figuring out how to validly instantiate constrained generics. The IL might be acceptable for RyuJIT because of how RyuJIT is implemented, but it might not be acceptable to Mono’s JIT. The JIT (or the runtime) is not a good validator.

I think it’s a better investment to add this to ILVerify if it’s missing. Now, ILVerify currently checks for verifiable IL. There’s an issue somewhere to add a mode that checks for valid instead of verifiable IL. The concept of verifiable IL kind of lost its meaning after execution models like Silverlight died.

MichalStrehovsky on Dec 30, 2021