RFC: Include "F12 - go to decompiled source" in OmniSharp?

[christophwille]

Visual Studio already integrates the ILSpy decompilation engine as an experimental F12 go to decompiled source. Given how "small" such an integration is, wouldn't that also be of interest in the OmniSharp engine to have that sort of functionality in-box? (ie like this: if package has SourceLink, use that. If not, use our engine).

Maybe we should start a discussion with the https://github.com/OmniSharp/omnisharp-roslyn devs if that is even within their charter.

Our addin hasn't seen and proably won't see much additional activity feature-wise (it started out as a proof that we are actually properly x-plat). Thus having it integrated in the C# feature for VS Code might be way more appropriate.

[christophwille]

@filipw I don't want to "pollute" the OmniSharp repo with a discussion that might be totally inappropriate, that's why I tagged you here. Would that sort of feature be of value in OmniSharp (and part of its charter), or is that something that is not of interest feature-wise?

[filipw]

Hi sorry for the delay, I somehow missed it.

Yes we absolutely want to bring this feature into OmniSharp - it's been quite heavily asked for. At the moment we only support metadata as source which is very limited.

[christophwille]

ICSharpCode.Decompiler.dll is already included in Roslyn (but not latest, that somehow always takes some time). Please feel free to take inspiration from this repository, we actually would love to have it (F12 experience) part and parcel in OmniSharp!

[filipw]

I will have a look and come back to you with questions 😀

[christophwille]

No problem, @siegfriedpammer and I will be watching this space :-)

