Repository navigation
Feature request: Provide PeVerify without assembly loading or a way to assembly unload #3986
Description
Background
In the Roslyn compiler, we currently PeVerify a large amount of our emitted code code using CLR helpers in our tests, see http://source.roslyn.io/Roslyn.Test.Utilities/R/30b8bf380165b91a.html for the CLR call and the calls to http://source.roslyn.io/Roslyn.Compilers.CSharp.Test.Utilities/R/d49dabc2badbc8f0.html for examples of how it's used.
We have noticed that this call can cause AssemblyLoad events to the assemblies referenced by the emitted assembly, so we have provided an assembly loader to hand over the bytes of the referenced assemblies (since many of the assemblies exist only in memory). See http://source.roslyn.io/#Roslyn.Test.Utilities/HostedRuntimeEnvironment.cs for details.
Problem
Since the compiler will often attempt to compile against different versions of the same assembly e.g., mscorlib, we need to isolate every call to PeVerify and prevent any cached assemblies from being erroneously loaded. We currently do so by wrapping all calls to PeVerify which reference different versions of already loaded assemblies with a new AppDomain, see http://source.roslyn.io/#Roslyn.Test.Utilities/HostedRuntimeEnvironment.cs,60.
CoreCLR, however, has discarded the AppDomain story. Running each test in its own process is very expensive in run time, and manually sorting the tests into "identical assembly sets" would be very expensive in developer time.
Solution
Up to coreCLR. :) It seems as though the optimal solution is simply a PeVerify which doesn't load assemblies. An alternative would be to provide a way to sandbox assembly loading (even with mscorlib) or allow assembly unloading.
Activity
[-]Feature request: Provide PeVerify without assembly loading or assembly unload[/-][+]Feature request: Provide PeVerify without assembly loading or a way to assembly unload[/+]jkotas commentedon Feb 19, 2015
The right way to address this is to build a standalone IL verify that does not even load the runtime.
eatdrinksleepcode commentedon Feb 20, 2015
I was not aware that CoreCLR was discarding AppDomains, nor can I find any information about that decision. Can you provide a link for that?
jkotas commentedon Feb 20, 2015
AppDomains were primarily designed for sandboxing - running untrusted code downloaded from internet. AppDomains bundled many different levels of isolation together: assembly load context, security context, ... that made them very expensive to create and shutdown.
None of the current .NET Core scenarios require sandboxing. It is the reason why AppDomains are not included in the .NET Core framework surface.
However, there are still valid scenarios that need some of the isolation levels that AppDomains provided. The typical use for AppDomains in .NET Framework required just one type isolation, but one had to pay for all of them. It was not possible to pick what you are going to pay for. Our strategy is to provide specific solutions or recommendations for the individual types of isolation without bundling them all together:
I expect that the design for this feature request is going to follow the above pattern of providing targeted solutions. Note that AppDomains were not really addressing the IL verification well because of mscorlib was fixed by runtime - it was not possible to supply custom mscorlib as noted above.
MattWhilden commentedon Apr 4, 2015
@agocke It seems like a few things have been suggested. Any comments? Can we make headway or close this issue out?
jkotas commentedon Apr 4, 2015
It was not fixed yet - it should stay open.
dsyme commentedon Jan 18, 2016
@jkotas Some questions on this
On the topic of communication across a computation boundary, it's worth noting that serializing exceptions no longer works in CoreCLR, see dotnet/coreclr#2715. (Previously it would be fairly normal to serialize an exception across an AppDomain boundary, or even across processes, but serializing from a slave process seems now to have become harder)
FWIW I'm fine with AppDomains disappearing, but I'd just like to see more guidance on communicating with isolated processes.
Thanks
Don
dsyme commentedon Jan 18, 2016
@jkotas BTW here are 130,000 mentions of "AppDomain" in existing C# code on github, might be worth scanning the scenarios there. https://github.com/search?l=csharp&q=AppDomain&type=Code&utf8=%E2%9C%93
jkotas commentedon Dec 31, 2016
I have pushed my old prototype of standalone fully managed IL verifier to https://github.com/dotnet/corert/tree/ILVerify
cc @OmarTawfik
agocke commentedon Jan 4, 2017
@jkotas Nice! Since we run PEVerify thousands of times in the Roslyn tests I think it would be a good test of this to run both versions of peverify during our tests and confirm that they produce the same results.
I'll try and mock up that change when I get a chance.
jkotas commentedon Jan 4, 2017
@agocke Sync with @OmarTawfik. He is planning to look into this as well.
The prototype is incomplete. It needs work to be a viable for Roslyn tests.
gregkalapos commentedon Apr 27, 2017
@jkotas @agocke
My team (outside Microsoft) has the same problem.
The prototype implementation is very promising! Thanks for that!
I noticed that around exceptions there is not much implemented, so the tool gives up very quickly on a ‘real-world’ assembly. I started to implement some missing features. What is the current state of this tool? Does Microsoft have an internal version, or is the prototype here on GitHub the latest version? What does the Roslyn team use currently?
I would be happy to do pull requests with our stuff, but it is currently on a very old branch. What’s the plan regarding branches? Shouldn’t this be moved to master?
Thanks!
jkotas commentedon Apr 27, 2017
The GitHub branch has the latest version.
PEVerify tool from .NET Framework. They would love to have something better that works cross-plat, is open source, etc. We just did not find time to make it happen.
PRs are welcomed! I have merged master to the ILVerify branch to make it current.
The tool is not in master because there was no traction on it. If there is interest in the standalone IL Verify and somebody (that can be you) is actively working on it, we will look into moving it to master.
gregkalapos commentedon May 3, 2017
@jkotas Great, thanks! With @mmayr-at we already sent pull requests.
What would be the best way to discuss further questions?
I already have two points:
What about testing? Imo the easiest way would be to simply prepare dlls (both valid and invalid) and let run ilverify with them and assert to the expected errors (or success if there is no error).
One problem we currently have is that in Implement TypedReference in CoreRT corert#3000 "ByReference'1" was added to the s_wellKnownTypeNames in https://github.com/dotnet/corert/blob/master/src/Common/src/TypeSystem/Common/MetadataTypeSystemContext.cs This basically assumes that ByReference`1 is defined everywhere in the base dll. If I understand it correctly this type will be in the base dll (like mscorlib, or PrivateCoreLib) from the next version, but it is not defined in the current and past releases – so IlVerify crashes because it wants to preload this type, but does not find it. Unfortunately, in the MetadataTypeSystemContext class the s_wellKnownTypeNames is private static, so we cannot overwrite it without touching the MetadataTypeSystemContext class (e.g. we could make it protected virtual and define "well known types in all releases" in SimpleTypeSystemContext). Like this. Can we do this? Or does anyone you have an other recommendation?
jkotas commentedon May 3, 2017
Issues in CoreRT repo. I have created https://github.com/dotnet/corert/labels/area-ILVerification, and also added you as collaborator to the CoreRT repo so that the issues in this area can be assigned to you.
Yes, tests would be useful. I think the sources for the tests should be under ILVerify\tests as .il files, and they should get compiled as binaries. (BTW: ilasm has /ERROR option that suppresses some of its error checking that would otherwise prevent invalid test cases from compiling.)
Related tests should live in the same .il file, so that we end up with like a few dozen .il files at the end. I do not think we would want to have thousands little .il files as tests (slow builds, slow sync times, ...).
We tend to like to use XUnit as the test driver. Maybe
[Theory]with MemberData generator will work well for this?I think the best way to deal with is to make this and similar types optional in MetadataTypeSystemContext (not fail or assert if the type is not present).
MichalStrehovsky commentedon May 3, 2017
A good place to take the inspiration from would be \src\ILCompiler.TypeSystem\tests in the CoreRT repo: that's where we keep the type system tests. The type system tests have similar characteristics - each test loads an assembly we built from IL as part of the repo build and does stuff with it.
jkotas commentedon May 10, 2017
Let's track further progress on the ILVerify tool in CoreRT repo:
I have opened following issues in CoreRT repo on topics discussed above:
@mmayr-at @gregkalapos If you would like to work on any of them, just add a note to the issue that you are looking into it. Thank you!
jkotas commentedon Jun 3, 2020
ILVerify tool was moved to dotnet/runtime. #35038 (comment) has an example how to use it.