Thursday, June 14, 2012

Bug or no bug - that is the question

Or with other words: when something is not what it looks to be - and you have no clue why.

Let me explain: Recently over on Nicks blog Márton mentioned that TRttiMethod.Invoke cannot handle var and out parameters. While I already created a runtime patch for the bug in the QC entry for 2010 and XE I was not sure about the handling of var and out parameters. I remembered I ran into some problem with calling the Invoke routine and Barry gave me the correct hint on how to handle passing values by reference. So I tried it:


program Project1;

{$APPTYPE CONSOLE}

uses
  Rtti;

type
  TTest = class
  public
    procedure CallVar(var i: Integer);
  end;

procedure TTest.CallVar(var i: Integer);
begin
  Inc(i);
end;

var
  test: TTest;
  ctx: TRttiContext;
  t: TRttiType;
  m: TRttiMethod;
  i: Integer;
begin
  test := TTest.Create;
  t := ctx.GetType(TTest);
  m := t.GetMethod('CallVar');
  i := 42;
  m.Invoke(test, [TValue.From<Integer>(i)]);
  Writeln(i);
  test.Free;
  Readln;
end.

It showed 42. First thought: yes, he is right, it does not handle them correctly. Second thought: wait, TValue is not taking any kind of reference to i. It just takes the value and stores it. So I changed the program a bit to check what was in the passed TValue argument.


var
  [...]
  v: TValue;
begin
  [...]
  v := TValue.From<Integer>(i);
  m.Invoke(test, [v]);
  Writeln(v.AsInteger);
  [...]
end.

Output remained 42. I messed around with passing the pointer to i inside the TValue but then the invoke method raised the EInvalidCast exception telling me: 'VAR and OUT arguments must match parameter type exactly'. I knew that this method checks this (not like the Invoke routine mentioned earlier) and passes them correctly. So what was going on? I changed it again:

var
  [...]
  v: TArray<TValue>;
begin
  [...]
  SetLength(v, 1);
  v[0] := TValue.From<Integer>(i);
  m.Invoke(test, v);
  Writeln(v[0].AsInteger);
  [...]
end.

Hooray, it showed the expected 43. What happened here? In the case where it did not work I used the open array constructor. The documentation says that it equivalent to passing a static array filled with the values passed to the open array constructor. Ok, I tested that:

var
  [...]
  v: array[0..0] of TValue;
begin
  [...]
  v[0] := TValue.From<Integer>(i);
  m.Invoke(test, v);
  Writeln(v[0].AsInteger);
  [...]
end.

Guess what? It returns 43. Seems it is not equivalent. Could it be that the code the compiler creates for an open array constructor does not handle the nested record inside of TValue - TValueData where the actual value is stored in - correctly? I was staring at the assembler code but as you know I pretty much suck reading something out there. While I was glad that TRttiMethod.Invoke actually handles var and out parameters correctly I could make a fix for DSharp mocks. But I still have no clue why passing the value with the open array constructor does not keep the value. Any ideas?

