Decompiling types that aren't in the main module

[davidwengier]

Hi,

I work on the Roslyn team, and as I'm sure you know, we use ICSharpCode.Decompiler to show decompiled source inside Visual Studio. We're seeing users hit this NotSupportedException and wondering if its something you would welcome an issue for, or if its not something that would be likely to change, for architectural or other reasons?

I can't know exactly how many users are hitting it, but in the last 7 days the absolute maximum possible would be 2% of users, so its not a huge proportion that's for sure.

Unfortunately I also can't give you exactly repro steps to know what sort of code they're decompiling, in order to judge whether its something that is likely to continue to be a trend, though potentially if there is interest I can see about trying to find out.

Let me know what you think.

[siegfriedpammer]

This sounds like a bug in the VS integration. CSharpDecompiler is designed to only load the main module as a full image and all the other references are loaded metadata-only to reduce the memory footprint, so there is no IL code that could be decompiled. Do you have a stack trace that shows the problem?

[davidwengier]

I don't think a stack trace wouldn't be interesting here - we are simply constructing a CSharpDecompiler instance and calling DecompileType, for some type that comes from user code. The Roslyn code that does this is here: https://github.com/dotnet/roslyn/blob/main/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs#L98-L107

To be clear, this exception is not causing any issues, and has no user impact, its just a signal that got flagged to us, via automatic exception tracking tech we have in VS. If the answer is that this behaviour is by design and not going to change, thats 100% fine, we can either update our code to not call ILSpy in these situations, or we can simply ignore the signal.

[jasonmalinowski]

main module as a full image and all the other references are loaded metadata-only to reduce the memory footprint, so there is no IL code that could be decompiled.

Is that configurable in this case?

[siegfriedpammer]

Is that configurable in this case?

Yes, you can construct full image PEFiles for all assemblies in your IAssemblyResolver implementation.

[dgrunwald]

The decompiler only has somewhat limited support for multi-module assemblies.
You can decompile code outside the main-module, but you will need to call the CSharpDecompiler constructor with the PEFile that actually contains the type to be decompiled. If you call the constructor with the assembly's main module, you can only decompile types from that main module.
Our terminology is a bit confusing here: the "main module" of the DecompilerTypeSystem is the PEFile passed to the decompiler constructor, which is not necessarily the main module of any assembly.

[siegfriedpammer]

I have just updated the exception message, to clarify what the problem is.

@jasonmalinowski @davidwengier Unfortunately, the decompiler would fall apart, if we were to remove this check.

Some reasons I could think of:

• In many parts of the code, we assume that typeSystem.MainModule refers to the module containing the IL being decompiled.
• We assume that IModule.PEFile is not null for the module containing the IL being decompiled.
• If a DecompilerTypeSystem with missing known types (i.e., types such as void, bool, but also IEnumerable or Task, etc.) is constructed, we inject the missing known types as part of a MinimalCorlib reference. Of course, because these are synthesized, there is no PEFile backing them. If one were to decompile System.Int32 using such a DTS, things would go horribly wrong pretty quickly.

[CyrusNajmabadi]

@siegfriedpammer I see that you helped write most/all of our AssemblyResolver implementation. Could you let me know if you have an idea of where in this impl you were thinking woudl need to be updated.

For context, i have no context/understanding of any of htis system. But the issue is currently assigned to me, so i'm seeing out subject-matter-expert help to help ensure the best fix goes in the right location. Thanks!

[siegfriedpammer]

As explained in my previous comment I don't think that just loading all assemblies with PEStreamOptions.PrefetchEntireImage will help. Would it be possible for you to provide some information on the nature of the metadataReference and assemblyLocation parameters passed to us? (see https://github.com/dotnet/roslyn/blob/main/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs#L44)

The decompiler expects that the type described by fullName is contained (the actual TypeDef, not only an ExportedType) in the assembly that assemblyLocation points to. I am not really sure what to expect from a MetadataReference, it seems that this can be anything. I hope, you can shed some light on how the AddToSourceToAsync method is used by VS. Thanks!

[CyrusNajmabadi]

Hey @siegfriedpammer i can try my best here :)

MetadataReference is normally a reference to an actual PEFile containing hte metadata and IL you would expect in a .net dll. "assemblyLocation" is intended to be its location on disk. If we see that the assembly we're looking at has the ReferenceAssemblyAttribute set for it, we do try to find the actual impl dll for it in a few locations (like nuget path, or GAC).

--

Note: i'm approaching this issue from two different perspectives.

1. Just stop crashing. Figure out a minimal way to update the code to pass data in that you won't throw on. This may mean some niche scenarios do not work, but it's still better than them crashing.
2. Figure out how to exactly map things properly from roslyn semantics to yours. This would make more scenarios work, but might take a lot longer and be more complex.

Right now, i would just prefer '1'. We haven't gotten any information that indicates that this is a mainline scenario. So i'd prefer to just do what we can on our end to prevent your side from throwing. Is there some simple check we can do on our end to address this?

Thanks! :)

[siegfriedpammer]

I think, to "just make it stop crashing" the following fix should be enough:

diff --git a/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs b/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs
index cce225223bf..5c755d317f0 100644
--- a/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs
+++ b/src/EditorFeatures/CSharp/DecompiledSource/CSharpDecompiledSourceService.cs
@@ -103,6 +103,10 @@ public static async Task<Document> FormatDocumentAsync(Document document, Syntax
 
             var fullTypeName = new FullTypeName(fullName);
 
+            var type = decompiler.TypeSystem.MainModule.GetTypeDefinition(fullTypeName);
+            if (type is null)
+                return null;
+
             // Try to decompile; if an exception is thrown the caller will handle it
             var text = decompiler.DecompileTypeAsString(fullTypeName);

Essentially this tries to resolve the type name in the main module and only if that's successful, the decompiler is called.

[CyrusNajmabadi]

excellent. Thanks!