Fixed critical rtti cache bug

[nortg]

bug present in stable, trunk.

TRttiInfo.ComputeCache method assumes Cache parameter will be zeroed out by caller but this is not true because of usage of out prefix which discards initial value. The method does not correctly initialize Cache structure therefor it contains whatever garbage is currently on the stack. The only time out parameter would "work" is if Cache would contain another managed type field which would force the compiler to initialize the structure on method entry.

Fix by using var prefix so that Cache is passed as reference. To be honest because of importance of this method in RTTI generation I would not leave it up to the caller to correctly initialize the Cache record. I would still use out parameter but the first line should be Default(Cache); so that it zeroes out the structure.

See following example

program test;

{$I mormot.defines.inc}
uses // >
 {$I mormot.uses.inc}
  mormot.core.base,
  mormot.core.os,
  mormot.core.log,
  mormot.core.Text,
  mormot.orm.core,
  mormot.rest.sqlite3,
  mormot.DB.raw.sqlite3.static;

type
  TOrmTest = class(TOrm)
  private
    FName: RawUTF8;
  published
    property Name: RawUTF8 read FName write FName stored False;
  end;

var
  LModel: TOrmModel;
  FDB: TRestServerDB;
  LTest: TOrmTest;
begin
  LModel := TOrmModel.Create([TOrmTest], 'test');
  FDB := TRestServerDB.Create(LModel, 'test.db');
  LModel.Owner := FDB;
  FDB.Server.CreateMissingTables;
  try
    LTest := TOrmTest.Create;
    try
      LTest.Name := 'test';
      while FDB.Orm.Add(LTest, True) <> 0 do ;
    finally
      LTest.Free;
    end;
  finally
    FDB.Free;
  end;

end.

Because of Unique constraint on property "Name", Sqlite returns error if already exists in table.
This kicks off the following events

  raise DB exception
     serialize exception into json
        generate first time rtti for exception
           > incorrectly initializes property Cache flags with stack garbage
               causes json serialization EAccessViolation because of wrong assumption
               (for example Cache flag can contain rttiHasOrd flag despite being a string property,
               causes incorrect assignment of DEFAULT parameter (0 but should be NO_DEFAULT),
               the code then trips itself on invalid function call because it assumes ordinal get&setter)

[synopse]

Which compiler are you using?
On FPC and early Delphi compiler, there is no difference between "var" and "out" parameter (they are both just passed by value). As they should. Just check the generated asm to ensure this.
The https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Program_Control_(Delphi) official documentation does not mention "out" at all, and we expect a similar behavior to "var" for such records.
If it changed on some most recent Delphi compiler revision, it sounds like a regression of the compiler.

This method is only used internally, and only once from TRttiCustom.FromRtti() so it sounded safe to assume the cache record is filled with zeroes.

I will merge your pull request (which does not hurt for sure), but please discuss here or in the forum which compiler do you use, and ensure that there is really a difference between "var" and "out" by looking at the asm using Alt-F2 in the IDE.

[nortg]

Compiler is FPC 3.2.3 (fixes-3_2) and Lazarus 2.2.5 (fixes_2_2).
I think you meant passed by reference and not value.

Consider the following simple example

{$I mormot.defines.inc}
uses // >
 {$I mormot.uses.inc}
  typInfo;

type

  TTestEnum = (one, two, three, four, five);
  TTestSet = set of TTestEnum;
  PTest = ^TTest;
  TTest = record
    X: integer;
    Y: integer;
    E: TTestSet;
  end;

var
  LX: TTest;

  procedure TestVar(var ATest: TTest);
  begin
    ATest.Y := High(integer);
  end;

  procedure TestOut(out ATest: TTest);
  begin
    ATest.Y := High(integer);
  end;

begin

  LX := Default(TTest);
  TestVar(LX);
  Writeln('X = ', LX.X, ' | ', 'Y = ', LX.Y, ' | ', 'E = ',
    TypInfo.SetToString(TypInfo.PTypeInfo(TypeInfo(TTestSet)), PLongint(@LX.E)^, True));

  LX := Default(TTest);
  TestOut(LX);
  Writeln('X = ', LX.X, ' | ', 'Y = ', LX.Y, ' | ', 'E = ',
    TypInfo.SetToString(TypInfo.PTypeInfo(TypeInfo(TTestSet)), PLongint(@LX.E)^, True));

end.

On my computer using that compiler the out param function fills record with garbage except for the field specifically set. I would expect it do this because I've specifically told the compiler that I don't care about what the initial value of record is.

The documentation also confirms this
https://www.freepascal.org/docs-html/ref/refsu66.html

The purpose of an out parameter is to pass values back to the calling routine: the variable is passed by reference. The initial value of the parameter on function entry is discarded.

[nortg]

Assembly is also different

for TestVar

lea rdi,[rip+$00570384]
call TestVar
mov rax,rdi
mov [rax+$04], High(Integer)

where as TestOut

lea rdi,[rip+$00570287]
call TestOut
mov rax,[rbp-$08]
mov [rax+$04], High(Integer)

As you can see clearly not working from same pointer offset.

[synopse]

With my FPC 3.2.3 the writeln() output is the very same with TestVar and TestOut, and the asm is exactly the same.

P$MORMOT2TESTS_$$_TESTVAR$TTEST PROC
        mov     rax, rdi                                ; 0000 _ 48: 89. F8
        mov     dword ptr [rax+4H], 2147483647          ; 0003 _ C7. 40, 04, 7FFFFFFF
        ret                                             ; 000A _ C3

P$MORMOT2TESTS_$$_TESTOUT$TTEST PROC
        mov     rax, rdi                                ; 0000 _ 48: 89. F8
        mov     dword ptr [rax+4H], 2147483647          ; 0003 _ C7. 40, 04, 7FFFFFFF
        ret                                             ; 000A _ C3

also when calling the functions:

        lea     rdi, ptr [U_$P$MORMOT2TESTS_$$_LX]      ; 002A _ 48: 8D. 3D, 00000000(rel)
        call    P$MORMOT2TESTS_$$_TESTVAR$TTEST         ; 0031 _ E8, 00000000(PLT r)
        call    fpc_get_output                          ; 0036 _ E8, 00000000(PLT r)
...
        lea     rdi, ptr [U_$P$MORMOT2TESTS_$$_LX]      ; 00F2 _ 48: 8D. 3D, 00000000(rel)
        call    P$MORMOT2TESTS_$$_TESTOUT$TTEST         ; 00F9 _ E8, 00000000(PLT r)

I don't know where the asm you are showing comes from, but its does not make any sense anyway.
It sounds like if your FPC compiler is in a wrong/buggy state...

[synopse]

I also tried to reproduce your initial program test issue, but there is no problem at all on my side - neither with FPC or Delphi.
I run it several times with no issue, and debugging it shows no problem about the cache.

Something seems wrong with your FPC tool chain...

[nortg]

Well that's awkward ...
Found out what it is, I had debug option "Trash variables" (-gt) compiler switch on while debugging.
Without this flag I get same assembly as you. Oh well, if it were not for your help I would probably waste even more days on this 👍

[synopse]

It makes sense then. :)

You may have asked directly on the forum, for better discussion.
But we will keep your PR modification, because it is an internal method, and it won't hurt to have "var" instead of "out".