Skip to content

Add IL verification with ILVerify in addition to PEVerify #37994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RikkiGibson merged 54 commits into from
Feb 11, 2022
Merged

Add IL verification with ILVerify in addition to PEVerify #37994

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
d1e402c
Add IL verification with ILVerify in addition to PEVerify
Nov 2, 2017
70d3ab8
Annotating more tests
Mar 22, 2018
3f8b70f
Use new package with strong name
jcouv Mar 22, 2018
6e2efce
Merge remote-tracking branch 'dotnet/main' into il-verify2
Dec 30, 2021
bf116d9
Fix a couple more tests
Dec 30, 2021
a993d0e
Address some PROTOTYPE markers
Dec 30, 2021
7af8ecc
Tighten checks
Dec 30, 2021
0ee5f6a
Fix IL verification plumbing
Dec 30, 2021
1701dbb
Transition ILVerify to Core test runner
Dec 31, 2021
260a40e
Fix Resolver
Dec 31, 2021
7b0263a
public
Dec 31, 2021
400d315
Start tightening the IL verification
Dec 31, 2021
e7a7c3f
Remove some unused error entries
Dec 31, 2021
9cafb53
Annotate failures
Dec 31, 2021
656d328
InitOnly and NotVisible
Jan 1, 2022
eea70d3
MissingStringType
Jan 1, 2022
d328e27
MissingAssembly
Jan 1, 2022
ebbc74a
BadReturnType
Jan 1, 2022
66aa35b
More
Jan 1, 2022
c34aa28
More
Jan 1, 2022
d23c09c
Change design of flags
Jan 1, 2022
5a01e58
Ctor
Jan 1, 2022
ae83e16
more
Jan 2, 2022
9b1260c
more
Jan 2, 2022
ec54177
tweak
Jan 2, 2022
99e0f0a
VB
Jan 2, 2022
b2fc7c7
Conditional package reference
Jan 3, 2022
ebacf4f
Remove tracking of specific failures
Jan 5, 2022
d57946e
Move check
Jan 5, 2022
a345b1b
VB
Jan 5, 2022
7abba2e
Undo some changes
Jan 5, 2022
f3a3859
Cleanup
Jan 5, 2022
262ee1e
Refine resolution of ambiguities
Jan 6, 2022
733ac37
Align on simple names
Jan 6, 2022
c1f0f8d
Use signed ILVerification package from dotnet7 feed
Jan 29, 2022
f193504
Enable on desktop runtime
Jan 29, 2022
fffe57a
Merge remote-tracking branch 'dotnet/main' into il-verify2
Jan 29, 2022
4cf0b49
Revert "Enable on desktop runtime"
Jan 29, 2022
dceda42
Update added tests
Jan 29, 2022
115da8a
Use compilation to identify corlib
Jan 31, 2022
af67143
Use label for version number
Jan 31, 2022
a7dc694
Address feedback
Feb 2, 2022
ca5027f
Address feedback
Feb 8, 2022
6d83957
usings
Feb 8, 2022
c7d7ba6
Rename
Feb 8, 2022
cb27c93
indentation
Feb 8, 2022
a11ad37
Merge remote-tracking branch 'dotnet/main' into il-verify2
Feb 8, 2022
e32bf0a
Resolve conflicts
Feb 8, 2022
7e0ec94
remove catch
Feb 9, 2022
f5387f8
Revert "remove catch"
Feb 9, 2022
f9cf399
Shrink usage of exceptions
Feb 9, 2022
c55a61d
Adjust to new logic
Feb 9, 2022
fcc2dc2
one more test
Feb 10, 2022
3db3d54
Address feedback
jcouv Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Align on simple names
  • Loading branch information
Julien Couvreur committed Jan 6, 2022
commit 733ac37fe248f9cee1e1d4795af1e007e6dcaac0
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/AssemblyReferencesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ class C
var compilation0 = CreateEmptyCompilation(src0, new[] { MscorlibRef, ref01, ref11 }, assemblyName: "C", options: TestOptions.DebugDll);
var compilation1 = compilation0.WithSource(src1).WithReferences(new[] { MscorlibRef, ref02, ref12 });

