Closed nortg closed 7 months ago
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.
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
.
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.
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...
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...
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 :+1:
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".
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
Because of Unique constraint on property "Name", Sqlite returns error if already exists in table. This kicks off the following events