search

microsoft / coyote

Coyote is a library and tool for testing concurrent C# code and deterministically reproducing bugs.
https://microsoft.github.io/coyote/
Other
1.48k stars 77 forks source link

Rewrite throws NullReferenceException #336

Closed egil closed 2 years ago

egil commented 2 years ago

Hi there,

Super interesting project. I would love to try to use it to track down some hard to reproduce concurrency/asynchronous issues I have with my library bUnit. However, when I try to rewrite some of my own and 3rd party assemblies, I get a NullReferenceException.

This is my repository with my current experimental Coyote code: https://github.com/bUnit-dev/bUnit/tree/experiments/coyote-testing/tests/bunit.coyotetest (NOTE: It does require .net core 3.1, .net 5, .net 6 and .net 7 preview to build.)

When I call coyote rewrite <path to bunit.coyotetest>\coyote.config.json, I get the following output:

❯ coyote rewrite .\coyote.config.json
Microsoft (R) Coyote version 1.5.4.0 for .NET 6.0.4
Copyright (C) Microsoft Corporation. All rights reserved.

. Rewriting the assemblies specified in C:\Source\bunit.dev\bUnit\tests\bunit.coyotetest\bin\debug\net6.0.
... Copying all files to the 'C:\Source\bunit.dev\bUnit\tests\bunit.coyotetest\bin\net6.0\rewritten' directory
... Rewriting the 'AngleSharpWrappers.dll' assembly (AngleSharpWrappers, Version=2.0.0.0, Culture=neutral, PublicKeyToken=fe2a2d410725906b)
..... Writing the modified 'AngleSharpWrappers.dll' assembly to C:\Source\bunit.dev\bUnit\tests\bunit.coyotetest\bin\net6.0\rewritten\AngleSharpWrappers.dll
... Rewriting the 'Bunit.Core.dll' assembly (Bunit.Core, Version=1.8.4.7195, Culture=neutral, PublicKeyToken=fe9c6306bcc1ce6f)
..... Writing the modified 'Bunit.Core.dll' assembly to C:\Source\bunit.dev\bUnit\tests\bunit.coyotetest\bin\net6.0\rewritten\Bunit.Core.dll
... Rewriting the 'Bunit.Web.dll' assembly (Bunit.Web, Version=1.8.4.7195, Culture=neutral, PublicKeyToken=fe9c6306bcc1ce6f)
Error: Object reference not set to an instance of an object.

I tried cloning coyote debugging the rewrite code, and see this stack trace:

   at Mono.Cecil.ImportGenericContext.TypeParameter(String type, Int32 position)
   at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context)
   at Mono.Cecil.DefaultMetadataImporter.ImportReference(TypeReference type, IGenericParameterProvider context)
   at Mono.Cecil.ModuleDefinition.ImportReference(TypeReference type, IGenericParameterProvider context)
   at Mono.Cecil.ModuleDefinition.ImportReference(TypeReference type)
   at Microsoft.Coyote.Rewriting.TypeRewritingPass.RewriteAndImportType(TypeReference type, Options options, Boolean onlyImport, Boolean& isRewritten) in C:\Source\GitHub\coyote\Source\Test\Rewriting\Passes\Rewriting\Types\TypeRewritingPass.cs:line 203

Removing the two assemblies "Bunit.Web.dll" and "Microsoft.AspNetCore.Components.dll" allows the rewrite to complete. The first assembly is mine, the second is a 3rd party one.

pdeligia commented 2 years ago

Thanks for your interest in Coyote @egil! This seems like a rewriting bug, might be some corner case or type that is not handled, thanks for reporting this.

If you run coyote rewrite <path to bunit.coyotetest>\coyote.config.json -d (-d enables debug logging for rewriting) it will print very detailed output of what exactly is being rewritten, so this will most likely will show the exact IL instruction causing this. Would you be able to share this log here (just the last bits with the relevant instruction, as the log will likely be too large to share). Give me some time and I will also check out your test/code and try repro this to see what might be causing this.

In the meantime, until we address and fix this, could you try to run Coyote without rewriting these 2 libraries? Not sure if you have already tried running some tests without "Bunit.Web.dll" and "Microsoft.AspNetCore.Components.dll" rewritten?

