Feature: Export inner exceptions to dnSpy

[mitchcapper]

Rather than just grabbing the outer exception type and message grab it for inner exceptions as well.

This is a step towards #69

The overall strategy was:

• Get the same data for inner exceptions we get for the main exception itself (as possible) meaning type of exception and the message (and hResult).
• Refactor some functions to reduce any code duplication in the new code path
• Try to establish the same recursive pattern standard exception objects have with inner exceptions rather than stringifying or converting it at time of DbgException creation

It is mainly just for feedback/POC now as this mixes a whole bunch of approaches to see what is preferred.

• For public methods to add a new parameter that is not required would it be preferred for a new overload method to be created or is modifying the current function to have a new default parameter ok? The second would be cleaner but means any code that ends up mixed will throw an exception with the newer dll as the old method is gone. IE a plugin that uses it that hasn't been recompiled.
• I was not sure the best way to handle DbgObject base class. It didn't look like the data parameter got stuffed anywhere for DbgExceptions so I threw the InnerException data in there using another DbgException object. We could use another class for InnerException data but it seemed logical to tie to another DbgException so we have the additional methods and such available to us. The problem is DbgException inherits from DbgObject which makes sure data added is not classed from DbgObject. Could wrap the DbgException in an outer class (ie DbgInnerException which has a single property DbgException exception) that would bypass that check, but then the Close() function would be tricky. Even if DbgInnerException implemented iDisposable it would not be able to get the DbgDispatcher. Instead I hardcoded an exception into DbgObject for DbgException and took it a step further and made sure when DbgObject Close is called, it is also called on DbgException data. This could be more generalized to allow sub DbgObjects and similarly call Close on them, but I don't really have any core understanding of dnSpy. Why that assert is there, or what evil I may be unlocking. Given the limited InnerException data we could just switch to a new class with two strings and an int (hResult) and not worry about the DbgObject Close or check.
• Actually getting the InnerException object was abit trickier than expected. GetField does not work as it checks to make sure the reflected type is the same as the declared type and does not return it if private otherwise. To work around this I could call GetFields() to get all of them and then filter by name, but that seems like a waste. Instead I figured add an option for GetField. There are two ways I saw to do this. One is how I did, add another parameter allowTypeVarianceOnPrivateFields the other was to modifying the DmdBindingFlags and add an option like AllowTypeVarianceOnPrivateField. This being a mirror of the true BindingFlags class though seemed like a poor way to go too.
• As it is POC propagating AllowTypeVarianceOnPrivate* is not on getmethod, etc 100% yet despite them having similar checks.

Some potential negatives of the current code:

• No recursion protection, I don't think a loop is likely as InnerException is RO but im sure possible with a new _InnerException or similar
• It gets the entire InnerException tree for every exception even if we don't "care" about them (although generally still console logged). We could try defer processing InnerException but may need to keep more data/be careful about making sure everything gets released after.
• Not sure if it is worth trying to do something more like capture the stack trace or doing a ToString on the exception itself through eval. A break on an exception already lets you see the $exception in locals, calling ToString clearly could have potential other side effects for not a huge amount of value.

Assuming this is not the completely wrong way to go about this let me know some guidance on how you would like things handled and I will clean it up.

An exception thrown of:
throw new ArgumentNullException("Yes it was", new FileNotFoundException("Where is it Bobbbbbbbbb", "c:\\temp\\test.txt", new Exception("It fell off")));
returns in the main window showing:

I would take it a step further and likely add to the console log and messagebox as well.

[ElektroKill]

Hi, when it comes to modifying public APIs to add parameters, the general practice in dnSpy is to make a new overload instead of a default parameter due to the benefit of not needing recompilation for the couple of plugins that exist for dnSpy.

As for the additional modifications to the debugger metadata system, I don't think we really need them and we should just not use them for exceptions and instead use the CorDebug API to retrieve the values directly and process them. There is no advantage in using the DMD system here. Its main goal was to abstract away the access to metadata and values so that certain debugger components can work smoothly between CorDebug and Mono debugging. Since in this code path we are dealing only with CorDebug there is no need in using it. It also comes with some issues like not being able to retrieve the exception data, when it comes to heavily obfuscated or protected assemblies or exceptions that happen really early during the process lifetime. The CorDebug API does not have this problem. We already rely on the CorDebug API as the fallback method in case the DMD code fails so I think it would be acceptable to just use CorDebug here.

When it comes to the DbgObject, why did you choose to use the data storage to hold the inner exception? Can't we just add a new DbgException property to DbgException for nested exceptions? This seems much more intuitive.

As for recursion, we need some limits to prevent malicious exception objects from being exploited by a bad actor.

I don't think it's really necessary for adding stack trace info as I outlined initially in #69. The call stack tool window already serves this purpose.

Finally, as for actually showing the exception info to the user, I'm not really fond of making the status bar at the bottom of the dnSpy window show multiple lines. I much rather for the InnerException info to be included in the message box and debug output window.

[ElektroKill]

Hi, in commit 31fe70e I modified the exception reading code to always use the CorDebug API directly without going through the DMD system. Please update your PR and adapt your changes to the new code!