12 comments:

  1. If you put the TValue into an open array parameter, the value is copied into the array and the reference to that array is pushed on the stack.
    The static array on the other side is larger than 1, 2, 4, 8 bytes (SizeOf(TValue) = 24) and so a reference to the array is pushed on the stack.
    With the static array you can access the element on the caller side, whereas with the open array parameter the caller has no direct access to it so you can't read the modified element.

    ReplyDelete
    Replies
    1. That makes perfect sense. Thank you for explaining.

      Delete
  2. While Andreas explained what happens, let me say that this is not handling var and out parameters, it's just luck. Or maybe it's just a matter of how you think about var parameters. When I see this:

    procedure P(var X: Integer);

    I don't think about passing an address, (who knows on what platform this will eventually run, as far as I'm concerned, all parameters could be passed by reference) I see the declaration as "procedure P can modify the variable I pass in X". So when I look at TRttiMethod.Invoke, "const Args..." translates to me as "whatever you pass, you won't get back anything". And if I have a variable that I pass in a const parameter, I expect it to keep it's value, regardless of the variable's location in memory, calling convention, etc. So in my opinion, Rtti method invocation could be fixed either by adding a new, let's say InvokeEx method with (var Args: array of TValue) parameter or performing a check in Invoke and raising an exception if the method has var and out parameters.

    ReplyDelete
    Replies
    1. I completely agree. The TRttiMethod.Invoke method has a very ugly inconsistent behaviour (i.e. modifying the values I passed as const!). I think this is caused by the fact how the array is passed and not something we should rely on in the future.

      Since for example in DSharp mocks you don't call the Invoke method yourself (you call the actual interface method which is compiler safe) when using them the internal handling can rely on that behaviour (for now).

      Delete
    2. Yep, we use our invoking routines for test automation, scripting and remoting as well, so in most cases the object receiving the parameters has absolutely no information about the context.

      Delete
    3. The problem with "const" is that Delphi doesn't have the const-ness of C++. The "const" in "const Values: array of TValue" means that the pointer to the array can't be changed, so SetLength won't work (if it relocates the array). It doesn't mean that the content of the array is const. That would be "const Values: array of const TValue" what Delphi doesn't support.
      For static arrays the parameter is the array, so "const" means that the whole array with all elements is const.

      You have to think of the dynamic/open array as an object and the static array as a record. Even if you use "const Obj: TMyObject" you can change the object's content, but you can't assign a new object to the parameter. For "const Rec: TMyRecord" you can't change the content.

      Delete
    4. I disagree. When a procedure has and array parameter (array of ), you don't know if there will be a dynamic or static array behind that value when the procedure is called. Maybe exactly for this reason, the following generates a compile error:

      procedure P(const Values: array of Variant);
      begin
        Values[0] := 'FUBAR';
      end;

      However, this compiles, but violates the premise that the value passed will not change:

      procedure P(const V: Variant);
      begin
        PVariant(@V)^ := 'FUBAR';
      end;

      BTW can we post syntax-formatted code segments in comments?

      Delete
    5. You are right. I shouldn't have used "open array" here. They act like static arrays and not like dynamic arrays because the function doesn't know if you specified a dynamic array, a static array or an open array constructor (=>compiler creates a static array for it). So the compiler uses the safe way and treats it like a static array.
      Maybe that is because when open array parameters were added to the language, there were no dynamic array.

      So static arrays and open array parameters are like records whereas dynamic arrays like objects.

      Delete
    6. The problem is that inside the Invoke call the TValue arguments are manipulated using methods like TValue.GetReferenceToRawData and such. iirc there is some QC entry that is about using methods on const parameters to manipulate them and how that violates the const.

      @Márton: Can't atm. I have to figure out how to enable the appropiate html tag in the comments.

      Delete
  3. Your use of RTTI and Invoke obfuscate the matter a little. If you pass values using an open array constructor, a NEW array is created, on the stack, and the values are placed inside it. Then a pointer to the first element of that array is passed (alongside with the High() value of the array, as integer).

    If you pass an existing static or dynamic array, a pointer to the first element of those is passed. Now, in the case of an open array constructor, you are still modifying values in the array passed, but that contains volatile copies on the stack, whereas if you pass a really existing array (static or dynamic), you are modifying the originals.

    So what you see makes perfect sense, IMO.

    ReplyDelete
  4. By the way, I suspect the Win64 support of RTTI.pas may also be broken, somehow.

    When I implemented the interface-based service low-level x64 asm stubs for mORMot, I found out that the official VCL unit was not putting a reference-counted function result as the 2nd parameter. It put it as the last argument, just as with x86. But this was not correct, when you compare how Delphi XE2/XE3 is generating x64 code for such functions.

    So I suspect the Win64 target has also some bugs. I did not test it (since we do not use this unit), but I suppose it may be broken.

    This is one of the reasons (in addition to speed and support for older versions like Delphi 6/7/2007), we re-invented the wheel and use our own low-level RTTI stubs for mORMot. With the supplied regression tests, it was easy to add x64 support. And our mORMot.pas unit handles var/out parameters as expected.

    BTW, do you have any details/informations about the RttiPatch.pas, InterfaceRttiPatch.pas and ObjAutoPatch.pas units supplied with your libraries?

    ReplyDelete
    Replies
    1. Thanks for letting me know about the x64 bug - has it been submitted into QC yet?

      As I am slowly learning all the dirty internals of RTTI and assembly crafting I was glad I could use (almost) working code without having to implement it on my own ;)

      What details/informations do you need? Feel free to contact me by email.

      Delete