microsoft / ConcordExtensibilitySamples

Visual Studio Debug Engine Extensibility Samples
Other
122 stars 50 forks source link

IDkmClrExpressionCompiler bug/limitation of the generated MSIL #32

Closed onor13 closed 7 years ago

onor13 commented 7 years ago

When generating the MSIL file to get the locals variables, based on the doc from here https://github.com/Microsoft/ConcordExtensibilitySamples/wiki/CLR-Expression-Evaluators, I found a bug/limitation. The documentation says and I quote:

“locals of the query method match the arguments and locals of the method we are evaluating in”

But what if the MSIL needed to compute a variable is very complex and requires additional local variables? A natural solution to bypass this limitation is to use a call an intermediate function that does all necessary work. So the code would look something like that:

.class public QueryClass 
{
  .method public hidebysig static int32 M1(int32 arg1, int32 arg2) cil managed
   {
      .locals init ([0]int32 local1, [1]string local2)
      ldarg.0
      call int32 QueryClass::MyMethod(int32 arg)
      ret
   }
   .method public hidebysig static int32 MyMethod(int32 arg)
   {
       ldarg.0
       ret
   }
}

The code above works fine (If I didn’t make a typo when writing it). But when I try to do something more complex in my MyMethod sometimes it fails and I can’t understand why. Now let’s look at 2 cases where it fails for the reasons I don’t understand: 1) To somehow debug I created a logger that uses a TextWriter class to write to some file. The call to the logger looks something like

     ldstr "my Log message"
     call void [LoggerLib] TmpLogger::Log(class [mscorlib]System.String)

When I place the call to the logger in the "M1" method everything works fine, but if I put the same call in the "MyMethod" method, I get an exception:

“method calls into native method Microsoft.Win32.Win32Native.WriteFile(). Evaluation of native methods in this context is not supported.”

Why the same call works in “M1” and not in “MyMethod”??? It looks like for some reasons I am more limited in the library usage in “MyMethod”. 2) The second example and the main reason why I created this topic is that I want “MyMethod” to return System.Array. Let’s take the example from above and replace int32 by System.Array:

.class public QueryClass
{
    .method public hidebysig static static class [mscorlib]System.Array M1(int32 arg1, int32 arg2) cil  managed
    {
       .locals init ([0]int32 local1, [1]string local2)
        call class [mscorlib]System.Array QueryClass::MyMethod()
        ret
    }

   .method public hidebysig static class [mscorlib]System.Array MyMethod()
   {
    ldtoken [mscorlib]System.String
        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 10
    call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)
    ret
   }
}

As you can see this code is very similar to the first example but in this case we create a System.Array of size 10. For some unknown reasons it does not work: the value, the name and the type for the specified local is blank in the debugger local window. If we inline the “MyMethod” call, everything works fine. Again, it looks like making call to an intermediate functions creates a bug. Here is the inline version that works correctly:

.class public QueryClass
{
   .method public hidebysig static static class [mscorlib]System.Array M1(int32 arg1, int32 arg2) cil managed
   {
      .locals init ([0]int32 local1, [1]string local2)
       ldtoken [mscorlib]System.String
        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 10
    call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)
       ret
   }
}
plnelson commented 7 years ago

Yes, calculating the result of an expression in many cases will require temporary variables. The C# Expression Compiler will in fact use temporaries sometimes. The documentation was incorrect so I corrected it.

In a nutshell, you can define and use temporary variables as long as they are after all of the locals defined in the original method.

In terms of the other behaviors you were seeing:

Essentially what is happening is the debugger uses an emulator to interpret the code generated for the expression. This emulator is very limited in terms of the code it can execute. A few of the limitations are: can't execute P-Invokes (with a few exceptions), can't find the target method of a real delegate, can't call methods/properties on real System.Type and types in System.Reflection. By "real", I mean a value from the debuggee process rather than a value created in the emulator.

The reason most evaluations work is because the debugger will try to execute calls made in the root frame in the debuggee process itself rather than try to emulate the call. We call execution of the method in-process "func eval". With few exceptions, we only func eval from the root frame of the evaluation. The reasons for this get really complicated so I'll skip that subject for now.

With this knowledge, another solution you can use (besides temporaries) is to have your compiler/runtime put these helper methods in the process itself rather than the query assembly. If the method lives in the process, the debugger can func eval it.

