[Bug ada/114710] New: Temporary object finalized too late

[Bug ada/114710] New: Temporary object finalized too late

[jhb.chat at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

            Bug ID: 114710
           Summary: Temporary object finalized too late
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ada
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jhb.chat at gmail dot com
                CC: dkm at gcc dot gnu.org
  Target Milestone: ---

Created attachment 57940
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57940&action=edit
temporrary finalized too late

Version:  GCC/GNAT version 13.2.0, Win10 msys2 and Godbolt

Godbolt indicates this is a long standing functionality (all the way down to
8.2, which is the last I could check there).

When making a scoped lock / scoped mutex, I found that temporary variables
generated by a function were not being finalized after they were no longer
used.  This lead to a deadlock situation with the scope lock / scoped mutex. 
There doesn't appear to be any reason to hold onto the object returned by the
Lock function in this example. The call to the Lock function returns a
temporary object that is never saved and should be finalized immediately

In the example below, you can see the finalization happen after the body of
main completes instead of before by commenting out the v2 declaration (to
remove the deadlock).

Example:
-------------------------------------------------------
with Ada.Finalization;
with Ada.Text_IO; use Ada.Text_IO;

procedure Example is

    package Deadlocker is

        protected type Mutex is
            entry Lock;
            procedure Unlock;
        private
            Locked : Boolean := False;
        end Mutex;

        type Instance
            (Element : not null access Integer) 
        is 
            new Ada.Finalization.Limited_Controlled
        with record
            M : not null access Mutex;
        end record;

        overriding procedure Finalize(Self : in out Instance);

        type Object is tagged limited record
            Locker : aliased Mutex;
            Item : aliased Integer;
        end record;

        function Lock
            (Self : aliased in out Object) 
             return Instance'Class;

    end Deadlocker;

    package body Deadlocker is
        protected body Mutex is
            entry Lock when not Locked is
            begin
                Locked := True;
                Put_Line("Locked!");
            end Lock;

            procedure Unlock is
            begin
                Locked := False;
                Put_Line("Unlocked!");
            end Unlock;
        end Mutex;

        function Lock
            (Self : aliased in out Object) 
            return Instance'Class
        is begin
            return Result : Instance := Instance'
                (Ada.Finalization.Limited_Controlled with
                 Element => Self.Item'Access, 
                 M       => Self.Locker'Access)
            do 
                Put_Line("Locking...");
                Result.M.Lock;
            end return;
        end Lock;

        procedure Finalize(Self : in out Instance) is
        begin
            Put_Line("Unlocking...");
            Self.M.Unlock;
        end Finalize;
    end Deadlocker;

    Test : Deadlocker.Object;
    v1 : constant Integer := Test.Lock.Element.all; -- Should release lock
after
    v2 : constant Integer := Test.Lock.Element.all; -- Should release lock
after
begin
    Put_Line("In main body");
end Example;
-------------------------------------------------------

Output:
-------------------------------------------------------
Program returned: 143
Program stdout

Locking...
Locked!
Locking...

Program stderr


Killed - processing time exceeded
Program terminated with signal: SIGKILL
-------------------------------------------------------


Expected Output:
-------------------------------------------------------
Program returned: 0
Program stdout

Locking...
Locked!
Unlocking...
Unlocked!
Locking...
Locked!
Unlocking...
Unlocked!
In main body
-------------------------------------------------------

[Bug ada/114710] temporary object finalized too late

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|Temporary object finalized  |temporary object finalized
                   |too late, causing deadlock  |too late
   Last reconfirmed|                            |2024-04-20
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Yes, this works as expected in simpler cases, but not here.

[Bug ada/114710] temporary object finalized too late

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ebotcazou at gcc dot gnu.org

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I'll have a look.

[Bug ada/114710] too late finalization of temporary object

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Marc Poulhi?s <dkm@gcc.gnu.org>:

https://gcc.gnu.org/g:9e490bea69205ec4cad8caf21f19d8a8a89a7b43

commit r15-1260-g9e490bea69205ec4cad8caf21f19d8a8a89a7b43
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Mon Apr 22 09:35:44 2024 +0200

    ada: Fix too late finalization of temporary object

    The problem is that Is_Finalizable_Transient returns false when a transient
    object is subject to a renaming by another transient object present in the
    same transient scope, thus forcing its finalization to be deferred to the
    enclosing scope.  That's not necessary, as only renamings by nontransient
    objects serviced by transient scopes need to be rejected by the predicate.

    The change also removes now dead code in the finalization machinery.

    gcc/ada/

            PR ada/114710
            * exp_ch7.adb (Build_Finalizer.Process_Declarations): Remove dead
            code dealing with renamings.
            * exp_util.ads (Is_Finalizable_Transient): Rename Rel_Node to N.
            * exp_util.adb (Is_Finalizable_Transient): Likewise.
            (Is_Aliased): Remove obsolete code dealing wih EWA nodes and only
            consider renamings present in N itself.
            (Requires_Cleanup_Actions): Remove dead code dealing with
renamings.

[Bug ada/114710] too late finalization of temporary object

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

--- Comment #4 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Marc Poulhi?s <dkm@gcc.gnu.org>:

https://gcc.gnu.org/g:916a0f026e1516de4608b308cd7e4b68b8e562bb

commit r15-1264-g916a0f026e1516de4608b308cd7e4b68b8e562bb
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Tue Apr 23 19:54:32 2024 +0200

    ada: Fix fallout of previous finalization change

    Now that Is_Finalizable_Transient only looks at the renamings coming from
    nontransient objects serviced by transient scopes, it must find the object
    ultimately renamed by them through a chain of renamings.

    gcc/ada/

            PR ada/114710
            * exp_util.adb (Find_Renamed_Object): Recurse if the renamed object
            is itself a renaming.

[Bug ada/114710] too late finalization of temporary object

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Marc Poulhi?s <dkm@gcc.gnu.org>:

https://gcc.gnu.org/g:72ccc99d6fc22aaae4aaf74027329acafd70cbfc

commit r15-1271-g72ccc99d6fc22aaae4aaf74027329acafd70cbfc
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Thu Apr 25 16:05:33 2024 +0200

    ada: Fix oversight in latest finalization fix

    The Defining_Identifier of a renaming may be a E_Constant in the context.

    gcc/ada/

            PR ada/114710
            * exp_util.adb (Find_Renamed_Object): Recurse for any renaming.

[Bug ada/114710] too late finalization of temporary object

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114710

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |15.0
         Resolution|---                         |FIXED

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Fixed on the mainline.