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

Disassembler fails with generic instance #640

Closed
xoofx opened this Issue Feb 1, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@xoofx
Member

xoofx commented Feb 1, 2018

Hello! (Followup of #612)
So I wanted to try the disassembler with the 0.10.12 version but getting a crash on the following example:

    [DisassemblyDiagnoser]
    public class BenchFail<T> where T : new()
    {
        [Benchmark] public void Bench() => new T();
    }
   // ...
   // In the main
   BenchmarkDotNet.Running.BenchmarkRunner.Run<BenchFail<int>>();

And the error:

// AfterAll
Failed to disassemble with following exception:
Object reference not set to an instance of an object.
   at BenchmarkDotNet.Disassembler.Program.Disassemble(Settings settings, ClrRuntime runtime, State state)
   at BenchmarkDotNet.Disassembler.Program.Handle(Settings settings)
   at BenchmarkDotNet.Disassembler.Program.Main(String[] args)

Unhandled Exception: System.InvalidOperationException: There is an error in XML document (0, 0). ---> System.Xml.XmlException: Root element is missing.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.ParseDocumentContent()
   at System.Xml.XmlReader.MoveToContent()
   at Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializationReaderDisassemblyResult.Read9_DisassemblyResult()
   --- End of inner exception stack trace ---
   at System.Xml.Serialization.XmlSerializer.Deserialize(XmlReader xmlReader, String encodingStyle, XmlDeserializationEvents events)
   at BenchmarkDotNet.Diagnosers.WindowsDisassembler.Dissasemble(DiagnoserActionParameters parameters)
   at BenchmarkDotNet.Diagnosers.DisassemblyDiagnoser.Handle(HostSignal signal, DiagnoserActionParameters parameters)
   at BenchmarkDotNet.Extensions.CommonExtensions.ForEach[T](IList`1 source, Action`1 command)
   at BenchmarkDotNet.Loggers.SynchronousProcessOutputLoggerWithDiagnoser.ProcessInput()
   at BenchmarkDotNet.Toolchains.Executor.Execute(Process process, Benchmark benchmark, SynchronousProcessOutputLoggerWithDiagnoser loggerWithDiagnoser, ILogger logger)
   at BenchmarkDotNet.Toolchains.Executor.Execute(Benchmark benchmark, ILogger logger, String exePath, String workingDirectory, String args, IDiagnoser diagnoser, IResolver resolver, IConfig config)
   at BenchmarkDotNet.Toolchains.Executor.Execute(ExecuteParameters executeParameters)
   at BenchmarkDotNet.Running.BenchmarkRunnerCore.Execute(ILogger logger, Benchmark benchmark, IToolchain toolchain, BuildResult buildResult, IConfig config, IResolver resolver)
   at BenchmarkDotNet.Running.BenchmarkRunnerCore.RunCore(Benchmark benchmark, ILogger logger, ReadOnlyConfig config, String rootArtifactsFolderPath, Func`2 toolchainProvider, IResolver resolver, BuildResult buildResult)
   at BenchmarkDotNet.Running.BenchmarkRunnerCore.Run(BenchmarkRunInfo benchmarkRunInfo, ILogger logger, String title, String rootArtifactsFolderPath, Func`2 toolchainProvider, IResolver resolver, List`1 artifactsToCleanup)
   at BenchmarkDotNet.Running.BenchmarkRunnerCore.Run(BenchmarkRunInfo benchmarkRunInfo, Func`2 toolchainProvider)

Likely something not supported/correctly escaped when using a generic instance? (I was actually glad to see that I could run some benchmark with this - without the print disassembly)

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Feb 1, 2018

Hello @xoofx, thanks for the bug report!
@adamsitnik could you take a look?

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Feb 3, 2018

@xoofx I have made a little research and some debugging and it looks like it's currently impossible to get the asm of generic type method. It's a ClrMD limitation, which we can not overcome.

Example:

namespace ConsoleApp7
{
    class Program
    {
        static void Main(string[] args)
        {
            var _int = new Generic<int>().Create();
            var _short = new Generic<short>().Create();
            var _object = new Generic<object>().Create();

            if (_int + _short * _object.GetHashCode() == 123)
                throw new Exception();
        }
    }

    public class Generic<T> where T : new()
    {
        public T Create() => new T();
    }
}

When I attach to such simple program ClrMD reports that there is only one type called Generic, it has method Create but this method is not compiled (CompilationType == None) and there is no address of the implementation (NativeCode == ulong.MaxValue)

image

Of course, this is how the generic works, but I would expect that it's going to report generics of value types as separate types Generic<int> Generic<short> and that the Generic type methods are going to be compiled for reference types.

This problem is described in ClrMD docs

Of course, this is how the generic works, but I would expect that it's going to report generics of value types as separate types Generic<int> Generic<short> and that the Generic type methods are going to be compiled for reference types.

I am going to provide a nice error message and try to get the workaround suggested by @nietras in #631 working (separate type per

@adamsitnik adamsitnik closed this in 5dd1a53 Feb 4, 2018

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Feb 4, 2018

@xoofx I found the way to get what we need.

@AndreyAkinshin I added an extra, non-inlinable method to our auto-generated type. It's executed only once for jitting, when the diagnoser starts disassembling it starts with this method. In case of generic types the method contains address of the compiled generic method, and once we have this address the ClrMD is able to recognize the type.

After this build is finished the new package on our CI will contain the fix.

@adamsitnik adamsitnik self-assigned this Feb 4, 2018

@adamsitnik adamsitnik added this to the v0.10.13 milestone Feb 4, 2018

@nietras

This comment has been minimized.

Contributor

nietras commented Feb 4, 2018

@xoofx

This comment has been minimized.

Member

xoofx commented Feb 4, 2018

@xoofx I found the way to get what we need.

thanks a lot!

@AndreyAkinshin

This comment has been minimized.

Member

AndreyAkinshin commented Feb 4, 2018

@adamsitnik,

it's currently impossible to get the asm of generic type method

I found the way to get what we need.

It's always good to see when people are not afraid of impossible problems. =)

adamsitnik added a commit that referenced this issue Feb 4, 2018

@adamsitnik

This comment has been minimized.

Member

adamsitnik commented Feb 5, 2018

@xoofx @nietras the build I referenced last time did not pass, but this one did, so 0.10.12.423 is the version for you

It's always good to see when people are not afraid of impossible problems. =)

I hate to give up, and this kind of problems make me very motivated to solve them.

GeorgePlotnikov added a commit to GeorgePlotnikov/BenchmarkDotNet that referenced this issue Feb 25, 2018

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment