diff mbox series

[Ada] Missing finalization of cursor in "of" iterator loop

Message ID 20170906102736.GA116519@adacore.com
State New
Headers show
Series [Ada] Missing finalization of cursor in "of" iterator loop | expand

Commit Message

Arnaud Charlet Sept. 6, 2017, 10:27 a.m. UTC
This patch modifies the finalization machinery to ensure that the cursor of an
"of" iterator loop is properly finalized at the end of the loop. Previously it
was incorrectly assumed that such a cursor will never need finalization
ctions.

------------
-- Source --
------------

--  leak.adb

pragma Warnings (Off);

with Ada.Unchecked_Deallocation;
with Ada.Finalization;
with Ada.Iterator_Interfaces;
with Ada.Text_IO; use Ada.Text_IO;

procedure Leak is
   type El is tagged null record;

   type Integer_Access is access all Integer;

   procedure Unchecked_Free is new Ada.Unchecked_Deallocation
     (Integer, Integer_Access);

   type Cursor is new Ada.Finalization.Controlled with record
      Count : Integer_Access := new Integer'(1);
   end record;

   overriding procedure Adjust (C : in out Cursor);
   overriding procedure Finalize (C : in out Cursor);

   overriding procedure Adjust (C : in out Cursor) is
   begin
      C.Count.all := C.Count.all + 1;
      Put_Line ("Adjust   Cursor.   Count = " & C.Count.all'Img);
   end Adjust;

   overriding procedure Finalize (C : in out Cursor) is
   begin
      C.Count.all := C.Count.all - 1;
      Put_Line ("Finalize Cursor.   Count = " & C.Count.all'Img);
      if C.Count.all = 0 then
         Unchecked_Free (C.Count);
      end if;
   end Finalize;

   function Has_Element (C : Cursor) return Boolean is (False);

   package Child is
      package Iterators is new Ada.Iterator_Interfaces
        (Cursor       => Cursor,
         Has_Element  => Has_Element);

      type Iterator is
        new Ada.Finalization.Controlled
          and Iterators.Forward_Iterator
      with record
         Count : Integer_Access := new Integer'(1);
      end record;

      overriding function First (I : Iterator) return Cursor
      is (Ada.Finalization.Controlled with others => <>);

      overriding function Next (I : Iterator; C : Cursor) return Cursor
      is (Ada.Finalization.Controlled with others => <>);

      overriding procedure Adjust (I : in out Iterator);

      end Child;

   package body Child is
      overriding procedure Adjust (I : in out Iterator) is
      begin
         I.Count.all := I.Count.all + 1;
         Put_Line ("Adjust   Iterator. Count = " & I.Count.all'Img);
      end Adjust;

      overriding procedure Finalize (I : in out Iterator) is
      begin
         I.Count.all := I.Count.all - 1;
         Put_Line ("Finalize Iterator. Count = " & I.Count.all'Img);
         if I.Count.all = 0 then
            Unchecked_Free (I.Count);
         end if;
      end Finalize;
   end Child;

   type Iterable is tagged null record
     with Default_Iterator  => Iterate,
          Iterator_Element  => El'Class,
          Constant_Indexing => El_At;

   function Iterate
     (O : Iterable) return Child.Iterators.Forward_Iterator'Class
   is (Child.Iterator'(Ada.Finalization.Controlled with others => <>));

   function El_At (Self : Iterable; Pos : Cursor'Class) return El'Class
   is (El'(others => <>));

   Seq : Iterable;

begin
   Put_Line ("START");
   for V of Seq loop
      null;
   end loop;
   Put_Line ("END");
end Leak;

----------------------------
-- Compilation and output --
----------------------------

$ gnatmake -q leak.adb -largs -lgmem
$ ./leak
$ gnatmem ./leak > leaks.txt
$ grep -c "Number of non freed allocations" leaks.txt
START
Adjust   Iterator. Count =  2
Finalize Iterator. Count =  1
Adjust   Cursor.   Count =  2
Finalize Cursor.   Count =  1
Adjust   Cursor.   Count =  2
Finalize Cursor.   Count =  1
Finalize Cursor.   Count =  0
Finalize Iterator. Count =  0
END
0

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-09-06  Hristian Kirtchev  <kirtchev@adacore.com>

	* einfo.adb (Status_Flag_Or_Transient_Decl): The attribute is now
	allowed on loop parameters.
	(Set_Status_Flag_Or_Transient_Decl): The attribute is now allowed
	on loop parameters.
	(Write_Field15_Name): Update the output for
	Status_Flag_Or_Transient_Decl.
	* einfo.ads: Attribute Status_Flag_Or_Transient_Decl now applies
	to loop parameters. Update the documentation of the attribute
	and the E_Loop_Parameter entity.
	* exp_ch7.adb (Process_Declarations): Remove the bogus guard
	which assumes that cursors can never be controlled.
	* exp_util.adb (Requires_Cleanup_Actions): Remove the bogus
	guard which assumes that cursors can never be controlled.
diff mbox series

Patch

Index: exp_ch7.adb
===================================================================
--- exp_ch7.adb	(revision 251753)
+++ exp_ch7.adb	(working copy)
@@ -2100,15 +2100,6 @@ 
                elsif Is_Ignored_Ghost_Entity (Obj_Id) then
                   null;
 
-               --  The expansion of iterator loops generates an object
-               --  declaration where the Ekind is explicitly set to loop
-               --  parameter. This is to ensure that the loop parameter behaves
-               --  as a constant from user code point of view. Such object are
-               --  never controlled and do not require finalization.
-
-               elsif Ekind (Obj_Id) = E_Loop_Parameter then
-                  null;
-
                --  The object is of the form:
                --    Obj : [constant] Typ [:= Expr];
 
Index: exp_util.adb
===================================================================
--- exp_util.adb	(revision 251762)
+++ exp_util.adb	(working copy)
@@ -11972,16 +11972,6 @@ 
             elsif Is_Ignored_Ghost_Entity (Obj_Id) then
                null;
 
-            --  The expansion of iterator loops generates an object declaration
-            --  where the Ekind is explicitly set to loop parameter. This is to
-            --  ensure that the loop parameter behaves as a constant from user
-            --  code point of view. Such object are never controlled and do not
-            --  require cleanup actions. An iterator loop over a container of
-            --  controlled objects does not produce such object declarations.
-
-            elsif Ekind (Obj_Id) = E_Loop_Parameter then
-               return False;
-
             --  The object is of the form:
             --    Obj : [constant] Typ [:= Expr];
             --
Index: einfo.adb
===================================================================
--- einfo.adb	(revision 251760)
+++ einfo.adb	(working copy)
@@ -3371,7 +3371,9 @@ 
 
    function Status_Flag_Or_Transient_Decl (Id : E) return N is
    begin
-      pragma Assert (Ekind_In (Id, E_Constant, E_Variable));
+      pragma Assert (Ekind_In (Id, E_Constant,
+                                   E_Loop_Parameter,
+                                   E_Variable));
       return Node15 (Id);
    end Status_Flag_Or_Transient_Decl;
 
@@ -6546,7 +6548,9 @@ 
 
    procedure Set_Status_Flag_Or_Transient_Decl (Id : E; V : E) is
    begin
-      pragma Assert (Ekind_In (Id, E_Constant, E_Variable));
+      pragma Assert (Ekind_In (Id, E_Constant,
+                                   E_Loop_Parameter,
+                                   E_Variable));
       Set_Node15 (Id, V);
    end Set_Status_Flag_Or_Transient_Decl;
 
@@ -10087,6 +10091,7 @@ 
             Write_Str ("Related_Instance");
 
          when E_Constant
+            | E_Loop_Parameter
             | E_Variable
          =>
             Write_Str ("Status_Flag_Or_Transient_Decl");
Index: einfo.ads
===================================================================
--- einfo.ads	(revision 251760)
+++ einfo.ads	(working copy)
@@ -4325,12 +4325,12 @@ 
 --       expression may consist of the above xxxPredicate call on its own.
 
 --    Status_Flag_Or_Transient_Decl (Node15)
---       Defined in variables and constants. Applies to objects that require
---       special treatment by the finalization machinery, such as extended
---       return results, IF and CASE expression results, and objects inside
---       N_Expression_With_Actions nodes. The attribute contains the entity
---       of a flag which specifies particular behavior over a region of code
---       or the declaration of a "hook" object.
+--       Defined in constant, loop, and variable entities. Applies to objects
+--       that require special treatment by the finalization machinery, such as
+--       extended return results, IF and CASE expression results, and objects
+--       inside N_Expression_With_Actions nodes. The attribute contains the
+--       entity of a flag which specifies particular behavior over a region of
+--       code or the declaration of a "hook" object.
 --       In which case is it a flag, or a hook object???
 
 --    Storage_Size_Variable (Node26) [implementation base type only]
@@ -5846,7 +5846,7 @@ 
    --    Esize                               (Uint12)
    --    Extra_Accessibility                 (Node13)   (constants only)
    --    Alignment                           (Uint14)
-   --    Status_Flag_Or_Transient_Decl       (Node15)   (constants only)
+   --    Status_Flag_Or_Transient_Decl       (Node15)
    --    Actual_Subtype                      (Node17)
    --    Renamed_Object                      (Node18)
    --    Size_Check_Code                     (Node19)   (constants only)