There are additional samples available, such as ilspycmd (https://github.com/icsharpcode/ILSpy/tree/master/ICSharpCode.Decompiler.Console) or even inside Roslyn you will find the integration code that Siegfried provided.

[siegfriedpammer]

See https://github.com/dotnet/roslyn/tree/master/src/EditorFeatures/CSharp/DecompiledSource for the Roslyn integration.

[christophwille]

@filipw saw your branch https://github.com/filipw/omnisharp-roslyn/tree/feature/decompile - anything we can help with?

[filipw]

thanks unfortunately I got side tracked with a lot of other things in OmniSharp which had taken priority 😫 I am planning to pick up on this soon... The branch is just a PoC but it shows that it's a viable spike. What I plan to do is to make it toggle-able with the MetadataService, so that when enabled it replaces completely Go-To-Metadata when invoked.

[christophwille]

Currently, VS is also using a toggle plus that copyright dialog (Yes -> decompile, No -> use good old metadata approach). However, they honor an additional one that you might want to consider given that VSCode is a MS product: https://github.com/dotnet/roslyn/blob/e704ca635bd6de70a0250e34c4567c7a28fa9f6d/src/EditorFeatures/Core/Implementation/MetadataAsSource/MetadataAsSourceFileService.cs#L121-L126 is the check for the SuppressIldasm attribute. The comments on the blog article https://devblogs.microsoft.com/visualstudio/decompilation-of-c-code-made-easy-with-visual-studio/ indicate that people really seem to care about this "be nice" attribute.... (haven't had a too close look at your code, you might already implicitly using that via Roslyn, so don't know)

[christophwille]

@filipw We are approaching Preview 3 of v6 - and would have a few spare cycles to help.

[filipw]

thanks, the latest code has been moved to the official feature branch (it's currently on top of latest master) https://github.com/OmniSharp/omnisharp-roslyn/tree/feature/decompile

[christophwille]

Did you encounter any problems, snags, things we could do differently to make it easier, any other suggestions?

[filipw]

i tried a few approaches, but ended up reflection calling into the internal Roslyn services, it's the simplest and this way the behavior appears to be pretty much the same

[filipw]

@christophwille actually, perhaps you could help here? I ran into a weird issue when trying to decompile code from assembly that depends on netstandard.dll. It only happens when I use the Roslyn services via reflection, so I would rather expect the problem to be there not in ILSpy, but anyway perhaps you have some idea

it tries to look for netstandard.dll' in the windows directory instead of at the correct assembly location. The offending code is here http://sourceroslyn.io/#Microsoft.CodeAnalysis.CSharp.EditorFeatures/DecompiledSource/AssemblyResolver.cs,99

System.IO.FileNotFoundException: Could not find file 'C:\Windows\netstandard.dll'.
File name: 'C:\Windows\netstandard.dll'
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access)
   at ICSharpCode.Decompiler.Metadata.PEFile..ctor(String fileName, PEStreamOptions streamOptions, MetadataReaderOptions metadataOptions)
   at Microsoft.CodeAnalysis.Editor.CSharp.DecompiledSource.AssemblyResolver.<Resolve>g__MakePEFile|4_0(IAssemblySymbol assembly)
   at Microsoft.CodeAnalysis.Editor.CSharp.DecompiledSource.AssemblyResolver.Resolve(IAssemblyReference name)
   at ICSharpCode.Decompiler.TypeSystem.DecompilerTypeSystem..ctor(PEFile mainModule, IAssemblyResolver assemblyResolver, TypeSystemOptions typeSystemOptions)
   at Microsoft.CodeAnalysis.Editor.CSharp.DecompiledSource.CSharpDecompiledSourceService.PerformDecompilation(Document document, String fullName, Compilation compilation, String assemblyLocation)
   at Microsoft.CodeAnalysis.Editor.CSharp.DecompiledSource.CSharpDecompiledSourceService.<AddSourceToAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Roslyn.CSharp.Services.Decompilation.DecompilationExternalSourceService.<GetAndAddExternalSymbolDocument>d__5.MoveNext() in C:\code\5\omnisharp-roslyn\src\OmniSharp.Roslyn.CSharp\Services\Decompilation\DecompilationExternalSourceService.cs:line 64
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Roslyn.CSharp.Services.Navigation.GotoDefinitionService.<Handle>d__4.MoveNext() in C:\code\5\omnisharp-roslyn\src\OmniSharp.Roslyn.CSharp\Services\Navigation\GotoDefinitionService.cs:line 73
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Endpoint.EndpointHandler`2.<GetFirstNotEmptyResponseFromHandlers>d__19.MoveNext() in C:\code\5\omnisharp-roslyn\src\OmniSharp.Host\Endpoint\EndpointHandler.cs:line 201
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Endpoint.EndpointHandler`2.<HandleRequestForLanguage>d__20.MoveNext() in C:\code\5\omnisharp-roslyn\src\OmniSharp.Host\Endpoint\EndpointHandler.cs:line 230
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Endpoint.EndpointHandler`2.<Process>d__16.MoveNext() in C:\code\5\omnisharp-roslyn\src\OmniSharp.Host\Endpoint\EndpointHandler.cs:line 131
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at OmniSharp.Stdio.Host.<HandleRequest>d__13.MoveNext() in C:code\5\omnisharp-roslyn\src\OmniSharp.Stdio\Host.cs:line 215"

[christophwille]

@siegfriedpammer I think you are better suited to answer this

[siegfriedpammer]

Yes, I wrote that code, however I am merely a consumer of another Roslyn API namely IAssemblySymbol/Compilation.GetReferencedAssemblySymbols().

In Visual Studio's case we retrieve all known references and use their file locations from IAssemblySymbol.Display, these usually point to the GAC, reference assemblies folders, etc.

I don't know anything about how OmniSharp uses Roslyn, maybe there's some difference in how the IAssemblySymbols are constructed. I think you are better off asking someone on the Roslyn team, how these APIs were intended to be used.

[filipw]

thanks. it seems to me we do things exactly the same ways as VS, but clearly something is off.

What the assembly resolver for ILSpy in Roslyn code does, is this http://sourceroslyn.io/#Microsoft.CodeAnalysis.CSharp.EditorFeatures/DecompiledSource/AssemblyResolver.cs,104

So it uses Display property from metadata reference. The problem is on this line http://sourceroslyn.io/#Microsoft.CodeAnalysis/MetadataReference/MetadataImageReference.cs,53 notice that Display returns display value (i.e. netstandard.dll) if present, or falls-back to FilePath.

It appears that in VS it only works cause Display is null and it goes through the fallback, but in our case Display always contains the short name and thus things fail 😀

I internalized all that code into OmniSharp (which is not great as there is a lot of copying and a lot of internal reflection, instead of a single top level reflection call) and switched to use FilePath propperty of the metadata reference and now everything seems to work. Not ideal but we may have to live with this now. Most optimally this line in Roslyn should be changed to use FilePath too instead of the fallback http://sourceroslyn.io/#Microsoft.CodeAnalysis.CSharp.EditorFeatures/DecompiledSource/AssemblyResolver.cs,104

[siegfriedpammer]

I am sorry for the confusion, I was referring to the IAssemblySymbol interface before... I got this mixed up - we are using Compilation.GetMetadataReference(IAssemblySymbol) to retrieve the MetadataReference instance first.

Most optimally this line in Roslyn should be changed to use FilePath too instead of the fallback.

The problem here is that FilePath is not part of Microsoft.CodeAnalysis.MetadataReference. I have not enough knowledge of the Roslyn APIs, whether there is a better implementation, but using the MetadataReference.Display property is what we were told to do by the Roslyn team.

Maybe @sharwell could step in and provide more information? Would it be acceptable to do an as cast to PortableExecutableReference and then use FilePath? Thank you very much!

[filipw]

yes the cast itself would work (as in, it would succeed) and in PortableExecutableReference Display and FilePath actually return the same thing http://sourceroslyn.io/#Microsoft.CodeAnalysis/MetadataReference/PortableExecutableReference.cs,37, but it wouldn't solve the problem.

The problem is if the metadata reference already is of runtime type MetadataImageReference, which is what I am observing, then MetadataImageReference overrides Display http://sourceroslyn.io/#Microsoft.CodeAnalysis/MetadataReference/MetadataImageReference.cs,49 so even if there is an upcast back to PortableExecutableReference, calling the property still would behave the same way

[filipw]

actually, ofcourse what I wrote is nonsense 😄 yeah this works fine for me:

            PEFile MakePEFile(IAssemblySymbol assembly)
            {
                // reference assemblies should be fine here, we only need the metadata of references.
                var reference = _parentCompilation.GetMetadataReference(assembly);
                Log("Loading from: {0}", ((PortableExecutableReference)reference).FilePath);
                return new PEFile(((PortableExecutableReference)reference).FilePath, PEStreamOptions.PrefetchMetadata);
            }

this is what I ended up using on the feature branch too https://github.com/OmniSharp/omnisharp-roslyn/blob/feature/decompile/src/OmniSharp.Roslyn.CSharp/Services/Decompilation/AssemblyResolver.cs#L96-L102

[christophwille]

Filip's PR is here https://github.com/OmniSharp/omnisharp-roslyn/pull/1751

[christophwille]

@filipw I saw that it was just merged. I assume you will be blogging / tweeting once it ships to end users - please let us know so we can retweet. Thanks.

Our extension will continue to exist - for one, we allow decompilation of user-selected assemblies (no project context), secondly it allows us to ship ics.d versions different from Roslyn, thirdly it has a tree UI similar to ILSpy itself.

[christophwille]

It seems to be in 1.21.17 https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csharp @filipw can you confirm? (how would I go about activating the feature?)

[filipw]

yes it's released now. To activate it you can do one of 2 things: create a global or local omnisharp.json (https://github.com/OmniSharp/omnisharp-roslyn/wiki/Configuration-Options#global-omnisharpjson)

And add the following config:

{
    "RoslynExtensionsOptions": {
        "enableDecompilationSupport": true
    }
}

In the future, we will add a shortcut setting directly into VS Code settings too.

@christophwille @siegfriedpammer actually, there is one more issue that you maybe can help with. On Windows everything works as expected, on Mono as well, for "regular" references (nuget packages or external DLLs), however when trying to navigate to BCL types on Mono we get

System.NotSupportedException: Decompiling types that are not part of the main module is not supported.
  at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileType (ICSharpCode.Decompiler.TypeSystem.FullTypeName fullTypeName) [0x00049] in <63c75710ce364af8bb694956fa724da0>:0

Is it expected that it would behave like this when the application is running inside a Mono VM?

[siegfriedpammer]

That depends on how you create the decompiler type system. It is necessary that all types you try to decompile are located in the main module, that is, the first parameter of the DecompilerTypeSystem ctor. We haven't tested the decompiler that much on Mono, however I don't see a reason why it would behave differently.

Could you point me to the location were you instantiate the decompiler so I can take a look? Thank you very much!

[filipw]

thanks a lot - it's here https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn.CSharp/Services/Decompilation/OmniSharpCSharpDecompiledSourceService.cs#L128 this code is actually internalized from Roslyn. aside from the 1 line change (https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn.CSharp/Services/Decompilation/AssemblyResolver.cs#L101) and some non-public calls that needed to be converted to reflection, this should be 1-to-1 the same decompilation service as running in VS (maybe that's why it has problems on Mono 😀)

[siegfriedpammer]

I think I would have to debug this in order to get some insight as to why it fails. Is it possible to reproduce this in a Ubuntu VM? I currently do not have access to my MBP.

[filipw]

I will try to create a repro. Meanwhile, I added some extra logging hope this helps shed some light.

So the following code:

            var fullTypeName = new FullTypeName(fullName);

            // Load the assembly.
            var file = new PEFile(assemblyLocation, PEStreamOptions.PrefetchEntireImage);

            var assemblyResolver = new AssemblyResolver(compilation, _loggerFactory);
            var settings = new DecompilerSettings();
            var typeSystem = new DecompilerTypeSystem(file, assemblyResolver, settings);
            var type = typeSystem.FindType(fullTypeName.TopLevelTypeName).GetDefinition();

            logger.LogInformation($"Decompiling {fullName}, top level typename: {fullTypeName.TopLevelTypeName}, from location: {assemblyLocation}, typeSystem");
            logger.LogInformation($"Type in typesystem {type.FullTypeName}");
            logger.LogInformation($"Type parent module {((MetadataModule)type.ParentModule).FullAssemblyName}");
            logger.LogInformation($"Typesystem main module {typeSystem.MainModule.FullAssemblyName}");

prints this on Mono:

[info]: OmniSharpCSharpDecompiledSourceService
        Decompiling System.Collections.Generic.Dictionary`2, top level typename: System.Collections.Generic.Dictionary`2, from location: /Library/Frameworks/Mono.framework/Versions/6.10.0/lib/mono/4.5/Facades/System.Collections.dll, typeSystem
[info]: OmniSharpCSharpDecompiledSourceService
        Type in typesystem System.Collections.Generic.Dictionary`2
[info]: OmniSharpCSharpDecompiledSourceService
        Type parent module System.Collections, Version=4.1.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
[info]: OmniSharpCSharpDecompiledSourceService
        Typesystem main module System.Collections, Version=4.0.10.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a`

In this case the problem is we are in net472 project (omnisharp) but working on a net core 3.1 project. I am trying to decompile something from System.Collections and it ends up resolving to a Mono facade dll (notice the path) instead of the Microsoft.NETCore.App.Ref path. At the end we have a mismatch on versions.

This is caused by this special handling of ref assemblies. When I get rid of this code https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn.CSharp/Services/Decompilation/OmniSharpCSharpDecompiledSourceService.cs#L72-L91 (which, as mentioned, comes from http://sourceroslyn.io/#Microsoft.CodeAnalysis.CSharp.EditorFeatures/DecompiledSource/CSharpDecompiledSourceService.cs,51) then it seems to work fine.

Decompilation works then and I see:

[info]: OmniSharpCSharpDecompiledSourceService
        Decompiling System.Collections.Generic.Dictionary`2, top level typename: System.Collections.Generic.Dictionary`2, from location: /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Ref/3.1.0/ref/netcoreapp3.1/System.Collections.dll, typeSystem
[info]: OmniSharpCSharpDecompiledSourceService
        Type in typesystem System.Collections.Generic.Dictionary`2
[info]: OmniSharpCSharpDecompiledSourceService
        Type parent module System.Collections, Version=4.1.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
[info]: OmniSharpCSharpDecompiledSourceService
        Typesystem main module System.Collections, Version=4.1.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a

Do you know why in VS they have the extra search in GAC? I think we can safely remove it. In fact that code part returns null on Windows - so that's why it works there...

[siegfriedpammer]

In this case the problem is we are in net472 project (omnisharp) but working on a net core 3.1 project. I am trying to decompile something from System.Collections and it ends up resolving to a Mono facade dll (notice the path) instead of the Microsoft.NETCore.App.Ref path.

Reference assemblies are useless for a decompiler (as they only contain public metadata and zero implementation). VS usually works with reference assemblies, because these suffice for most IDE features, but the decompiler needs an assembly containing the actual implementation.

I don't quite understand: The assembly from the Microsoft.NETCore.App.Ref path contains IL bytes, but the Mono facade does not?

[filipw]

The mono facade does too, but there is a mismatch of versions (assembly from .NET core 3.1 is newer than in Mono GAC as it only matches on partial name) so this condition is not satisfied https://github.com/icsharpcode/ILSpy/blob/master/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs#L826

When I remove any attempt to handle ref assemblies, at least the ref assemblies gets decompiled. This is of course quite useless, but at least better than the exception. Also - most importantly - that's the same behavior I see in VS anyway - ref assemblies don't work correctly for me in VS2019 and I just see decompiled ref assembly with properties and methods that throw null. This is acceptable for now I'd say.

[siegfriedpammer]

Also - most importantly - that's the same behavior I see in VS anyway - ref assemblies don't work correctly for me in VS2019 and I just see decompiled ref assembly with properties and methods that throw null.

This is definitely not helpful and not what I had in mind for the feature, but due to some missing functionality in Visual Studio, that is, answering the question "Which file is loaded at runtime for the given assembly?", the feature will always misbehave in some cases. That question is - of course - very hard to answer, depending the project configuration, etc., so we might never get it to work properly everywhere.

The VS debugger team afaik uses the assemblies loaded into the debuggee, so they do not have this problem.

When I remove any attempt to handle ref assemblies, at least the ref assemblies gets decompiled. This is of course quite useless, but at least better than the exception.

If that "fixes" the problem for OmniSharp on all supported platforms, please go for it. As I said before, the "runtime assembly" question is hard to answer. Sorry, for not being able to provide a good solution to this.

ILSpy itself uses a lot of complex code to correctly handle .NET Framework, .NET Core and Portable assemblies and their references. And even with that we might not get it right, but in our case the user still has the possibility to manually add the correct assemblies to the current assembly list.

[filipw]

Thanks, this is definitely complex, especially when the code targeted and the code running are different runtimes.

What I have managed to find out is that the problem in this code that we mirrored from VS, is that it ends up using a Mono specific implementation of some Roslyn GAC proxy (http://sourceroslyn.io/#Microsoft.CodeAnalysis.Scripting/Hosting/Resolvers/MonoGlobalAssemblyCache.cs,fa2b10cde63dabec) that is not in use in VS at all - since, well, it's mono specific. It works in a questionable way - it was built for C# scripting to handle the x-plat semantics there and it for example picks 4.5 facades even when 4.7.2 facades are available too and so on. Then the mismatch causes the not supported exception which makes things even worse.

I think we take a step back and remove any attempts to handle ref assemblies for now, and (it appears to me) this would bring it on par with the current IDE VS experience anyway. We can then improve upon this later progressively.

[christophwille]

And add the following config:

{
    "RoslynExtensionsOptions": {
        "enableDecompilationSupport": true
    }
}

In the future, we will add a shortcut setting directly into VS Code settings too.

Are there current plans for the shortcut setting yet?

[filipw]

it's planned but apparently there need to be some things defined first on how the legal disclaimer is shown and so on @JoeRobich knows more

[christophwille]

Ah yes, something along the lines of the dialog that pops up in Visual Studio.

[JoeRobich]

@christophwille You can try this in our pre-release build https://github.com/OmniSharp/omnisharp-vscode/releases/tag/v1.21.19-beta1

[christophwille]

That should be easy enough for everyone to find :-)

I am closing this issue - if you have further questions (@filipw or @JoeRobich) about specifics of our engine, feature requests or bugs, please use https://github.com/icsharpcode/ILSpy/issues. We are more than happy to help.

[jiristary]

Thank you very much for this feature! I couldn't use VS Code for C# without it. Would it be possible that features like "Find References" would work as expected in decompiled files? E.g. finding references in that decompiled source as well as finding references in my project.