To give you some context: although ideally you want to run Coyote with it controlling as much of your code as possible (for better coverage), the tool also supports what we call "partially-controlled concurrency" which allows it to run on a partially rewritten set of assemblies. Because it does not control everything, it can lose coverage (depends how on which assemblies are skipped and their relevance to the concurrency in your test, so it's hard to tell without knowing your codebase) but often it works well. This mode is enabled by default so you do not need to do anything special besides running your test. If this fails, you can also try run the tool in "concurrency fuzzing" mode. This is a new feature and not documented yet, but see this recent answer for more context and how to enable. This is a more relaxed mode that can also work nicely if not everything is rewritten (but again with reduced coverage comparing to full rewriting + control). (Just suggesting some workarounds in case you are blocked and want to try things out until we address this issue.)

egil commented 2 years ago

Thanks for the quick response. Much appreciated.

Ps. Where do you prefer to get questions about the use of lib? In the discussions forum or issues list?

egil commented 2 years ago

Here is the debug log from rewriting the two all files that fail for me:

Hope this helps you track down the bug.

pdeligia commented 2 years ago

Thanks @egil, this is very helpful! Give me a few days to investigate and I will circle back.

pdeligia commented 2 years ago

Hi @egil, apologies for some delay here, I was juggling between this and other work this week.

The good news is that I found and fixed the bug, it was a missing corner case inside the Coyote binary rewriter for instructions containing a call to the modreq in IL. I will be pushing a new NuGet release ASAP and let you know! Thanks again for reporting this issue :)

Btw, at the same time I found another potential issue in your rewriting configuration coyote.config.json. I see that you are including a lot of DLLs that are either from the .NET library or external to your codebase:

"Assemblies": [
  "AngleSharp.Css.dll",
  "AngleSharp.Diffing.dll",
  "AngleSharp.dll",
  "AngleSharpWrappers.dll",
  "AutoFixture.dll",
  "AutoFixture.Xunit2.dll",
  "Bunit.Core.dll",
  "bunit.coyotetest.dll",
  "Bunit.TestAssets.dll",
  "Bunit.Web.dll",
  "Castle.Core.dll",
  "CoyoteCoverageReportMerger.dll",
  "DiffEngine.dll",
  "EmptyFiles.dll",
  "Fare.dll",
  "Microsoft.ApplicationInsights.dll",
  "Microsoft.AspNetCore.Authorization.dll",
  "Microsoft.AspNetCore.Components.Authorization.dll",
  "Microsoft.AspNetCore.Components.Forms.dll",
  "Microsoft.AspNetCore.Components.Web.dll",
  "Microsoft.AspNetCore.Components.WebAssembly.Authentication.dll",
  "Microsoft.AspNetCore.Http.Abstractions.dll",
  "Microsoft.AspNetCore.Http.Features.dll",
  "Microsoft.AspNetCore.Metadata.dll",
  "Microsoft.Extensions.DependencyInjection.Abstractions.dll",
  "Microsoft.Extensions.DependencyInjection.dll",
  "Microsoft.Extensions.DependencyModel.dll",
  "Microsoft.Extensions.Localization.Abstractions.dll",
  "Microsoft.Extensions.Logging.Abstractions.dll",      
  "Microsoft.Extensions.Options.dll",       
  "Microsoft.JSInterop.dll",
  "Microsoft.Win32.Registry.AccessControl.dll",
  "Microsoft.Win32.SystemEvents.dll",
  "NuGet.Frameworks.dll",       
  "Serilog.Extensions.Logging.dll",
  "Shouldly.dll",
  "System.CodeDom.dll",
  "System.ComponentModel.Composition.dll",
  "System.ComponentModel.Composition.Registration.dll",
  "System.Configuration.ConfigurationManager.dll",
  "System.Data.Odbc.dll",
  "System.Data.OleDb.dll",
  "System.Data.SqlClient.dll",
  "System.Diagnostics.EventLog.dll",
  "System.Diagnostics.PerformanceCounter.dll",
  "System.DirectoryServices.AccountManagement.dll",
  "System.DirectoryServices.dll",
  "System.DirectoryServices.Protocols.dll",
  "System.Drawing.Common.dll",
  "System.IO.Packaging.dll",
  "System.IO.Ports.dll",
  "System.Management.dll",
  "System.Private.ServiceModel.dll",
  "System.Reflection.Context.dll",
  "System.Runtime.Caching.dll",     
  "System.Security.Cryptography.ProtectedData.dll",     
  "System.Security.Permissions.dll",
  "System.ServiceModel.dll",
  "System.ServiceModel.Duplex.dll",
  "System.ServiceModel.Http.dll",
  "System.ServiceModel.NetTcp.dll",
  "System.ServiceModel.Primitives.dll",
  "System.ServiceModel.Security.dll",
  "System.ServiceModel.Syndication.dll",
  "System.ServiceProcess.ServiceController.dll",
  "System.Threading.AccessControl.dll",
  "System.Windows.Extensions.dll",
  "testhost.dll",
  "xunit.abstractions.dll",
  "xunit.assert.dll",
  "Xunit.Combinatorial.dll",
  "xunit.core.dll",
  "xunit.execution.dotnet.dll",
  "xunit.runner.reporters.netcoreapp10.dll",
  "xunit.runner.utility.netcoreapp10.dll",
  "xunit.runner.visualstudio.dotnetcore.testadapter.dll"
]