It works from the root frame because the debugger will func eval it as described above. If it's not in the root frame, the emulator has to execute it, and the emulator doesn't support this method. One experiment you can do is force the emulator to be used and get an idea of what the problem is. You can force the emulator by using the "emulator" format specifier. An example from the Immediate window in VS:

Array.CreateInstance(typeof(string), 10),emulator
Evaluation of method System.Array.CreateInstance requires reading field System.IntPtr.m_value, which is not available in this context.

Without going into too much detail, Array.CreateInstance would need to be special cased in order to emulate it. This was probably never done because Array.CreateInstance is rarely used.

Generally instead of:

    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 10
    call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)

The compiler will generate:

    ldc.i4.s 10
    newarr string

Of course, that will only work for a single dimension array. If you to create and initialize a multi-dimensional array you can use RuntimeHelpers.InitializeArray. Here's an example of creating and initializing a more complicated array:

    ldc.i4.s 10
    ldc.i4.s 10
    newobj instance void int32[0..., 0...]::ctor(int32, int32)
    ldtoken field valuetype [mylib]MyCompilerGeneratedField::MyArrayData1
    call void [mscorlib]System.Runtime.CompilerServices.RuntimeHelpers::InitializeArray(
                class [mscorlib]System.Array,
                valuetype [mscorlib]System.RuntimeFieldHandle)

The emulator will be able to handle the last two code snippets. Hopefully that helps. Let me know if you have any other questions.
-Patrick

onor13 commented 7 years ago

Thank you for the detailed response Patrick, The reason why I am using the Array.CreateInstance is to be able to create multidimensional array with non-zero lower bounds. I didn’t find any other .NET API that allows to create multidimensional arrays with custom lower bounds. I could create my own API on top of newobj instance void int32[0..., 0...]::ctor(int32, int32) to handle custom lower bounds myself, but then I also need to take control over the expansion which brings me to the other question that I reported here https://github.com/Microsoft/ConcordExtensibilitySamples/issues/31 Just to be sure that I understood correctly, the code

.class public QueryClass
{
   .method public hidebysig static static class [mscorlib]System.Array M1(int32 arg1, int32 arg2) cil managed
   {
      .locals init ([0]int32 local1, [1]string local2)
       ldtoken [mscorlib]System.String
        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 10
    call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)
       ret
   }
}

works because the debugger managed to execute the call in the debuggee process itself. And the similar code with the addition of a temporary variable “arr” to store the array

.class public QueryClass
{
   .method public hidebysig static static class [mscorlib]System.Array M1(int32 arg1, int32 arg2) cil managed
   {
      .locals init ([0]int32 local1, [1]string local2, [2]class [mscorlib]System.Array arr)
       ldtoken [mscorlib]System.String
        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 10
    call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)
         stloc.2
         ldloc.2 
       ret
   }
}

Does not work, because the debugger didn’t manage to execute it in the debuggee process and was forced to use the emulation which have some limitations that you described.

Vladimir

plnelson commented 7 years ago

The second code snippet should work. Are you saying that it doesn't work?

BTW, Roslyn doesn't correctly handle arrays with non-zero lower bound. See https://github.com/dotnet/roslyn/issues/3803. I ran into this over a year ago when working on the sample, but looks like it was never fixed. You'll likely run into the same problem, but you should be able to mitigate it if you implement your own IDkmClrResultProvider as I suggested in https://github.com/Microsoft/ConcordExtensibilitySamples/issues/31.

plnelson commented 7 years ago

I opened an internal bug for Array.CreateInstance not working (292294). At this point, I don't know when the fix will make it into Visual Studio.

onor13 commented 7 years ago

Yes, the second snippet doesn't work, it fails on the stloc instruction. I started to look at the implementation of IDkmClrResultProvider in the Roslyn solution, it will solve both the Array with custom lower bounds and the Tree like structure variable expansion but it will take some time to implement it. Thank you for you help Patrick. Vladimir.

plnelson commented 7 years ago

I'm surprised that stloc of a temporary variable fails and it seems concerning. The implementation is very simple. Do you have a repro you can share so I can take a look? BTW, you can IM me on gitter if you don't want to share a repro publically. I'm also on vacation for the next few days so expect slow responses.

onor13 commented 7 years ago

