icsharpcode / ILSpy

.NET Decompiler with support for PDB generation, ReadyToRun, Metadata (&more) - cross-platform!
21.49k stars 3.35k forks source link

Automatic event not matched (MCS?) #1145

Closed tamlin-mike closed 6 years ago

tamlin-mike commented 6 years ago

The following IL can be assembled using ilasm.

The problem seems to be that in Pattern.DoMatchCollection the cur2 variable doesn't have a trailing type Role after its reference to CompareExchange, but the pattern automaticEventPatternV4MCS expects it.

Additionally, when trying to decompile this, ILSpy hits an assert about the Threading namespace being inserted but is not part of referenced, which I guess in a sense is correct since it probably expected the event patternmatcher to have handled this and therefore not pull in Threading manually.

.class public auto ansi sealed EvType {}

.class public auto ansi serializable beforefieldinit OwningClass 
{

.event EvType EvName
{
    .addon instance void OwningClass::add_EvName(class EvType)
    .removeon instance void OwningClass::remove_EvName(class EvType)
}

.method public hidebysig specialname 
    instance void add_EvName (
        class EvType 'value'
    ) cil managed 
{
    .maxstack 3
    .locals init (
        [0] class EvType,
        [1] class EvType
    )

    IL_0000: ldarg.0
    IL_0001: ldfld class EvType OwningClass::EvName
    IL_0006: stloc.0
    // loop start (head: IL_0007)
        IL_0007: ldloc.0
        IL_0008: stloc.1
        IL_0009: ldarg.0
        IL_000a: ldflda class EvType OwningClass::EvName
        IL_000f: ldloc.1
        IL_0010: ldarg.1
        IL_0011: call class [mscorlib]System.Delegate [mscorlib]System.Delegate::Combine(class [mscorlib]System.Delegate, class [mscorlib]System.Delegate)
        IL_0016: castclass EvType
        IL_001b: ldloc.0
        IL_001c: call !!0 [mscorlib]System.Threading.Interlocked::CompareExchange<class EvType>(!!0&, !!0, !!0)
        IL_0021: stloc.0
        IL_0022: ldloc.0
        IL_0023: ldloc.1
        IL_0024: bne.un IL_0007
    // end loop
    IL_0029: ret
} // end of method OwningClass::add_EvName

.method public hidebysig specialname 
    instance void remove_EvName (
        class EvType 'value'
    ) cil managed 
{
    .maxstack 3
    .locals init (
        [0] class EvType,
        [1] class EvType
    )

    IL_0000: ldarg.0
    IL_0001: ldfld class EvType OwningClass::EvName
    IL_0006: stloc.0
    // loop start (head: IL_0007)
        IL_0007: ldloc.0
        IL_0008: stloc.1
        IL_0009: ldarg.0
        IL_000a: ldflda class EvType OwningClass::EvName
        IL_000f: ldloc.1
        IL_0010: ldarg.1
        IL_0011: call class [mscorlib]System.Delegate [mscorlib]System.Delegate::Remove(class [mscorlib]System.Delegate, class [mscorlib]System.Delegate)
        IL_0016: castclass EvType
        IL_001b: ldloc.0
        IL_001c: call !!0 [mscorlib]System.Threading.Interlocked::CompareExchange<class EvType>(!!0&, !!0, !!0)
        IL_0021: stloc.0
        IL_0022: ldloc.0
        IL_0023: ldloc.1
        IL_0024: bne.un IL_0007
    // end loop
    IL_0029: ret
} // end of method OwningClass::remove_EvName

} // end of class OwningClass
siegfriedpammer commented 6 years ago

Is this from some code found in the wild or is it a constructed example? Two points that make me wonder:

1) The IL does not contain any field declaration. I find it baffling that ilasm does not issue any warning at all. 2) The event type is EvType, which is declared as plain class. Because it is not deriving from System.Delegate in any way, the decompiler correctly adds casts to System.Delegate, when passing evType2 and value to Delegate.Combine/Delegate.Remove...

This is not a bug. If this is in the real code, the IL is horribly broken and cannot be translated to a C# auto event. It would not compile because of CS0066: event must be of a delegate type.

Could you please send me the original assembly to verify this?

siegfriedpammer commented 6 years ago

I have adjusted your test case and added a fix to the mcs pattern for the missing type arguments in the output of ExpressionBuilder. Can you please confirm, whether this fixes the issue?

tamlin-mike commented 6 years ago

It's from code found in the wild, though I obviously messed it up little when trying to anonymize it. The actual delegate (still anonymized) is

.class public auto ansi sealed EvType extends [mscorlib]System.MulticastDelegate
{
    // Methods not included. Just the run of the mill ctor + {Begin|End}?Invoke 
}

OwningClass indeed also have .field private class EvType EvName

Sorry about that - though in retrospect, I am also a bit surprised ilasm didn't complain.

I have verified that it now works with HEAD, but I'll still send you the assembly for completeness.