ArgumentNullException when running coverage

[twcclegg]

Things have worked great for me in the past, however in joining a new project; when running dotnet test /p:AltCover=true I get:

C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error :  [C:\Users\tclegg\<path>\<project>.csproj]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error : ERROR *** Instrumentation phase failed [C:\Users\tclegg\<path>\<project>.cspro
j]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error :  [C:\Users\tclegg\<path>\<project>.csproj]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error :  [C:\Users\tclegg\<path>\<project>.csproj]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error : Value cannot be null. [C:\Users\tclegg\<path>\<project>.csproj]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error :  [C:\Users\tclegg\<path>\<project>.csproj]
C:\Users\tclegg\.nuget\packages\altcover\8.3.838\build\netstandard2.0\AltCover.targets(83,5): error : Details written to C:\Users\tclegg\<path>\bin\Debug\netcoreapp3.1\__Instrumented_<project>\AltCover-2022-09-09--11-57-25.log [C:\Users\tclegg\<path>\<project>.csproj]

Log file contains the following:

System.ArgumentNullException: Value cannot be null.
   at Mono.Cecil.SignatureWriter.WritePrimitiveValue(Object value)
   at Mono.Cecil.MetadataBuilder.GetConstantSignature(ElementType type, Object value)
   at Mono.Cecil.MetadataBuilder.GetConstantSignature(ConstantDebugInformation constant)
   at Mono.Cecil.MetadataBuilder.AddLocalConstants(ScopeDebugInformation scope)
   at Mono.Cecil.MetadataBuilder.AddLocalScope(MethodDebugInformation method_info, ScopeDebugInformation scope)
   at Mono.Cecil.MetadataBuilder.AddLocalScope(MethodDebugInformation method_info, ScopeDebugInformation scope)
   at Mono.Cecil.MetadataBuilder.AddMethodDebugInformation(MethodDebugInformation method_info)
   at Mono.Cecil.Cil.CodeWriter.WriteResolvedMethodBody(MethodDefinition method)
   at Mono.Cecil.Cil.CodeWriter.WriteMethodBody(MethodDefinition method)
   at Mono.Cecil.MetadataBuilder.AddMethod(MethodDefinition method)
   at Mono.Cecil.MetadataBuilder.AddMethods(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddNestedTypes(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddTypes()
   at Mono.Cecil.MetadataBuilder.BuildTypes()
   at Mono.Cecil.MetadataBuilder.BuildModule()
   at Mono.Cecil.MetadataBuilder.BuildMetadata()
   at Mono.Cecil.ModuleWriter.<>c.<BuildMetadata>b__2_0(MetadataBuilder builder, MetadataReader _)
   at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TItem item, Func`3 read)
   at Mono.Cecil.ModuleWriter.BuildMetadata(ModuleDefinition module, MetadataBuilder metadata)
   at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
   at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
   at Mono.Cecil.ModuleDefinition.Write(Stream stream, WriterParameters parameters)
   at Mono.Cecil.AssemblyDefinition.Write(Stream stream, WriterParameters parameters)
   at AltCover.Instrument.I.writeAssembly(AssemblyDefinition assembly, String path) in /_//AltCover.Engine/Instrument.fs:line 462
   at AltCover.Instrument.I.writeAssemblies(AssemblyDefinition definition, String file, IEnumerable`1 targets, FSharpFunc`2 sink) in /_//AltCover.Engine/Instrument.fs:line 822
   at AltCover.Instrument.I.visitAfterAssembly(InstrumentContext state, AssemblyEntry assembly) in /_//AltCover.Engine/Instrument.fs:line 1084
   at AltCover.Instrument.I.instrumentationVisitorWrapper(FSharpFunc`2 core, InstrumentContext state, Node node) in /_//AltCover.Engine/Instrument.fs:line 1426
   at [email protected](T node) in /_//AltCover.Engine/Visitor.fs:line 1520
   at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList`1 cons, FSharpFunc`2 f, FSharpList`1 x) in D:\a\_work\1\s\src\fsharp\FSharp.Core\local.fs:line 237
   at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc`2 mapping, FSharpList`1 x) in D:\a\_work\1\s\src\fsharp\FSharp.Core\local.fs:line 248
   at Microsoft.FSharp.Collections.SeqModule.Fold[T,TState](FSharpFunc`2 folder, TState state, IEnumerable`1 source) in D:\a\_work\1\s\src\fsharp\FSharp.Core\seq.fs:line 731
   at AltCover.Visitor.visit(IEnumerable`1 visitors, IEnumerable`1 assemblies) in /_//AltCover.Engine/Visitor.fs:line 1506
   at [email protected](Unit unitVar0) in /_//AltCover.Engine/Main.fs:line 744
   at AltCover.PathOperation.DoPathOperation[TAny](FSharpFunc`2 operation, FSharpFunc`2 handle) in /_//AltCover.Engine/Output.fs:line 14
ParamName = 
<null>
TargetSite = 
Void WritePrimitiveValue(System.Object)
Data = 
seq []
InnerException = 
<null>
HelpLink = 
<null>
Source = 
"Mono.Cecil"
HResult = 
-2147467261

Any insight would be much appreciated, thanks.

[SteveGilham]

This is going to be an interesting one to track down, as it's from deep inside the Mono.Cecil library that I call to add instrumentation to the code for which coverage is to be gathered; and, as we can see, it gives no context as to what piece of code was being processed when it failed, and thus what construct in the source is being mishandled.

Would it be possible for you to cut this down to a minimum repro case, sufficiently sanitized that I might be allowed visibility? The code would not have to do anything apart from compile, so it would be a case of just cutting chunks of code away stopping and backtracking when the issue goes away.

Meanwhile, I'll look to reverse engineering the part of the assembly writing that fails to create a version of AltCover that will provide some tracing of where the issue might lie, and what the code construct is that causes the fault, that you could then run.

[SteveGilham]

Here's an experimental build that attempts to identify and fix the defective constant definitions
altcover.8.3.702-github-pre.nupkg.zip

It will log as a warning any such constant giving its name and the method in which it is declared; that should also help with the task of identifying a minimal repro case that can then be used as a regression test.

[twcclegg]

Thank you for the test build, it seems like it did indeed point to an issue:
C:\Users\tclegg\.nuget\packages\altcover\8.3.702-github-pre\build\netstandard2.0\AltCover.targets(83,5): warning : Null Primitive Constant <thing> in method System.Void <Namespace>.<AbstractClass>/<Function>d__43::MoveNext(), where thing is const IList<string> thing = null;
The tests also now successfully run and output coverage rather than terminating fairly immediately.

I tried last week to make a minimal repo but it was hard going in blind and trying to keep it in a compilable state. I'll try again now that I have the identifying info.

[SteveGilham]

That's very interesting, as the type IList<string> is not a primitive (integer, bool, or such), but one that does have a legitimate null value. The MoveNext() method is of course a compiler generated one within <Namespace>.<AbstractClass>.<Function> which is most likely async but may be one that returns an iterator by yielding.

I'm surprised that this hasn't shown up before -- it may simply be that const is vary rarely used in practice outside magic numbers and strings. It looks like Cecil is not correctly identifying some categories of type; so a repro will be used to raise an issue there.

[twcclegg]

Yeah once I knew where to look having a null const seemed quite odd. Inlining it, the problem goes away (using the above build and 8.3.838). As such I do longer have any particular need for a fix, but I'll try to make a minimal repro.
Thanks again for the experimental build, it was invaluable in finding the issue.

[twcclegg]

I'm able to repro with https://github.com/twcclegg/NullConst

[SteveGilham]

That's even simpler than I'd expected. Thank you for that.

[SteveGilham]

Cecil issue 873 raised; and PR submitted. For the moment, though, I shall retain the experimental changes, awaiting a new release of that library.

[SteveGilham]

AltCover 8.3.839 release contains this fix but with the messages directed to the Verbose rather than Warn output channel.

[twcclegg]

Thanks1