I can't share the whole project due to copyright issues, but it should be easy to reproduce it with the Iris sample with some minor adjustments: 1) Replace the GetClrLocalVariableQuery implementation with the following:

  DkmCompiledClrLocalsQuery IDkmClrExpressionCompiler.GetClrLocalVariableQuery(DkmInspectionContext inspectionContext, DkmClrInstructionAddress instructionAddress, bool argumentsOnly)
        {

            string pathToTest = "C:/WORK/Plugin/ConcordDoc/locals.dll";
            byte[] testBytes = System.IO.File.ReadAllBytes(pathToTest);               
           var locals = new List<DkmClrLocalVariableInfo>();                       
            locals.Add(DkmClrLocalVariableInfo.Create(
                  "arr1",
                  "arr1",
                  "$.M3",
                   DkmClrCompilationResultFlags.None,
                  DkmEvaluationResultCategory.Data,
                  null));

             return DkmCompiledClrLocalsQuery.Create(
                   inspectionContext.RuntimeInstance,
                   null,
                   inspectionContext.Language.Id,
                   new ReadOnlyCollection<byte>(testBytes),
                   "$.C0",
                   locals.AsReadOnly());                                    
        }

2) The locals.dll from the previous point is the following assembled file (where you need to adjust the parameters and locals variables accordingly:

.assembly extern HELLO { }  // the name of the DLL I am debugging
.assembly extern mscorlib
{
  .ver 4:0:0:0
  .publickeytoken = (b77a5c561934e089)
}
.assembly $.C0 { }

.class public $.C0
{
   .method public hidebysig static class [mscorlib]System.Array $.M3() cil managed

   {
        .maxstack 32
        .locals init ( [0] class [mscorlib]System.Array myres)  
        ldtoken [mscorlib]System.String
    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
    ldc.i4.s 9
     call class [mscorlib]System.Array [mscorlib]System.Array::CreateInstance(class [mscorlib]System.Type, int32)               
     stloc myres         
     ldloc myres    
     ret
    }
}

The only feedback I am getting from Visual Studio is:

The debugger is unable to evaluate this expression

Another interesting fact, that looks like a bug: If I reference a local with the index out of bounds the Experimental instance of Visual Studio freezes and the msvsmon crashes a couple of minutes later. So for example in my $.M3() method I have only 1 local with the index 0 and if I write an instruction stloc 1 it will crash. I do understand that this code is completely wrong, but a freeze and a crash seems to be an inadequate response.

In a couple of hours I'll be on vacation too, till Monday. Plus by looking at the times when your are posting your messages, it looks like we have a big time difference: you, probably being in USA, and me in Europe (Belgium).

plnelson commented 7 years ago

If you get the message "The debugger is unable to evaluate this expression", you may be able to figure out what's going on by debugging devenv.exe (or msvsmon if 64-bit debugging) and enable stopping on Ilrun.InterpreterExecutionException. Make sure you have Just My Code turned off first. When the exception is thrown, take a look at the message. These messages tend to be too low level to show the user and are intended for us debugger developers to understand, but they may also make sense to someone working on a compiler.

In terms of the crash with invalid IL code: This is a known problem. The CLR emulator is not at all hardened against invalid IL. We have been gradually fixing these bugs, but we tend to prioritize the most commonly encountered problems. The C#/VB compilers very rarely output invalid IL code in my experience. If you run into a crash and you can't figure out why, stopping on the exception as above may give you an idea as to what's going on.

onor13 commented 7 years ago

Yes the message I am getting is "The debugger is unable to evaluate this expression". I added "Ilrun.InterpreterExecutionException" to the Exception Settings. Does it matter to which category I am adding? I tried with Common Language Runtime Exception and Managed Debugging Assistants but did not get any additional messages.

plnelson commented 7 years ago

Yes, it's a CLR exception. You have to be debugging devenv or msvsmon. Take a look at the Output window to see which exceptions are thrown. You may also need to add AssertedInterpreterExecutionException.

onor13 commented 7 years ago

Still nothing. The only exception I see is

Exception thrown: 'System.ArgumentException' in vsdebugeng.manimpl.dll

But this exception is always shown, no matter if the code is correct or not. I am debugging devenv, is it even possible to debug msvsmon? Msvsmon is launched by devenv so I have no control over it, do I ?

plnelson commented 7 years ago

Yes, you can debug msvsmon. You can either attach to it or you can use the Child Process Debugging Power Tool

plnelson commented 7 years ago

Did you ever debug msvsmon? Is there any reason to leave this issue open?