[PATCH] fortran: Remove redundant was_finalized tracking mechanism

[Antony Lewis]

The was_finalized tracking mechanism was introduced in commit 1af22e4 to
prevent duplicate finalization of the same expression+component combination
within a namespace. However, this mechanism had a flaw: it
stored gfc_expr* pointers that could become stale, leading to use-after-free
bugs and compiled code memory leaks as reported in PR120637.

Upon analysis, this tracking mechanism is redundant because the finalization
logic already handles proper scoping and lifetime management.
The original problem that commit 1af22e4 was trying to solve is better
handled by the existing finalization logic without needing to cache
expression pointers.

This change removes the problematic caching mechanism while maintaining
correct finalization behavior. The test case from the original commit
(finalize_36.f90) continues to pass, confirming that the memory leak
originally fixed is still resolved without the redundant tracking.

PR fortran/120637

gcc/fortran/ChangeLog:

* class.cc (finalize_component): Remove redundant was_finalized
check and tracking code.
* gfortran.h (gfc_was_finalized): Remove structure definition.
(gfc_namespace): Remove was_finalized field.
* symbol.cc (gfc_free_namespace): Remove was_finalized cleanup code.
---
 gcc/fortran/class.cc   | 16 
 gcc/fortran/gfortran.h | 11 ---
 gcc/fortran/symbol.cc  | 10 --
 3 files changed, 37 deletions(-)

diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc
index df18601e45b..af6ac3c1e99 100644
--- a/gcc/fortran/class.cc
+++ b/gcc/fortran/class.cc
@@ -1041,19 +1041,10 @@ finalize_component (gfc_expr *expr, gfc_symbol 
*derived, gfc_component *comp,
 {
   gfc_expr *e;
   gfc_ref *ref;
-  gfc_was_finalized *f;

   if (!comp_is_finalizable (comp))
 return;

-  /* If this expression with this component has been finalized
- already in this namespace, there is nothing to do.  */
-  for (f = sub_ns->was_finalized; f; f = f->next)
-{
-  if (f->e == expr && f->c == comp)
-   return;
-}
-
   e = gfc_copy_expr (expr);
   if (!e->ref)
 e->ref = ref = gfc_get_ref ();
@@ -1227,13 +1218,6 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, 
gfc_component *comp,
sub_ns);
   gfc_free_expr (e);
 }
-
-  /* Record that this was finalized already in this namespace.  */
-  f = sub_ns->was_finalized;
-  sub_ns->was_finalized = XCNEW (gfc_was_finalized);
-  sub_ns->was_finalized->e = expr;
-  sub_ns->was_finalized->c = comp;
-  sub_ns->was_finalized->next = f;
 }

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index f73b5f9c23f..97cbac4f5d9 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2205,15 +2205,7 @@ gfc_oacc_routine_name;

 #define gfc_get_oacc_routine_name() XCNEW (gfc_oacc_routine_name)

-/* Node in linked list to see what has already been finalized
-   earlier.  */

-typedef struct gfc_was_finalized {
-  gfc_expr *e;
-  gfc_component *c;
-  struct gfc_was_finalized *next;
-}
-gfc_was_finalized;

 /* A namespace describes the contents of procedure, module, interface block
or BLOCK construct.  */
@@ -2317,10 +2309,7 @@ typedef struct gfc_namespace
   struct gfc_omp_assumptions *omp_assumes;
   struct gfc_omp_namelist *omp_allocate;

-  /* A hash set for the gfc expressions that have already
- been finalized in this namespace.  */

