Issue with pdb files after updating

[GeertvanHorrik]

3>------ Rebuild All started: Project: Orc.Controls.NET46, Configuration: Debug Any CPU ------
2>    Fody: Fody (version 2.0.5.0) Executing
2>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Core.1.1.2\lib\Microsoft.WindowsAPICodePack.dll'
2>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Shell.1.1.1\lib\Microsoft.WindowsAPICodePack.Shell.dll'
2>      Fody/Obsolete:   	Removing reference to 'Obsolete.dll'.
2>      Fody/Catel:   Weaving properties
2>      Fody/Catel:   Weaving exposed properties
2>      Fody/Catel:   Weaving arguments
2>      Fody/Catel:   Weaving logging
2>      Fody/Catel:   Weaving xml schemas is disabled
2>MSBUILD : error : Fody: An unhandled exception occurred:
2>MSBUILD : error : Exception:
2>MSBUILD : error : Value does not fall within the expected range.
2>MSBUILD : error : StackTrace:
2>MSBUILD : error :    at Mono.Cecil.Pdb.ISymUnmanagedWriter2.OpenScope(Int32 startOffset, Int32& pRetVal)
2>MSBUILD : error :    at Mono.Cecil.Pdb.NativePdbWriter.DefineScope(ScopeDebugInformation scope, MethodDebugInformation info, MetadataToken& import_parent)
2>MSBUILD : error :    at Mono.Cecil.Pdb.NativePdbWriter.DefineScope(ScopeDebugInformation scope, MethodDebugInformation info, MetadataToken& import_parent)
2>MSBUILD : error :    at Mono.Cecil.Pdb.NativePdbWriter.Write(MethodDebugInformation info)
2>MSBUILD : error :    at Mono.Cecil.Cil.CodeWriter.WriteResolvedMethodBody(MethodDefinition method)
2>MSBUILD : error :    at Mono.Cecil.Cil.CodeWriter.WriteMethodBody(MethodDefinition method)
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.AddMethod(MethodDefinition method)
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.AddMethods(TypeDefinition type)
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.AddTypes()
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.BuildTypes()
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.BuildModule()
2>MSBUILD : error :    at Mono.Cecil.MetadataBuilder.BuildMetadata()
2>MSBUILD : error :    at Mono.Cecil.ModuleWriter.<>c.<BuildMetadata>b__1_0(MetadataBuilder builder, MetadataReader _)
2>MSBUILD : error :    at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TItem item, Func`3 read)
2>MSBUILD : error :    at Mono.Cecil.ModuleWriter.BuildMetadata(ModuleDefinition module, MetadataBuilder metadata)
2>MSBUILD : error :    at Mono.Cecil.ModuleWriter.WriteModuleTo(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
2>MSBUILD : error :    at Mono.Cecil.ModuleDefinition.Write(String fileName, WriterParameters parameters)
2>MSBUILD : error :    at InnerWeaver.WriteModule() in C:\projects\fody\FodyIsolated\ModuleWriter.cs:line 18
2>MSBUILD : error :    at InnerWeaver.Execute() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 88
2>MSBUILD : error : Source:
2>MSBUILD : error : Mono.Cecil.Pdb
2>MSBUILD : error : TargetSite:
2>MSBUILD : error : Void OpenScope(Int32, Int32 ByRef)
2>MSBUILD : error : 
2>    Fody:   Finished Fody 1440ms.

[GeertvanHorrik]

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS2012	Cannot open 'C:\Source\Orc.Controls\src\Orc.Controls\Orc.Controls.NET46\obj\Debug\Orc.Controls.pdb' for writing -- 'The process cannot access the file 'C:\Source\Orc.Controls\src\Orc.Controls\Orc.Controls.NET46\obj\Debug\Orc.Controls.pdb' because it is being used by another process.'	Orc.Controls.NET46	C:\Source\Orc.Controls\src\Orc.Controls\Orc.Controls.NET46\CSC	1	Active

[GeertvanHorrik]

Other libs are just fine, so checking out what's different about this one.

[SimonCropp]

what weavers?

[GeertvanHorrik]

<?xml version="1.0" encoding="utf-8"?>
<Weavers VerifyAssembly="true">
  <ModuleInit/>
  <Costura>
    <IncludeAssemblies>
      Microsoft.WindowsAPICodePack
      Microsoft.WindowsAPICodePack.Shell
    </IncludeAssemblies>
  </Costura>
  <Obsolete/>
  <Catel/>
</Weavers>

But I believe the other projects used these as well.

[SimonCropp]

can u incrementally remove the weavers and see if we can isolate?

[GeertvanHorrik]

Once this happens, an msbuild instance keeps a lock on the pdb file and needs to be killed in order to release it.

[GeertvanHorrik]

can u incrementally remove the weavers and see if we can isolate?

on it

[GeertvanHorrik]

It's Catel causing this one. Weird, it works fine on other ones. But it looks like I have some fixing to do ;-)

[SimonCropp]

let me have a look at catel

[GeertvanHorrik]

No, I found it. Whenever I disasble the WeaveProperties feature, the issue goes away. Probably a wrong import.

[GeertvanHorrik]

Found the causing VM, checking what's broken on this one.

[SimonCropp]

ok. let me know if u need help

[GeertvanHorrik]

Found it, it's caused when a combination of this is used:

1. Auto properties with default values:

public bool ShowErrors { get; set; } = true;

2. Multiple constructors

[GeertvanHorrik]

Definitely something in the Catel weaver, Fody is all good!

[SimonCropp]

but how does that lock files?

[GeertvanHorrik]

Probably because mono.cecil fails to generate the pdb or assembly.

[GeertvanHorrik]

Re-opening for the moment. At least something is going wrong in Cecil:

[SimonCropp]

ping @jbevain thoughts?

[GeertvanHorrik]

Building cecil locally so I have access to all PDB files.

[GeertvanHorrik]

[jbevain]

@GeertvanHorrik do you have a small C# file that reproduces the Cecil exception?

[GeertvanHorrik]

Trying to repro. It seems to happen only to a single property (same stuff on the same class works fine). Trying to break it down to a single repro for you (or fix it if it's something on my side).

[GeertvanHorrik]

It goes wrong in WriteResolvedMethodBody. Checking out why it fails so hard. In the past, it would just result in invalid IL, but now it doesn't even output an assembly / pdb (both 0 bytes).

[GeertvanHorrik]

@jbevain how can I easily export List so you can take a look at it?

[jbevain]

@SimonCropp one thing I can think of: the weaver modifies the IL to the extent that the info in the debug scope goes bonkers.

@GeertvanHorrik I don't understand the question :)

[GeertvanHorrik]

I have triple checked all generated IL, it looks good and works fine in other tests. The only thing that could be broken are the operands, but again, I would expect it to generate invalid IL, not crash.

Unfortunately I cannot easily disable pdb's because Fody doesn't seem to like that. Will create a custom build without it for now.

[GeertvanHorrik]

@jbevain I mean: is there a simple "to string" for the list so you can see that it's valid IL. Even if the IL scope goes totally broken, can't it be rebuild from scratch or is that my ignorance speaking here?

[GeertvanHorrik]

Without symbols:

1>    Fody: Fody (version 2.0.5.0) Executing
1>MSBUILD : warning : Fody: No debug symbols found. It is recommended to build with debug symbols enabled.
1>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Core.1.1.2\lib\Microsoft.WindowsAPICodePack.dll'
1>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Shell.1.1.1\lib\Microsoft.WindowsAPICodePack.Shell.dll'
1>      Fody/Obsolete:   	Removing reference to 'Obsolete.dll'.
1>      Fody/Catel:   Weaving properties
1>      Fody/Catel:   Weaving exposed properties
1>      Fody/Catel:   Weaving arguments
1>      Fody/Catel:   Weaving logging
1>      Fody/Catel:   Weaving xml schemas is disabled
1>    Fody:   Finished Fody 3194ms.
1>  Orc.Controls.NET40 -> C:\Source\Orc.Controls\output\debug\NET40\Orc.Controls\Orc.Controls.dll
1>    Fody:   Verifying assembly
1>    Fody:   Finished verification in 287ms.

I verified the IL & C# code automatically (PEVerify) and manually by hand, that looks good. This gives me the idea that this is a symbols issue in Mono.Cecil. What info do you need? A repro is very hard since I tried to reproduce this in the unit tests for Catel.Fody and there it worked just fine.

I've tried to analyze several things, such as:

1. Maybe it's the first value type property definition with default value after a reference type
2. Maybe it's the name of the property

The weird thing is that it only happens to a single property. Whenever I disable this property conversion, it all works fine.

[jbevain]

@GeertvanHorrik we have a few primitives in our test suite you could reuse to dump a method as a string representation, but nothing built-in.

If something is modifying the collection of instructions, it's also responsible for making sure the model in the DebugInformation of the method makes sense. I don't know if this issue is related to that, it might be that the scope gets completely out of sync the modified IL and the pdb writer doesn't know what to make of it. Or it could be a simple bug in the pdb writer.

I'm afraid I need a way to reproduce the issue to fix it :)

[SimonCropp]

@GeertvanHorrik perhaps remove fody from the picture and replicate your il manipulation code in a console app?

[GeertvanHorrik]

I wasn't aware that I was responsible for updating the debug info as well. Does that mean that whenever I add / remove a field / property / method, I need to update the debug info?

[jbevain]

Only methods have debug information.

You can add all you want (new stuff just won't have debug information). If you modify an existing method body, you need to make sure its .DebugInformation still makes sense (including scopes and sequence points).

[GeertvanHorrik]

Thanks for the info. I will take a look at how I can fix this. Maybe I can write an extension method for this that might benefit others as well.

[GeertvanHorrik]

Looks like we are having a bit of success:

2>  Orc.Controls.NET45 -> C:\Source\Orc.Controls\output\debug\NET45\Orc.Controls\Orc.Controls.dll
2>    Fody:   Verifying assembly
2>    Fody:   Finished verification in 327ms.
1>    Fody: Fody (version 2.0.5.0) Executing
1>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Core.1.1.2\lib\Microsoft.WindowsAPICodePack.dll'
1>      Fody/Costura:   	Embedding 'C:\Source\Orc.Controls\lib\WindowsAPICodePack-Shell.1.1.1\lib\Microsoft.WindowsAPICodePack.Shell.dll'
1>      Fody/Obsolete:   	Removing reference to 'Obsolete.dll'.
1>      Fody/Catel:   Weaving properties
1>      Fody/Catel:   Weaving exposed properties
1>      Fody/Catel:   Weaving arguments
1>      Fody/Catel:   Weaving logging
1>      Fody/Catel:   Weaving xml schemas is disabled
1>    Fody:   Finished Fody 2229ms.

This is the extension method I use after modifying a method:

private const long AddressToIgnore = 16707566;

        private static readonly FieldInfo SequencePointOffsetFieldInfo = typeof(SequencePoint).GetField("offset", BindingFlags.Instance | BindingFlags.NonPublic);
        private static readonly FieldInfo InstructionOffsetInstructionFieldInfo = typeof(InstructionOffset).GetField("instruction", BindingFlags.Instance | BindingFlags.NonPublic);

        public static void UpdateDebugInfo(this MethodDefinition method)
        {
            var debugInfo = method.DebugInformation;
            var instructions = method.Body.Instructions;
            var scope = debugInfo.Scope;

            if (scope == null || instructions.Count == 0)
            {
                return;
            }

            var oldSequencePoints = debugInfo.SequencePoints;
            var newSequencePoints = new Collection<SequencePoint>();

            // Step 1: check if all variables are present
            foreach (var variable in method.Body.Variables)
            {
                var hasVariable = scope.Variables.Any(x => x.Index == variable.Index);
                if (!hasVariable)
                {
                    var variableDebugInfo = new VariableDebugInformation(variable, $"__var_{variable.Index}");
                    scope.Variables.Add(variableDebugInfo);
                }
            }

            // Step 2: Make sure the instructions point to the correct items
            foreach (var oldSequencePoint in oldSequencePoints)
            {
                //var isValid = false;

                // Special cases we need to ignore
                if (oldSequencePoint.StartLine == AddressToIgnore ||
                    oldSequencePoint.EndLine == AddressToIgnore)
                {
                    continue;
                }

                var instructionOffset = (InstructionOffset)SequencePointOffsetFieldInfo.GetValue(oldSequencePoint);
                var offsetInstruction = (Instruction)InstructionOffsetInstructionFieldInfo.GetValue(instructionOffset);

                // Fix offset
                for (var i = 0; i < instructions.Count; i++)
                {
                    var instruction = instructions[i];
                    if (instruction == offsetInstruction)
                    {
                        var newSequencePoint = new SequencePoint(instruction, oldSequencePoint.Document)
                        {
                            StartLine = oldSequencePoint.StartLine,
                            StartColumn = oldSequencePoint.StartColumn,
                            EndLine = oldSequencePoint.EndLine,
                            EndColumn = oldSequencePoint.EndColumn
                        };

                        newSequencePoints.Add(newSequencePoint);

                        //isValid = true;

                        break;
                    }
                }
            }

            debugInfo.SequencePoints.Clear();

            foreach (var newSequencePoint in newSequencePoints)
            {
                debugInfo.SequencePoints.Add(newSequencePoint);
            }

            // Step 3: update the scopes by setting the indices
            scope.Start = new InstructionOffset(instructions.First());
            scope.End = new InstructionOffset(instructions.Last());
        }

I will investigate whether I can strip it down.

[GeertvanHorrik]

It looks like this is sufficient:

            var debugInfo = method.DebugInformation;
            var instructions = method.Body.Instructions;
            var scope = debugInfo.Scope;

            if (scope == null || instructions.Count == 0)
            {
                return;
            }

            scope.Start = new InstructionOffset(instructions.First());
            scope.End = new InstructionOffset(instructions.Last());

[GeertvanHorrik]

Using the extended method is recommended since it does check (and update) all the still relevant sequence points and will remove the ones no longer valid.

Thanks for all the help @SimonCropp & @jbevain , let's close this one.

[jbevain]

Glad to hear this is fixed!