var v0 = CompileAndVerify(compilation0);
// ILVerify: Multiple modules named 'Lib' were found
var v0 = CompileAndVerify(compilation0, verify: Verification.FailsIlVerify);

var f0 = compilation0.GetMember<MethodSymbol>("C.F");
var f1 = compilation1.GetMember<MethodSymbol>("C.F");
Expand Down
35 changes: 12 additions & 23 deletions src/Compilers/Test/Core/CompilationVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,41 +233,29 @@ private class Resolver : ILVerify.ResolverBase

internal Resolver(IList<ModuleData> allModuleData)
{

foreach (var module in allModuleData)
{
var image = module.Image;
imagesByName.Add(module.FullName, image);

// Note: although the signature in ResolverBase.ResolveCore calls the parameter "simpleName"
// ILVerify can also pass in full names. So we do allow resolving both.
if (module.SimpleName != module.FullName)
string name = module.SimpleName;
if (imagesByName.ContainsKey(module.SimpleName))
{
if (imagesByName.ContainsKey(module.SimpleName))
{
imagesByName.Remove(module.SimpleName);
imagesByName.Add(module.SimpleName, default);
}
else
{
imagesByName.Add(module.SimpleName, image);
}
throw new Exception($"Multiple modules named '{name}' were found");
Copy link
Member

@jaredpar jaredpar Feb 2, 2022 •
edited by jcouv
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running on .NET Framework it would be legal for multiple assemblies to have the same simple name, presuming they had different strong names. If we must maintain this invariant then tests which do that won't ever be able to run against ILVerify. That isn't a blocker as there are very few of these but want to make sure this was understood. #ByDesign

Copy link
Member Author

@jcouv jcouv Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The issue is that ILVerify.ResolverBase.ResolveCore(string simpleName) gives a simple name.
So tests that deal with duplicate simple names will need to skip ILVerify for the moment.

Copy link
Member

@jkotas jkotas Feb 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: dotnet/runtime#65573 is proposing passing the full assembly name to the resolver.

Copy link
Contributor

@cston cston Feb 9, 2022 •
edited by jcouv
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider constructing the Dictionary<,> in the caller, without throwing an exception. #Resolved

}
imagesByName.Add(name, module.Image);
}
}

protected override PEReader ResolveCore(string name)
protected override PEReader ResolveCore(string simpleName)
{
if (imagesByName.TryGetValue(name, out var image))
if (imagesByName.TryGetValue(simpleName, out var image))
{
if (image.IsDefault)
Copy link
Member

@jaredpar jaredpar Feb 2, 2022 •
edited by jcouv
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this be hit though? The above code throws on this condition, not add a default value. #Resolved

Copy link
Member Author

@jcouv jcouv Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. In an earlier iteration, I was storing "name -> default" into the map for duplicates.

{
throw new Exception($"ILVerify was not able to resolve a module named '{name}' because multiple exist in this compilation");
throw new Exception($"ILVerify was not able to resolve a module named '{simpleName}' because multiple exist in this compilation");
}
return new PEReader(image);
}

throw new Exception($"ILVerify was not able to resolve a module named '{name}'");
throw new Exception($"ILVerify was not able to resolve a module named '{simpleName}'");
}
}

