Skip to content
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.

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

Invalid mock assembly produced #19

Open
tamasvajk opened this issue Apr 28, 2023 · 8 comments
Open

Invalid mock assembly produced #19

tamasvajk opened this issue Apr 28, 2023 · 8 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@tamasvajk
Copy link

I'm generating mock assemblies and then inspecting them with ILSpy. I noticed that in some cases the generated assembly contains methods that can't be processed with ILSpy. I'm unsure if the problem lies in ILSpy or Refasmer. If I generate reference assemblies instead of the mocks, then I'm not facing the issue. Also, not all methods are impacted by the problem in a mock assembly.

The below commands

refasmer /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Ref/7.0.2/ref/net7.0/System.Runtime.dll -v -c -O=out -p -n -m
ilspycmd -o X out/System.Runtime.dll

produce an exception:

 Processing 1 assemblies
 Processing /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Ref/7.0.2/ref/net7.0/System.Runtime.dll
  [System.Runtime.dll] Using custom entity filter
 All done
Error decompiling @06000001 Microsoft.CodeAnalysis.EmbeddedAttribute..ctor
in assembly "out/System.Runtime.dll"
 ---> System.ArgumentNullException: Value cannot be null. (Parameter 'methodReference')
   at ICSharpCode.Decompiler.TypeSystem.MetadataModule.ResolveMethod(EntityHandle methodReference, GenericContext context) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\TypeSystem\MetadataModule.cs:line 421
   at ICSharpCode.Decompiler.IL.ILReader.DecodeCall(OpCode opCode) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\IL\ILReader.cs:line 1517
   at ICSharpCode.Decompiler.IL.ILReader.DecodeInstruction() in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\IL\ILReader.cs:line 926
   at ICSharpCode.Decompiler.IL.ILReader.ReadInstructions(CancellationToken cancellationToken) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\IL\ILReader.cs:line 443
   at ICSharpCode.Decompiler.IL.ILReader.ReadIL(MethodDefinitionHandle method, MethodBodyBlock body, GenericContext genericContext, ILFunctionKind kind, CancellationToken cancellationToken) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\IL\ILReader.cs:line 580
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileBody(IMethod method, EntityDeclaration entityDecl, DecompileRun decompileRun, ITypeResolveContext decompilationContext) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 1542
-- continuing with outer exception (ICSharpCode.Decompiler.DecompilerException) --
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileBody(IMethod method, EntityDeclaration entityDecl, DecompileRun decompileRun, ITypeResolveContext decompilationContext) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 1604
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DoDecompile(IMethod method, DecompileRun decompileRun, ITypeResolveContext decompilationContext) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 1478
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DoDecompile(ITypeDefinition typeDef, DecompileRun decompileRun, ITypeResolveContext decompilationContext) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 1318
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DoDecompileTypes(IEnumerable`1 types, DecompileRun decompileRun, ITypeResolveContext decompilationContext, SyntaxTree syntaxTree) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 574
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileWholeModuleAsSingleFile(Boolean sortTypes) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 610
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileWholeModuleAsSingleFile() in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 584
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileWholeModuleAsString() in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler\CSharp\CSharpDecompiler.cs:line 866
   at ICSharpCode.Decompiler.Console.ILSpyCmdProgram.Decompile(String assemblyFileName, TextWriter output, String typeName) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler.Console\IlspyCmdProgram.cs:line 234
   at ICSharpCode.Decompiler.Console.ILSpyCmdProgram.OnExecute(CommandLineApplication app) in D:\GitWorkspace\ILSpy_72\ICSharpCode.Decompiler.Console\IlspyCmdProgram.cs:line 153

From the below two commands, only the latter one is failing, so not all types are impacted:

ilspycmd out/System.Runtime.dll -t System.Security.Cryptography.CryptographicException
ilspycmd out/System.Runtime.dll -t Microsoft.CodeAnalysis.EmbeddedAttribute

Removing the -m works without any problem:

refasmer /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Ref/7.0.2/ref/net7.0/System.Runtime.dll -v -c -O=out -p -n
ilspycmd -o X out/System.Runtime.dll

I'm running the above on MacOS. The input DLL is the dotnet 7 reference assembly System.Runtime.dll.

@ForNeVeR ForNeVeR self-assigned this Sep 6, 2023
@ForNeVeR
Copy link
Contributor

The problem is that we use System.NotImplementedException in mocked method bodies, and this is a rare case when System.NotImplementedException itself is defined in the assembly we are creating a mocked version of. And even more: it is defined in the assembly later than the attribute in question; that's why the import logic fails.

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Oct 21, 2023

Not sure what to do about that, honestly. I've tried to get that exception imported first, before everything else, and it failed in JetBrains.Refasmer.MetadataImporter::Import with

if (dstHandle != _typeDefinitionCache[srcHandle])
    throw new Exception("WTF: type handle mismatch");

@ForNeVeR ForNeVeR added the help wanted Extra attention is needed label Oct 21, 2023
@ForNeVeR ForNeVeR removed their assignment Oct 21, 2023
@ForNeVeR ForNeVeR added the bug Something isn't working label Oct 21, 2023
@zabbius
Copy link
Contributor

zabbius commented Oct 23, 2023

As I remember, all methods in mockup mode are replaced with throw new NotImplementedException(). And we cannot import NotImplementedException itself, because it has constructor which is method and therefore should be replaced with throw new NotImplementedException(). I also don't know how to fix this without changing Refasmer logic.

I think that acceptable solution will be to check if NotImplementedException is defined in assembly and return error when trying to mockup such assembly. Or (a little harder to implement) maybe specify exception type (should be from assembly references) to throw.

@ForNeVeR
Copy link
Contributor

Well, technically such an implementation should be valid:

class NotImplementedException {
  public NotImplementedException() {
    throw new NotImplementedException();
  }
}

It is possible to write and compile in C#, so it has to be possible to express in metadata. But I'm not sure what exact sequence of actions would help us to achieve this.

@tamasvajk
Copy link
Author

What is the reason for using throw new NotImplementedException(); instead of throw null;?

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Oct 30, 2023

That's an excellent question actually.

I think that throw null is the canonical throw-away1 way to implement a "not implemented" method body in reference assembly, is it not?

Footnotes

  1. See what I did there? Yay!

@zabbius
Copy link
Contributor

zabbius commented Oct 30, 2023

Never used and never seen throw null is real life but maybe is can be a solution

@ForNeVeR
Copy link
Contributor

throw null is a well-known way to throw a NullReferenceException without actually referencing a NullReferenceException 😅

Though now I can't remember in what context I saw it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants