search

jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.77k stars 630 forks source link

System.IndexOutOfRangeException resolving local scopes in Cecil 0.11.4 #816

Closed SteveGilham closed 2 years ago

SteveGilham commented 2 years ago

Copying the relevant part of AltCover issue 135 

Unhandled Exception: System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at Mono.Cecil.Cil.InstructionCollection.ResolveInstructionOffset(InstructionOffset inputOffset, InstructionOffsetCache& cache)
   at Mono.Cecil.Cil.InstructionCollection.UpdateLocalScope(ScopeDebugInformation scope, Instruction removedInstruction, Instruction existingInstruction, InstructionOffsetCache& cache)
   at Mono.Cecil.Cil.InstructionCollection.UpdateLocalScope(ScopeDebugInformation scope, Instruction removedInstruction, Instruction existingInstruction, InstructionOffsetCache& cache)
   at Mono.Cecil.Cil.InstructionCollection.UpdateLocalScopes(Instruction removedInstruction, Instruction existingInstruction)
   at Mono.Collections.Generic.Collection`1.Insert(Int32 index, T item)

from a call to ILProcessor.InsertBefore on code built for .net Framework 4.8 on Windows.

Given the context, I don't have the relevant IL or a repro case, alas, but it's still worth having this on the record.

The stack trace is similar to those issues in #697 and #781 that appeared following the revision of that part of the codebase at the 0.11.3 release.

jbevain commented 2 years ago

@vitek-karas that seems related to previous changes. Any clue?

vitek-karas commented 2 years ago

Not really - That said I can see couple of places in the ResolveInstructionOffset which could lead to the exception as there are not enough checks. For example if there was a method with no IL instruction but the items array is non-empty (with nulls), that could cause an exception like this.

SteveGilham commented 2 years ago

While my previous issue reports in this area were caused by an assembly generated by an old version of the Mono C# compiler, the code base behind this one is modern C#, with a current Roslyn compiler : while I have not seen the code or the assembly in question, symbol names indicated that async local functions that are also closures -- giving 2 generated classes for the price of one small piece of source -- abound.

My guess would be that the offending method lies somewhere in the copious amounts of generated code, but I have not gone looking. It sufficed, for purposes of code instrumentation, to perform a one-pass scope resolution of my own for the method ahead of doing code injection.

jbevain commented 2 years ago

@SteveGilham could you try with Cecil's master to validate the fix in #824?

SteveGilham commented 2 years ago

I'm afraid I don't have a repro case; all the information I have is a stack trace from an issue against my AltCover project, which was enough to let me write a work-around.

jbevain commented 2 years ago

Closing this then, we should be good with the latest fixes.