BytesToVar code opinions
lazarus / 64-FPC development

----------------------------------------------------------------------
lagprogramming · 2023-07-31 11:50

packages/fcl-hash/src/fphashutils.pp has
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
BytesToVar(const aBytes: TBytes; out aLocation: Pointer); begin 
BytesToVar(aBytes,aLocation,Length(aBytes));end; 
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
BytesToVar(const aBytes: TBytes; out aLocation; aSize:
Integer); begin  if aSize>Length(aBytes) then   
aSize:=Length(aBytes);  Move(aBytes[0],aLocation,aSize);end;
The move procedure needs an initialized aLocation parameter.

I've searched for BytesToVar in the FPC directory an I've noticed that
in some routines the "out aLocation" parameter has a value assigned
prior to calling the BytesToVar procedure, while other routines don't
assign a value to this parameter.
Look at how procedure TTestECDSASHA256.TestVerify; assigns a value to
the aSignature variable by calling TECDSA.SignSHA256 vs. how
aSignature and Privatekey variables are used in:
Class function TJWTSignerES256.Verify(const aJWT: String; aPrivateKey:
TECCPrivateKey): Boolean;
procedure TTestECDSASHA256.TestVerifyFromKey;
procedure PemLoadPublicKey64FromList(List: TStrings; out PrivateKey:
TEccPrivateKey; out PublicKey: TEccPublicKey; out PublicKeyX64,
PublicKeyY64: AnsiString);

It looks like a bug. Either "out aLocation;" should be changed with
"var aLocation;", either a memory block should be allocated to
aLocation inside procedure BytesToVar(const aBytes: TBytes; out
aLocation; aSize: Integer);
What do you think about this procedure?

----------------------------------------------------------------------
AlexTP · 2023-07-31 12:00

Posted to https://gitlab.com/freepascal.org/fpc/source/-/issues/40372

----------------------------------------------------------------------
Thaddy · 2023-07-31 17:22

According to the documentation it is valid to use initialized out
parameters.
https://www.freepascal.org/docs-html/ref/refsu66.html so there is no
bug.
You may want to remove your report or close it.

----------------------------------------------------------------------
lagprogramming · 2023-07-31 17:55

Quote from: Thaddy on July 31, 2023, 07:22:57 pmAccording to the
documentation it is valid to use initialized out parameters.
https://www.freepascal.org/docs-html/ref/refsu66.html so there is no
bug.
You may want to remove your report or close it.
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
BytesToVar(const aBytes: TBytes; out aLocation; aSize:
Integer); begin  if aSize>Length(aBytes) then   
aSize:=Length(aBytes);  Move(aBytes[0],aLocation,aSize);end;QuoteMove
Synopsis: Move data from one location in memory to another
Declaration: procedure Move(const source; var dest; count: SizeInt)
QuoteIf a variable must be used to pass a value to a function and
retrieve data from the function, then a variable parameter must be
used. If only a value must be retrieved, a out parameter can be used.
The move procedure needs a valid destination(aLocation) with an
allocated memory block. Notice the declaration: procedure
BytesToVar(const aBytes: TBytes; out aLocation; aSize: Integer);

----------------------------------------------------------------------
Thaddy · 2023-07-31 18:15

Yes. aLocation must be allocated with GetMem/AllocMem and the caller
is responsible for its release: aLocation needs heap memory. That is
the only caveat I can see if aLocation is not externally initialized.
That may require a remark in the documentation. In this case aLocation
is assumed to be initialized.

----------------------------------------------------------------------
PascalDragon · 2023-07-31 18:41

Quote from: lagprogramming on July 31, 2023, 01:50:09 pmIt looks like
a bug. Either "out aLocation;" should be changed with "var
aLocation;", either a memory block should be allocated to aLocation
inside procedure BytesToVar(const aBytes: TBytes; out aLocation;
aSize: Integer);
What do you think about this procedure?

It is perfectly fine to pass a formal out-parameter to a formal var-
parameter, because the memory the parameter references needs to be
valid in both cases.
Also Move does not require its destination parameter to have
initialized data. Nowadays it would probably have out instead of var,
but Move is from TP times which doesn't know out.

----------------------------------------------------------------------
lagprogramming · 2023-08-01 06:52

I see the issue has been closed. The following code has a memory leak.
Where is the bug in the following code?
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program
Project1;//Regarding memory leaks, what's wrong with this code?uses
sysutils, fpecc, fphashutils; var  p:pointer;  a:tbytes;begin 
setlength(a,sizeof(TECCSignature));  //Call bytestovar with valid
parameteres  bytestovar(a,p);  //Write some data 
writeln('Pointer='+inttostr(sizeuint(p))); 
writeln('Sizeof(TECCSignature)='+inttostr(sizeof(TECCSignature))); 
//Manually free memory  setlength(a,0);  freemem(p);{Output with
heaptrc (-gh):Pointer=0Sizeof(TECCSignature)=64Heap dump by heaptrc
unit of "/project1"14 memory blocks allocated : 1634/167213 memory
blocks freed     : 1554/15921 unfreed memory blocks : 80True heap size
: 393216True free heap : 392928Should be : 392944}end.

----------------------------------------------------------------------
lagprogramming · 2023-08-02 08:20

Shouldn't the gitlab issue be reopened if there's nothing wrong with
the program's pascal lines!? After all, the memory leak is there and
the pointer is nil.
https://gitlab.com/freepascal.org/fpc/source/-/issues/40372

----------------------------------------------------------------------
bytebites · 2023-08-02 17:28

But pointer p has not valid value.

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program
Project1;{$mode objfpc}{$H+}//Regarding memory leaks, what's wrong
with this code?uses sysutils, fpecc, fphashutils;var 
p:pointer;//array[1..90] of char;  a:tbytes;  e: pchar absolute
p; procedure BytesToVar2(const aBytes: TBytes; out aLocation; aSize:
Integer); begin  if aSize>Length(aBytes) then   
aSize:=Length(aBytes);  Move(aBytes[0],aLocation,aSize);end; procedure
BytesToVar2(const aBytes: TBytes; out aLocation: Pointer); begin 
BytesToVar2(aBytes,aLocation^,Length(aBytes));end; begin  a:=nil; 
setlength(a,sizeof(TECCSignature));  a[0]:=65;  //Call bytestovar with
valid parameteres  p:=getmem(sizeof(TECCSignature));  e:=pchar(p); 
bytestovar2(a,p);  writeln(e);  //Write some data 
writeln('Pointer='+inttostr(sizeuint(p))); 
writeln('Sizeof(TECCSignature)='+inttostr(sizeof(TECCSignature))); 
//Manually free memory  setlength(a,0);  freemem(p);end.
This works if Alocation is derefenced:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
BytesToVar(aBytes,aLocation^,Length(aBytes));

----------------------------------------------------------------------
lagprogramming · 2023-08-03 08:50

@bytebites
Yes, but most likely it works by mistake due to a compiler poor
optimization. :'(
Change the line
procedure BytesToVar2(const aBytes: TBytes; out aLocation: Pointer);
with
procedure BytesToVar2(const aBytes: TBytes; var aLocation: Pointer);
and notice that the trunk(development) version of FPC produces the
same code(use the -a compiler parameter to keep the assembler files on
disk).