Severe Bug in .NET Reflector 6.5.0.135 : Wrong Order

Explore, browse and analyze .NET assemblies

Moderators: Luke Jefferson, Charles Brown, StephenC, Alex.Davies, Greg.Tillman, melvyn.harbour

Severe Bug in .NET Reflector 6.5.0.135 : Wrong Order

Postby Elmue » Thu Dec 09, 2010 7:23 pm

Hello

There is a severe bug in .NET reflector which modifies the order of the commands.

The following Managed C++ code calls into a unmanaged function.

Code: Select all
bool ManagedClass::ConvertValue(Object* o_In, [Out]Object** o_Out, Char c_Type)
{
   VARIANT k_In, k_Out;
   Marshal::GetNativeVariantForObject(o_In, &k_In);

   BOOL b_Ret = cNative::ConvertValue(&k_In, &k_Out, c_Type);

   *o_Out = Marshal::GetObjectForNativeVariant(&k_Out);

   VariantClear(&k_In);
   VariantClear(&k_Out);
   
   return (b_Ret == TRUE);
}

_________________________________________

What .NET REflector shows is this:

Code: Select all
public static unsafe bool ConvertValue(object o_In, out object o_Out, [MarshalAs(UnmanagedType.U2)] char c_Type)
{
    tagVARIANT gvariant;
    tagVARIANT gvariant2;

    IntPtr pDstNativeVariant = new IntPtr();
    pDstNativeVariant = new IntPtr((void*) &gvariant2);
    Marshal.GetNativeVariantForObject(o_In, pDstNativeVariant);

    IntPtr pSrcNativeVariant = new IntPtr();
    pSrcNativeVariant = new IntPtr((void*) &gvariant);
    o_Out = Marshal.GetObjectForNativeVariant(pSrcNativeVariant);

    VariantClear(&gvariant2);
    VariantClear(&gvariant);

    return (cNative.ConvertValue((void*) &gvariant2, (void*) &gvariant, c_Type) == 1);
}

This is complete nonsense because in this code the native function is called after VariantClear() has erased the values of the variants!

How is it possible that Reflector changes the order at its own discretion ?

The code has been compiled on VS 2003

Elmü
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm

Postby Brian Donahue » Fri Dec 10, 2010 1:57 pm

Hello,

.NET Reflector can create managed C++ code from IL. So the compiler must be creating the IL to do the operations in this order. If you can ILDASM this function and send the resulting IL, that would probably explain what is happening.

For instance, we don't even see BOOL b_Ret in the decompiled C++, but it is calculated at the end as what the function is going to return. I don't think Reflector is going to go in there and just arbitrarily rearrange code, so it's probably the C++ managed compiler that is producing the IL in a different way, and when that IL is read back and converted back to C++, this is the result.
Brian Donahue
 
Posts: 6669
Joined: Mon Aug 23, 2004 10:48 am

Postby Clive Tong » Fri Dec 10, 2010 5:36 pm

If you can send the assembly across, I'd be happy to take a look at it. I tried to write a small reproduction, but got correct code from Reflector.If you can't send the assembly, then (as Brian says) the IL for the method would allow us to debug this further.

Thanks.
Clive Tong
 
Posts: 283
Joined: Thu Dec 04, 2008 5:42 pm

ILDASM retuns correct order

Postby Elmue » Tue Dec 14, 2010 6:22 pm

Hello

ILDASM returns the correct order.
You see in the code below that VariantClear is called AFTER cNative.ConvertValue.

This result does not surprise me because if the code would be as .NET Reflector shows it, it would not work.
It would convert an erased variant.

So the compiler is not the problem.
There is definitely a severe bug !

Elmü