Expand All @@ -285,15 +273,16 @@ private void ILVerify(Verification verification)
var mscorlibModules = _allModuleData.Where(m => m.SimpleName == "mscorlib").ToArray();
if (mscorlibModules.Length == 1)
{
verifier.SetSystemModuleName(new AssemblyName(mscorlibModules[0].FullName));
verifier.SetSystemModuleName(new AssemblyName(mscorlibModules[0].SimpleName));
}
else
{
// ILVerify requires a "system" module to be identified (see ILVerify.Verifier.ThrowMissingSystemModule)
// So we auto-detect a candidate module
// This comes in handy in tests that use TestBase.AacorlibRef for instance.
foreach (var module in _allModuleData)
{
var name = module.FullName;
var name = module.SimpleName;
var metadataReader = resolver.Resolve(name).GetMetadataReader();
if (metadataReader.AssemblyReferences.Count == 0)
{
Expand All @@ -304,7 +293,7 @@ private void ILVerify(Verification verification)
}

// Main module is the first one
var result = verifier.Verify(resolver.Resolve(_allModuleData[0].FullName));
var result = verifier.Verify(resolver.Resolve(_allModuleData[0].SimpleName));
if (result.Count() == 0)
{
Copy link
Contributor

@cston cston Feb 8, 2022 •
edited by jcouv
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the catch block is unnecessary. #Resolved

Copy link
Member

@jaredpar jaredpar Feb 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch 😄

Copy link
Member Author

@jcouv jcouv Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't remove the catch. Tried again just to be sure:
image

Copy link
Contributor

@cston cston Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'd missed the fact that Verify() can throw exceptions. In that case, consider having the try { } catch { } wrap the Verify() call only, and catch specific exceptions rather than all.

if ((verification & Verification.FailsIlVerify) == 0)
Expand Down
27 changes: 18 additions & 9 deletions src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Retargeting/RetargetingTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,8 @@ End Namespace

'Retargetted - should result in No additional Errors also and same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[Success
]]>)
Main_Retarget.VerifyDiagnostics()
Expand Down Expand Up @@ -1307,7 +1308,8 @@ End Namespace

'//Retargetted - should result in No Errors also and same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[Success
]]>)
Main_Retarget.VerifyDiagnostics()
Expand Down Expand Up @@ -1693,7 +1695,8 @@ Imports System

'//Retargetted - should result in No Errors also and same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[11
12
13
Expand Down Expand Up @@ -1894,7 +1897,8 @@ MethodOverload(Base)

'//Retargetted - should result in No Errors also same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[Sharedmethod
method(Other)
MethodOverload(Other)
Expand Down Expand Up @@ -2172,7 +2176,8 @@ Success]]>)

'//Retargetted - should result in No Errors also and same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[Success
Success
Success
Expand Down Expand Up @@ -2365,7 +2370,8 @@ test

'//Retargetted - should result in No Errors also
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[1
test
]]>)
Expand Down Expand Up @@ -2681,12 +2687,14 @@ End Class
</compilation>
''//All on same FX - should result in No Errors
Dim referenceLibrary_Compilation = DirectCast(CompileAndVerify(sourceLibV1, options:=TestOptions.ReleaseDll).Compilation, VisualBasicCompilation)
Dim main_NoRetarget = CompileAndVerify(sourceMain, references:={referenceLibrary_Compilation.ToMetadataReference})
' ILVerify: Multiple modules named 'mscorlib' were found
Dim main_NoRetarget = CompileAndVerify(sourceMain, references:={referenceLibrary_Compilation.ToMetadataReference}, verify:=Verification.FailsIlVerify)
main_NoRetarget.VerifyDiagnostics()

''//Retargetted - should result in No Errors also
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(sourceMain, references:={RetargetReference}, options:=TestOptions.ReleaseExe)
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(sourceMain, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify)
main_NoRetarget.VerifyDiagnostics()
End Sub

Expand Down Expand Up @@ -3081,7 +3089,8 @@ End Namespace

'//Retargetted - should result in No Errors also and same runtime behavior
Dim RetargetReference = RetargetCompilationToV2MsCorlib(referenceLibrary_Compilation)
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe,
' ILVerify: Multiple modules named 'mscorlib' were found
Dim Main_Retarget = CompileAndVerify(source, references:={RetargetReference}, options:=TestOptions.ReleaseExe, verify:=Verification.FailsIlVerify,
expectedOutput:=<![CDATA[Success
]]>)
Main_Retarget.VerifyDiagnostics()
Expand Down