Ideally, you only want to rewrite the DLLs of code that you own yourself. This is a bit subtle point, but to try to give you some intuition behind this, when you rewrite a DLL, Coyote will inject hooks to take over the concurrency, which allows it to explore interleavings during coyote test to find bugs. However, if you rewrite external DLLs, Coyote will waste a lot of time exploring interleavings that are not related to your code (you assume these external DLLs are correct after all!), causing the "state space" to explode and hurts coverage. Some-times you cannot avoid this (especially if most of the concurrency comes from these external DLLs) but ideally you want to try to only rewrite your own code, unless you hit coverage issues.

I think that perhaps the following might work for your case (and you might even be able to reduce it even more, but your "Bunit" dlls should be there for sure; but if you face any issues we can discuss about this):

"Assemblies": [
  "AngleSharp.Css.dll",
  "AngleSharp.Diffing.dll",
  "AngleSharp.dll",
  "AngleSharpWrappers.dll",
  "AutoFixture.dll",
  "AutoFixture.Xunit2.dll",
  "Bunit.Core.dll",
  "bunit.coyotetest.dll",
  "Bunit.TestAssets.dll",
  "Bunit.Web.dll",
  "DiffEngine.dll",
  "EmptyFiles.dll",
  "Fare.dll",       
  "Serilog.Extensions.Logging.dll",
  "Shouldly.dll",
]
egil commented 2 years ago

Thank you. I need to include a couple of Blazor libs as my library is extending these, e.g. a base class from Blazor, so I want that rewritten too.

pdeligia commented 2 years ago

What you say makes sense!

pdeligia commented 2 years ago

Version 1.5.5 with the fix is now out :) https://www.nuget.org/packages/Microsoft.Coyote/1.5.5

linkdotnet commented 1 year ago

I am not sure if it makes sense to answer to that title, but the exact same setup fails on net7.0 for me with Coyote 1.7.9.

coyote rewrite Microsoft.AspNetCore.Components.dll -v debug

This leads to said NullReferenceException.

Here the log from the command: log.txt.

@pdeligia let me know if opening a separate issue is the way forward.

pdeligia commented 1 year ago

Interesting! Thanks for letting me know @linkdotnet, must be some other corner case causing this, let me try investigate next week and will let you know. Your command & log is very helpful! Which version of Microsoft.AspNetCore.Components.dll is this btw? Could you point me to the nuget package (and version) that contains this DLL so I can try it out?

linkdotnet commented 1 year ago

Hey @pdeligia thanks for the swift response.

The package in question is this here: https://www.nuget.org/packages/Microsoft.AspNetCore.Components/7.0.0 It happens also with the latest patch version 7.0.5 (https://www.nuget.org/packages/Microsoft.AspNetCore.Components/7.0.5)