Code: Select all
.method public static bool
        marshal( unsigned int8)
        ConvertValue(object o_In,
                     [out] object& o_Out,
                     char  marshal( unsigned int16) c_Type) cil managed
{
  .maxstack  3
  .locals init ([0] native int V_0,
           [1] int32 b_Ret,
           [2] native int V_2,
           [3] valuetype tagVARIANT k_Out,
           [4] valuetype tagVARIANT k_In)
  IL_0000:  ldloca.s   V_2
  IL_0002:  initobj    [mscorlib]System.IntPtr
  IL_0008:  ldloca.s   V_2
  IL_000a:  ldloca.s   k_In
  IL_000c:  call       instance void [mscorlib]System.IntPtr::.ctor(void*)
  IL_0011:  ldarg.0
  IL_0012:  ldloc.2
  IL_0013:  call       void [mscorlib]System.Runtime.InteropServices.
                            Marshal::GetNativeVariantForObject(object, native int)
  IL_0018:  ldloca.s   k_In
  IL_001a:  ldloca.s   k_Out
  IL_001c:  ldarg.2
  IL_001d:  call       int32 modopt([mscorlib]System.Runtime.CompilerServices.CallConvCdecl)
                                    cNative.ConvertValue(void*, void*, uint16)
  IL_0022:  stloc.1
  IL_0023:  ldloca.s   V_0
  IL_0025:  initobj    [mscorlib]System.IntPtr
  IL_002b:  ldloca.s   V_0
  IL_002d:  ldloca.s   k_Out
  IL_002f:  call       instance void [mscorlib]System.IntPtr::.ctor(void*)
  IL_0034:  ldarg.1
  IL_0035:  ldloc.0
  IL_0036:  call       object [mscorlib]System.Runtime.InteropServices.Marshal::GetObjectForNativeVariant(native int)
  IL_003b:  stind.ref
  IL_003c:  ldloca.s   k_In
  IL_003e:  call       int32 modopt([Microsoft.VisualC]Microsoft.VisualC.IsLongModifier)
                             modopt([mscorlib]System.Runtime.CompilerServices.CallConvStdcall)
                             VariantClear(valuetype tagVARIANT*)
  IL_0043:  pop
  IL_0044:  ldloca.s   k_Out
  IL_0046:  call       int32 modopt([Microsoft.VisualC]Microsoft.VisualC.IsLongModifier)
                             modopt([mscorlib]System.Runtime.CompilerServices.CallConvStdcall)
                             VariantClear(valuetype tagVARIANT*)
  IL_004b:  pop
  IL_004c:  ldloc.1
  IL_004d:  ldc.i4.1
  IL_004e:  ceq
  IL_0050:  conv.u1
  IL_0051:  br.s       IL_0053
  IL_0053:  ret
}
Last edited by Elmue on Fri Dec 17, 2010 3:16 am, edited 1 time in total.
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm

Postby Elmue » Fri Dec 17, 2010 3:12 am

Hello Clive

Could you reproduce the bug ?

Elmü
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm

Postby Clive Tong » Fri Dec 17, 2010 9:20 am

Hi.

I haven't managed to reproduce it yet using C++ and VS2008. Sorry for the delay but we've been working hard on getting the EAP of Reflector 7 ready so I haven't had a lot of time.

Thanks for sending the IL. I'll try to convert that into a reproduction of the problem in the next day or so. Alternatively, if you have a small assembly that demonstrates the problem, please could you email it to me (clive DOT tong AT red HYPHEN gate DOT com)

Thanks.
Clive Tong
 
Posts: 283
Joined: Thu Dec 04, 2008 5:42 pm

Postby Elmue » Sun Dec 19, 2010 3:03 pm

Hello

I created a new demo project for you.

As I nearly always get problems with email servers rejecting emails that contain an attachment with a EXE or DLL, I prefer not to send you an email.

I uploaded the demo project (Source code + compiled assembly) here:

http://www.electronix.ch/ptbsync/.NetRe ... ugDemo.zip

! You should fix this bug before releasing the new version 7 !

Elmü
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm

Postby Clive Tong » Mon Dec 20, 2010 3:37 pm

Thank you very much for the self-contained example. I logged the issue as RP-831.

I've prototyped a potential fix for the problem. The problem is that Reflector isn't noticing a data dependency between the ConvertValue and the subsequent changes to the gvariant variables. With that in place, the code decompiles to:

Code: Select all
    tagVARIANT gvariant;
    tagVARIANT gvariant2;
    IntPtr pDstNativeVariant = new IntPtr();
    pDstNativeVariant = new IntPtr((void*) &gvariant2);
    Marshal.GetNativeVariantForObject(o_In, pDstNativeVariant);
    int local1 = cNative.ConvertValue((void*) &gvariant2, (void*) &gvariant, c_Type);
    IntPtr pSrcNativeVariant = new IntPtr();
    pSrcNativeVariant = new IntPtr((void*) &gvariant);
    o_Out = Marshal.GetObjectForNativeVariant(pSrcNativeVariant);
    VariantClear(&gvariant2);
    VariantClear(&gvariant);
    return (local1 == 1);


I'll let you know when a real fix is in place. Thanks again for your help.
Clive Tong
 
Posts: 283
Joined: Thu Dec 04, 2008 5:42 pm

Postby Elmue » Tue Dec 21, 2010 4:14 pm

Hello

This code looks correct.
Although a tiny optimization would be possible:

Code: Select all
IntPtr pSrcNativeVariant = new IntPtr();
         pSrcNativeVariant = new IntPtr((void*) &gvariant);


could be replaced with

Code: Select all
IntPtr pSrcNativeVariant = new IntPtr((void*) &gvariant);



When will the new version come ?

Elmü
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm

Postby Elmue » Wed Mar 16, 2011 8:00 pm

Hello

It is really INCREDIBLE !

Right now .NET Reflector did an automatic update to version 6.6.0.30 and this old bug is NOT fixed !

3 months have passed by and nothing happened!

This explains why the quality of Relector is quite bad: Lots of bugs and even crashes are usual.
The explanation is: Nobody fixes them.

So what is the sense of this forum ?
People can post bugs here but they are ignored.

In the old times of Lutz Roeder the quality was much better.

Elmü
Elmue
 
Posts: 7
Joined: Thu Dec 09, 2010 7:08 pm


Return to .Net Reflector 6.x and .NET Reflector 6.x Pro

Who is online

Users browsing this forum: No registered users and 0 guests