[fpc-devel] Memory leak @ tobjectdef.getcopy

[Blaise at blaise.ru]

1) Maybe it was not always the case, but tobjectdef.vmtentries is never nil (initialised to an empty list @ .create & @ .ppuload. With the following two exceptions, there are no other assigned(vmtentries) checks before dereferencing. I propose that these two useless checks be removed.

@ TVMTWriter.writevirtualmethod :
> 	if not assigned(_class.VMTEntries) then	// <-- 1) USELESS: ALWAYS FALSE
> 	  exit;> 	for i:=0 to _class.VMTEntries.Count-1 do

@ tobjectdef.getcopy :
> 	if assigned(vmtentries) then	// <-- 1) USELESS: ALWAYS TRUE
> 	  begin
> 	    tobjectdef(result).vmtentries:=TFPList.Create;	// <-- 2) MEMORY LEAK
> 	    tobjectdef(result).copyvmtentries(self);
> 	  end;

2) More importantly, the above snipped leaks an empty TFPList when non-nil result.vmtentries is overwritten.

3) Bonus: @ tobjectdef.destroy : under "if assigned(vmtentries)", using vmtentries.free instead of .Destroy makes no sense.

The attached patch remedies all the above.

-- 
βþ
-------------- next part --------------
# HG changeset patch
# User Blaise.ru
# Date 1575215603 -10800
#      01.12.2019 18:53:23 +0300
# Node ID d2a8edb9ebac5e202c8d905936a55c973cd58023
# Parent  2ae482b988eb0d4f0eac5ce091b2c134d5998fd3
! memory leak @ tobjectdef.getcopy
= tobjectdef.vmtentries is never nil; no point in checking

diff -r 2ae482b988eb -r d2a8edb9ebac ncgvmt.pas
--- a/ncgvmt.pas	01.12.2019 12:23:49 +0300
+++ b/ncgvmt.pas	01.12.2019 18:53:23 +0300
@@ -966,8 +966,6 @@
          hs : string;
 {$endif vtentry}
       begin
-        if not assigned(_class.VMTEntries) then
-          exit;
         for i:=0 to _class.VMTEntries.Count-1 do
          begin
            vmtentry:=pvmtentry(_class.vmtentries[i]);
diff -r 2ae482b988eb -r d2a8edb9ebac symdef.pas
--- a/symdef.pas	01.12.2019 12:23:49 +0300
+++ b/symdef.pas	01.12.2019 18:53:23 +0300
@@ -7287,7 +7287,7 @@
          if assigned(vmtentries) then
            begin
              resetvmtentries;
-             vmtentries.free;
+             vmtentries.Destroy;
              vmtentries:=nil;
            end;
          if assigned(vmcallstaticinfo) then
@@ -7330,11 +7330,7 @@
             for i:=0 to ImplementedInterfaces.count-1 do
               tobjectdef(result).ImplementedInterfaces.Add(TImplementedInterface(ImplementedInterfaces[i]).Getcopy);
           end;
-        if assigned(vmtentries) then
-          begin
-            tobjectdef(result).vmtentries:=TFPList.Create;
-            tobjectdef(result).copyvmtentries(self);
-          end;
+        tobjectdef(result).copyvmtentries(self);
       end;