-  gfc_was_finalized *was_finalized;

   /* Set to 1 if namespace is a BLOCK DATA program unit.  */
   unsigned is_block_data:1;
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 81aa81df2ee..29c5deeb6d2 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -4198,7 +4198,6 @@ gfc_free_namespace (gfc_namespace *&ns)
 {
   gfc_namespace *p, *q;
   int i;
-  gfc_was_finalized *f;

   if (ns == NULL)
 return;
@@ -4233,15 +4232,6 @@ gfc_free_namespace (gfc_namespace *&ns)

   gfc_free_data (ns->data);

-  /* Free all the expr + component combinations that have been
- finalized.  */
-  f = ns->was_finalized;
-  while (f)
-{
-  gfc_was_finalized* current = f;
-  f = f->next;
-  free (current);
-}
   if (ns->omp_assumes)
 {
   free (ns->omp_assumes->absent);
--
2.49.0

Re: [PATCH] fortran: Statically initialize length of SAVEd character arrays

[Jerry D]

On 6/18/25 2:02 PM, Mikael Morin wrote:

From: Mikael Morin 

  Regression-tested on x86_64-pc-linux-gnu.
  OK for master?


Was there a PR for this? or something you just ran into?

Not a problem either way, just curious.

It looks OK to me. OK for master.

Jerry


  -- >8 --

gcc/fortran/ChangeLog:

* trans-array.cc (gfc_trans_deferred_array): Statically
initialize deferred length variable for SAVEd character arrays.

gcc/testsuite/ChangeLog:

* gfortran.dg/save_alloc_character_1.f90: New test.

Re: [PATCH, part1, v3] Fortran: various fixes for STAT/LSTAT/FSTAT intrinsics [PR82480]

[Harald Anlauf]

Am 17.06.25 um 21:27 schrieb Harald Anlauf:

Am 17.06.25 um 19:44 schrieb Steve Kargl:

On Tue, Jun 17, 2025 at 12:05:34PM +0300, Janne Blomqvist wrote:

On Mon, Jun 16, 2025 at 9:41 PM Harald Anlauf  wrote:


Am 16.06.25 um 02:18 schrieb Steve Kargl:

Harald,

I did a quick glance at the patch and did not see anything that
jumped out as needing a change.  OK to commit.

Earlier today I came to the same conclusion that -1 on overflow is
probably the right thing to do.  Gfortran would need a way to
supply the value of ERANGE (on all supported targets) so a
user can write a test.  Yes, POSIX seems to define ERANGE as
34, but is that guaranteed on non-POSIX targets?


After thinking some more, I am struggling with these issues:

- ERANGE may not be defined, and the value of ERANGE may not
    be portable between targets.  How to handle that on the
    Fortran side?


intrinsics/getcwd.c uses ERANGE unconditionally, so it seems all
targets that GFortran supports do define ERANGE. As for the value,
POSIX does not specify numerical values that the errno constants
should take, though it seems 34 is more or less universal on Unix
systems. Also it seems windows uses 34. I'd say just use ERANGE if
appropriate, and if some target defines it differently then it's up to
the user to deal with it.


I think your last sentence is the problem with returning ERANGE.
If gfortran returns ERANGE, then gfortran should also have a facility
that a user can query for the actual value.  Walt mentions PXF in
another reply.  I have a copy of IEEE Std 1003.9-1992 (aka POSIX
FORTRAN77 Language Interface - Part 1).  I never found the time
to implement (parts of) it.

It should also be noted that on FreeBSD (and likely other UNIX-like
systems), one finds


% man 2 stat
...
RETURN VALUES
    Upon successful completion, the value 0 is returned; otherwise
    the value -1 is returned and the global variable errno is set
    to indicate the error.

Note, ERANGE is not documented as a value to which errno can be set.


- my initial proposal to return ERANGE has the potentially confusing
    effect that stat(3) may be successful, only the conversion to
    integer(kind=4) has an overflow in some component.  Is it then
    helpful to return ERANGE?  Or just return -1 for those components
    and document this new behavior?


Good point. I'd say set only the overflowing component to -1 and
document the behavior. It's possible the user is interested in some
component that doesn't overflow, and returning ERANGE would break
backwards compatibility?

As an aside, I note the documentation for [F,L,]STAT says the VALUES
array and STATUS should be INTEGER(4), which isn't correct is it? It
should be default kind INTEGER?


We probably want to make these GNU extensions generic.  I did this
for kill() and there are additional patches in

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

for umask() and unlink().




And don't forget that returning ERANGE needs adjustment of
testcases, where the only portable solution I have is the
specification of -fdefault-integer-8 ...


That might be a topic for a follow-up patch, but it would be useful to
be able to use the kind=8 version without having to redefine the
default integer size (which might need some additional headscratching
to deal with VALUES of INTEGER(kind=8) and STATUS of default kind..).
And then the documentation should recommend using that version in
order to avoid overflows.


Personally, I would like to see -fdefault-integer-8 deprecated, but
that ship sailed long ago.

Most intrinsic subprograms specified in the Fortran are generic.
Perhaps, gfortran's extension should follow suit.



OK, here is the conservative version along what Janne suggested,
with a documentation update: as a matter of fact, we currently
do support only default integer (which is kind=4, unless
-fdefault-integer-8 is specified) in the frontend,

We return -1 for elements where there would be overflow, but nothing
like ERANGE or so in STATUS.  If someone strongly feels that an error
should be returned, and a mechanism to make it possible to portably
detect this, this should become a separate PR.

With this version, no tweaking of existing tests is needed.

I intend to commit this within the next 24-48 hours as part 1
if it is ok.

Thanks,
Harald



Pushed as r16-1560-gce6abe54ff1548.

Re: [PATCH] fortran: Statically initialize length of SAVEd character arrays

[Thomas Koenig]

Hi Mikael,


  Regression-tested on x86_64-pc-linux-gnu.
  OK for master?


Just wondering... how does this relate to the recent fix of PR120483
by Andre? Is this also a regression?  If so, maybe a backport would be
in order.

Best regads

Thomas

[PATCH] fortran: Statically initialize length of SAVEd character arrays

[Mikael Morin]

From: Mikael Morin 

 Regression-tested on x86_64-pc-linux-gnu.
 OK for master?

 -- >8 --

gcc/fortran/ChangeLog:

* trans-array.cc (gfc_trans_deferred_array): Statically
initialize deferred length variable for SAVEd character arrays.

gcc/testsuite/ChangeLog:

* gfortran.dg/save_alloc_character_1.f90: New test.
---
 gcc/fortran/trans-array.cc| 12 --
 .../gfortran.dg/save_alloc_character_1.f90| 22 +++
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/save_alloc_character_1.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 960613167f7..3d274439895 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -12067,8 +12067,16 @@ gfc_trans_deferred_array (gfc_symbol * sym, 
gfc_wrapped_block * block)
   && !INTEGER_CST_P (sym->ts.u.cl->backend_decl))
 {
   if (sym->ts.deferred && !sym->ts.u.cl->length && !sym->attr.dummy)
-   gfc_add_modify (&init, sym->ts.u.cl->backend_decl,
-   build_zero_cst (TREE_TYPE 
(sym->ts.u.cl->backend_decl)));
+   {
+ tree len_expr = sym->ts.u.cl->backend_decl;
+ tree init_val = build_zero_cst (TREE_TYPE (len_expr));
+ if (VAR_P (len_expr)
+ && sym->attr.save
+ && !DECL_INITIAL (len_expr))
+   DECL_INITIAL (len_expr) = init_val;
+ else
+   gfc_add_modify (&init, len_expr, init_val);
+   }
   gfc_conv_string_length (sym->ts.u.cl, NULL, &init);
   gfc_trans_vla_type_sizes (sym, &init);
 
diff --git a/gcc/testsuite/gfortran.dg/save_alloc_character_1.f90 
b/gcc/testsuite/gfortran.dg/save_alloc_character_1.f90
new file mode 100644
index 000..ac16e77a01f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/save_alloc_character_1.f90
@@ -0,0 +1,22 @@
+! { dg-do run }
+!
+! Check that the length variable of SAVEd allocatable character arrays are
+! not initialized at function entry.
+
+program p
+  implicit none
+  call s(1)
+  call s(2)
+contains
+  subroutine s(i)
+integer, intent(in) :: i
+character(len=:), allocatable, save :: a(:)
+integer :: j
+if (i == 1) then
+  allocate(a, source= [ ('x' // achar(ichar('0') + j), j=1,7) ])
+else
+  if (len(a) /= 2) error stop 1
+  if (any(a /= ['x1','x2','x3','x4','x5','x6','x7'])) error stop 2
+end if
+  end subroutine s
+end program p
-- 
2.47.2