Re: [committed] Fortran: Add more documentation for mixed-language programming

2021-11-06 Thread Paul Richard Thomas via Fortran
Hi Sandra,

Looks good to me.

Thanks

Paul


On Fri, 5 Nov 2021 at 21:13, Sandra Loosemore 
wrote:

> I was recently pinged about PR35276.  It's an old issue, but a couple of
> the concerns raised there haven't been fixed yet, so I've checked in
> this patch to fill in the gaps.
>
> -Sandra
>


-- 
"If you can't explain it simply, you don't understand it well enough" -
Albert Einstein


Re: [PATCH] PR fortran/102715 - [12 Regression] ICE in gfc_simplify_transpose, at fortran/simplify.c:8184

2021-11-06 Thread Mikael Morin

Le 31/10/2021 à 22:35, Harald Anlauf via Fortran a écrit :

Dear Fortranners,

the fix for initialization of DT arrays caused an apparent regression for
cases where inconsistent ranks were used in such an initialization.
This caused either an ICE in subsequent uses of these arrays, or showed
up in valgrind as invalid reads, all of which seemed to be related to this
rank mismatch.

The cleanest solution seems to be to strictly reject rank mismatch earlier
than we used to, which helps error recovery.  I had to adjust one testcase
accordingly.

The place I inserted the check does not distinguish between explicit shape
and implied shape.  The Intel compiler does give a slightly different
error message for the implied shape case.  If anyone feels strongly about
this, I'm open to suggestions for better choices of handling this.

Regtested on x86_64-pc-linux-gnu.  OK for mainline / affected branches?



OK, thanks.

Mikael


Re: [PATCH] Fortran: Diagnose all operands/arguments with constraint violations

2021-11-06 Thread Mikael Morin

Le 05/11/2021 à 03:58, Sandra Loosemore a écrit :
This is an expanded version of the patch for PR 101337 that Bernhard 
sent out a few days ago with a request for me to finish it.  Bernhard 
did the part for operands and I added the pieces for procedure arguments 
and intrinsics, along with fixing up the test cases that were previously 
full of xfails and a few others that were now showing multiple 
diagnostics as a result of this change.


I suspect there might be other places where we are failing to check all 
subexpressions for errors, but this catches all the ones I wrote 
TS29113-related testcases for, at least.


OK to commit?



Ok. Thanks to both of you.

Mikael


Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers

2021-11-06 Thread Mikael Morin

Le 05/11/2021 à 19:46, Mikael Morin a écrit :
Don’t you get the same effect on the memory leaks if you keep just the 
following hunk?


 >>> @@ -1605,8 +1608,7 @@ generate_finalization_wrapper (gfc_symbol 
*derived, gfc_namespace *ns,

 >>> /* Set up the namespace.  */
 >>> sub_ns = gfc_get_namespace (ns, 0);
 >>> sub_ns->sibling = ns->contained;
 >>> -  if (!expr_null_wrapper)
 >>> -    ns->contained = sub_ns;
 >>> +  ns->contained = sub_ns;
 >>> sub_ns->resolved = 1;
 >>>
 >>> /* Set up the procedure symbol.  */



That’s probably not a good idea on second thought; it’s preferable to 
leak memory and not generate an empty finalization procedure.




Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers

2021-11-06 Thread Mikael Morin

Sorry, I hadn’t seen your message.

Le 05/11/2021 à 23:08, Bernhard Reutner-Fischer a écrit :

On Fri, 5 Nov 2021 19:46:16 +0100
Mikael Morin  wrote:


Le 29/10/2021 à 01:58, Bernhard Reutner-Fischer via Fortran a écrit :

On Wed, 27 Oct 2021 23:39:43 +0200
Bernhard Reutner-Fischer  wrote:
   

On Mon, 15 Oct 2018 10:23:06 +0200
Bernhard Reutner-Fischer  wrote:
  

If a finalization is not required we created a namespace containing
formal arguments for an internal interface definition but never used
any of these. So the whole sub_ns namespace was not wired up to the
program and consequently was never freed. The fix is to simply not
generate any finalization wrappers if we know that it will be unused.
Note that this reverts back to the original r190869
(8a96d64282ac534cb597f446f02ac5d0b13249cc) handling for this case
by reverting this specific part of r194075
(f1ee56b4be7cc3892e6ccc75d73033c129098e87) for PR fortran/37336.
  

I’m a bit concerned by the loss of the null_expr’s type interface.
I can’t convince myself that it’s either absolutely necessary or
completely useless.


It's a delicate spot, yes, but i do think they are completely useless.
If we do NOT need a finalization, the initializer can (and has to be
AFAIU) be a null_expr and AFAICS then does not need an interface.

Well, the null pointer itself doesn’t need a type, but I think it’s 
better if the pointer it’s assigned to has a type different from void*.

It will (hopefully) help the middle-end optimizers downstream.

I will see if I can manage to create a testcase where it makes a 
difference (don’t hold your breath, I don’t even have a bootstrapped 
compiler ready yet).




Don’t you get the same effect on the memory leaks if you keep just the
following hunk?


No, i don't think emitting the finalization-wrappers unconditionally is
correct. 

> (... lengthy explaination ...)
>
Agreed, it was a poor suggestion.



The rest of the changes (appart from class.c) are mostly OK with the nit
below and should be put in their own commit.

  >>> @@ -3826,10 +3828,8 @@ free_tb_tree (gfc_symtree *t)
  >>>
  >>> free_tb_tree (t->left);
  >>> free_tb_tree (t->right);
  >>> -
  >>> -  /* TODO: Free type-bound procedure structs themselves; probably
needs some
  >>> - sort of ref-counting mechanism.  */
  >>> free (t->n.tb);

Please keep a comment; it remains somehow valid but could be updated
maybe: gfc_typebound_proc’s u.generic field for example is nowhere freed
as far as I know.


Well that's a valid point, not sure where they are freed indeed.
Do you have a specific testcase in mind that leaks tbp's u.generic (or
specific for that matter) for me to look at?


Any testcase with generic typebound procedures, I guess.
typebound_generic_3.f03 for example seems like a good candidate.


I'm happy to change the comment to
TODO: Free type-bound procedure u.generic and u.specific fields
to reflect the current state. Ok?

I don’t think specific leaks because it’s one of gfc_namespace’s 
sym_root sub-nodes, and it’s freed with gfc_namespace.

OK without "and u.specific".


Re: gfortran.org

2021-11-06 Thread Bud Davis via Fortran
greetings to all,

my ownership of gfortran.org has lapsed and it is now owned by a
"speculator".  it appears to be available for $99, if someone is
interested.

gfortran.com is paid until June 25th, 2024.  this one is set to auto-renew,
whereas gfortran.org was not.

it redirects to http://gfortran.meteodat.ch , as it has for several years.

if someone wants the domain, I will transfer it.


Regards,
Bud Davis




On Fri, Nov 5, 2021 at 4:15 AM Manfred Schwarb  wrote:

> Hi,
>
> for quite some time, there was a forwarding from http://gfortran.org to my
> dumping ground http://gfortran.meteodat.ch.
>
> At the moment, at least for me, this domain points to some gaming blog.
> Does anybody know what is going on?
> There were some ideas what do with this domain at some point (Jerry,
> Paul?).
>
> Bud, are you still owner of this domain? Do you have some plans what to do
> with it?
>
> Cheers,
> Manfred
>
>
> PS: the domain gfortran.com still points to my site, but the above
> questions
> apply to this domain as well, of course.
>


Re: [PATCH] PR fortran/91497 -- Silence conversion warnings for MIN1 and MAX1

2021-11-06 Thread Manfred Schwarb via Fortran
Am 02.11.21 um 19:39 schrieb Thomas Koenig:
> On 02.11.21 15:22, Manfred Schwarb wrote:
>> Am 02.11.21 um 14:26 schrieb Thomas Koenig:
>>> Hi Manfred,
>>>
 In addition to the patches of Steve Kargl for PR 91497:
 The MIN1 and MAX1 intrinsics do explicit type conversions and should
 be silenced too for -Wconversion and -Wconversion-extra.

 Adjust testcase to only use *4 and *8 real types, provide a second
 testcase for *10 and *16 precisions.
>>> Two points:
>>>
>>> We should modify existing test cases only when necessary, because
>>> modification can impede a regression test.  It is better to create
>>> a new one.
>>>

I only changed the test case to use dg-require-effective-target and real(n) 
notation now.

Added a second case without real(10) and real(16), but for all targets, which
covers MIN1 and MAX1 too.


>>
>> Yes, but this was a quick-and-dirty test of mine, and I realized only 
>> afterwards
>> that Steve had used it as-is. The new testcase is more consistent and more 
>> complete.
>> Sandra got errors on targets without REAL(16) support and requested changes,
>> so I decided to split it.
>>
>> So you want me to "split" it in 3 parts?
>> - existing test as is, only for targets with REAL(16) support
>> - additional tests incl. complex intrinsics for targets with REAL(16) support
>> - additional tests incl. complex intrinsics for all targets, only single and 
>> double precision
>>
>> OTOH, it is perhaps not worth the trouble to do REAL(10) and REAL(16) tests, 
>> either
>> it warns or it does not.
>
> Anything that tests both complex and REAL(16) is fine by me.  I don't
> think you need to test the combination of COMPLEX(16), both
> codepaths have been seen by that time :-)
>
> Or you can split it three ways, like you wrote above.
>
>>> While we do recognize real*4 and real*8 and so on, these are
>>> non-standard extensions, and I would like to avoid to have these
>>> with new test cases.
>>>
>>> Instead of real*8, you can use real(8) or double precision.
>>>
>>
>> Well, double precision is deprecated AFAIK.
>
> Not in Fortran 2018.
>
> Best regards
>
>   Thomas
>

2021-11-06  Manfred Schwarb  

gcc/fortran/ChangeLog:

	PR fortran/91497
	* simplify.c (simplify_min_max): Disable conversion warnings for
	MIN1 and MAX1.

--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -5087,6 +5087,7 @@ min_max_choose (gfc_expr *arg, gfc_expr
 static gfc_expr *
 simplify_min_max (gfc_expr *expr, int sign)
 {
+  int tmp1, tmp2;
   gfc_actual_arglist *arg, *last, *extremum;
   gfc_expr *tmp, *ret;
   const char *fname;
@@ -5131,7 +5132,16 @@ simplify_min_max (gfc_expr *expr, int si
   if ((tmp->ts.type != BT_INTEGER || tmp->ts.kind != gfc_integer_4_kind)
   && (strcmp (fname, "min1") == 0 || strcmp (fname, "max1") == 0))
 {
+  /* Explicit conversion, turn off -Wconversion and -Wconversion-extra
+ warnings.  */
+  tmp1 = warn_conversion;
+  tmp2 = warn_conversion_extra;
+  warn_conversion = warn_conversion_extra = 0;
+
   ret = gfc_convert_constant (tmp, BT_INTEGER, gfc_integer_4_kind);
+
+  warn_conversion = tmp1;
+  warn_conversion_extra = tmp2;
 }
   else if ((tmp->ts.type != BT_REAL || tmp->ts.kind != gfc_real_4_kind)
 	   && (strcmp (fname, "amin0") == 0 || strcmp (fname, "amax0") == 0))
2021-11-06  Manfred Schwarb  

gcc/testsuite/ChangeLog:

	PR fortran/91497
	* gfortran.dg/pr91497.f90: Adjust test to use
	dg-require-effective-target directive.
	* gfortran.dg/pr91497_2.f90: New test to cover all targets.
	Cover MAX1 and MIN1 intrinsics.

--- a/gcc/testsuite/gfortran.dg/pr91497.f90
+++ b/gcc/testsuite/gfortran.dg/pr91497.f90
@@ -1,4 +1,6 @@
-! { dg-do compile { target { i?86-*-* x86_64-*-* } } }
+! { dg-do compile }
+! { dg-require-effective-target fortran_real_10 }
+! { dg-require-effective-target fortran_real_16 }
 ! { dg-options "-Wall" }
 ! Code contributed by Manfred Schwarb 
 ! PR fortran/91497
@@ -8,13 +10,13 @@
 !
 program foo

-  real*4 a,aa
-  real*8 b,bb
-  real*10 c,cc
-  real*16 d
-  integer*2 e,ee
-  integer*4 f,ff
-  integer*8 g,gg
+  real(4) a,aa
+  real(8) b,bb
+  real(10) c,cc
+  real(16) d
+  integer(2) e,ee
+  integer(4) f,ff
+  integer(8) g,gg
   PARAMETER(a=3.1415927_4)
   PARAMETER(b=3.1415927_8)
   PARAMETER(c=3.1415927_10)
@@ -36,11 +38,10 @@ program foo
   aa=CEILING(b)
   aa=CEILING(c)
   aa=CEILING(d)
-  !---unknown but documented type conversions:
+  !---DEC specific type conversions (-fdec):
   !!aa=FLOATI(e)
   !!aa=FLOATJ(f)
   !!aa=FLOATK(g)
-  !---documentation is wrong for sngl:
   aa=SNGL(c)
   aa=SNGL(d)
   bb=REAL(c, kind=8)
@@ -98,7 +99,7 @@ program foo
   ff=IFIX(a)
   ff=IDINT(b)
   ff=IDNINT(b)
-  !---LONG not allowed anymore in gfortran 10 (?):
+  !---LONG support got removed:
   !!ff=LONG(a)
   !!ff=LONG(b)
   !!ff=LONG(c)
--- /dev/null

Re: [PATCH] PR fortran/91497 -- Silence conversion warnings for MIN1 and MAX1

2021-11-06 Thread Thomas Koenig via Fortran



Hi Manfred,

looks good to me.

Thanks for the patch!

Best regards

Thomas


Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]

2021-11-06 Thread Harald Anlauf via Fortran

Hi Bernhard,

I cannot comment on the gcc/ parts, but

Am 05.11.21 um 22:17 schrieb Bernhard Reutner-Fischer via Gcc-patches:

From: Bernhard Reutner-Fischer 

gcc/fortran/ChangeLog:

PR fortran/68800
* cpp.h (gfc_cpp_free_cpp_dirs): New declaration.
* cpp.c (gfc_cpp_free_cpp_dirs): New definition.
(gfc_cpp_add_include_path, gfc_cpp_add_include_path_after): Add
comment.
* f95-lang.c (gfc_finish): Call gfc_cpp_free_cpp_dirs.
* scanner.c (add_path_to_list): Allocate unzeroed memory.
(gfc_add_include_path): Add comment and fix formatting.

---
Bootstrapped and regtested without regressions on x86_64-unknown-linux.
Ok for trunk?

Note: in add_path_to_list i changed dir->path = XCNEWVEC to XNEWVEC
since strcpy and strcat add a terminating null byte for us anyway.
Fixes:

-== 68 bytes in 1 blocks are still reachable in loss record 228 of 371
-==at : malloc (vg_replace_malloc.c:380)
-==by : xmalloc (xmalloc.c:149)
-==by : xstrdup (xstrdup.c:34)
-==by : gfc_add_include_path(char const*, bool, bool, bool, bool) (scanner.c
:428)
-==by : gfc_handle_option(unsigned long, char const*, long, int, unsigned in
t, cl_option_handlers const*) (options.c:702)
-==by : handle_option(gcc_options*, gcc_options*, cl_decoded_option const*,
unsigned int, int, unsigned int, cl_option_handlers const*, bool, diagnostic_con
text*) (opts-common.c:1181)
-==by : read_cmdline_option(gcc_options*, gcc_options*, cl_decoded_option*,
unsigned int, unsigned int, cl_option_handlers const*, diagnostic_context*) (opt
s-common.c:1431)
-==by : read_cmdline_options (opts-global.c:237)
-==by : decode_options(gcc_options*, gcc_options*, cl_decoded_option*, unsig
ned int, unsigned int, diagnostic_context*, void (*)()) (opts-global.c:319)
-==by : toplev::main(int, char**) (toplev.c:2273)
-==by : main (main.c:39)
---
  gcc/fortran/cpp.c  | 13 +++--
  gcc/fortran/cpp.h  |  1 +
  gcc/fortran/f95-lang.c |  2 +-
  gcc/fortran/scanner.c  |  7 ---
  4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index e86386c8b17..04fe8fe460b 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -728,12 +728,20 @@ gfc_cpp_done (void)
cpp_clear_file_cache (cpp_in);
  }


why do you introduce a wrapper for something outside of fortran
that is used only once,


-/* PATH must be malloc-ed and NULL-terminated.  */
+/* Free all cpp include dirs.  */
+void
+gfc_cpp_free_cpp_dirs (void)
+{
+  free_cpp_dirs ();
+}
+
+/* PATH must be NULL-terminated.  */
  void
  gfc_cpp_add_include_path (char *path, bool user_supplied)
  {
/* CHAIN sets cpp_dir->sysp which differs from 0 if PATH is a system
- include path. Fortran does not define any system include paths.  */
+ include path. Fortran does not define any system include paths.
+ incpath.c manages the incoming, xstrdup()ed path.  */
int cxx_aware = 0;

add_path (path, INC_BRACKET, cxx_aware, user_supplied);
@@ -742,6 +750,7 @@ gfc_cpp_add_include_path (char *path, bool user_supplied)
  void
  gfc_cpp_add_include_path_after (char *path, bool user_supplied)
  {
+  /* incpath.c manages the incoming, xstrdup()ed path.  */
int cxx_aware = 0;
add_path (path, INC_AFTER, cxx_aware, user_supplied);
  }
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index 44644a2a333..963b9a9c89e 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
  bool gfc_cpp_preprocess (const char *source_file);

  void gfc_cpp_done (void);
+void gfc_cpp_free_cpp_dirs (void);

  void gfc_cpp_add_include_path (char *path, bool user_supplied);
  void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 58dcaf01d75..ec4c2cf01d9 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -275,7 +275,7 @@ gfc_finish (void)
gfc_cpp_done ();
gfc_done_1 ();
gfc_release_include_path ();
-  return;


namely here?


+  gfc_cpp_free_cpp_dirs ();
  }


Why not call free_cpp_dirs () here directly, omit all unnecessary
stuff, and maybe only add a brief comment here?


  /* These functions and variables deal with binding contours.  We only
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 69b81ab97f8..268d1e3e7fb 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -370,7 +370,7 @@ add_path_to_list (gfc_directorylist **list, const char 
*path,
char *q;
size_t len;
int i;
-
+
p = path;
while (*p == ' ' || *p == '\t')  /* someone might do "-I include" */
  if (*p++ == '\0')
@@ -409,7 +409,7 @@ add_path_to_list (gfc_directorylist **list, const char 
*path,
  *list = dir;
dir->use_for_modules = use_for_modules;
dir->warn = warn; > -  dir->path = XCNEWVEC (char, strlen (p) + 2);
+  dir->path = XNEWVEC (char, strlen (p) + 2);
strcpy (dir->path, p);
strcat (

Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers

2021-11-06 Thread Bernhard Reutner-Fischer via Fortran
On Sat, 6 Nov 2021 13:04:07 +0100
Mikael Morin  wrote:

> Le 05/11/2021 à 23:08, Bernhard Reutner-Fischer a écrit :
> > On Fri, 5 Nov 2021 19:46:16 +0100
> > Mikael Morin  wrote:
> >   
> >> Le 29/10/2021 à 01:58, Bernhard Reutner-Fischer via Fortran a écrit :  
> >>> On Wed, 27 Oct 2021 23:39:43 +0200
> >>> Bernhard Reutner-Fischer  wrote:
> >>>  
>  On Mon, 15 Oct 2018 10:23:06 +0200
>  Bernhard Reutner-Fischer  wrote:
>  
> > If a finalization is not required we created a namespace containing
> > formal arguments for an internal interface definition but never used
> > any of these. So the whole sub_ns namespace was not wired up to the
> > program and consequently was never freed. The fix is to simply not
> > generate any finalization wrappers if we know that it will be unused.
> > Note that this reverts back to the original r190869
> > (8a96d64282ac534cb597f446f02ac5d0b13249cc) handling for this case
> > by reverting this specific part of r194075
> > (f1ee56b4be7cc3892e6ccc75d73033c129098e87) for PR fortran/37336.
> > 
> >> I’m a bit concerned by the loss of the null_expr’s type interface.
> >> I can’t convince myself that it’s either absolutely necessary or
> >> completely useless.  
> > 
> > It's a delicate spot, yes, but i do think they are completely useless.
> > If we do NOT need a finalization, the initializer can (and has to be
> > AFAIU) be a null_expr and AFAICS then does not need an interface.
> >   
> Well, the null pointer itself doesn’t need a type, but I think it’s 
> better if the pointer it’s assigned to has a type different from void*.
> It will (hopefully) help the middle-end optimizers downstream.

I would not expect this to help all that much or at all TBH.

So i compiled
for i in $(grep -li final $(grep -L dg-error 
/scratch/src/gcc-12.mine/gcc/testsuite/gfortran.dg/*.f*)); do gfortran -O2 
-fcoarray=single -c $i -g -g3 -ggdb3 -fdump-tree-original 
-fdump-tree-optimized;done
and diffed all .original and .optimized dumps against pristine trunk
and they are identical.

I inspected and ran the binary from finalize_14 and there is no change
in the leaks compared to pristine trunk. The 3 shape_w in p leak as
they used to. I do remember that finalize_14 was a good testcase, in
sum i glared at it for quite some time ;)
> 
> I will see if I can manage to create a testcase where it makes a 
> difference (don’t hold your breath, I don’t even have a bootstrapped 
> compiler ready yet).
> 
That'd be great, TIA!
[]

btw.. Just because it's vagely related.
I think f8add009ce300f24b75e9c2e2cc5dd944a020c28 for
PR fortran/88009 (ICE in find_intrinsic_vtab, at fortran/class.c:2761)
is incomplete in that i think all the internal class helpers should be
flagged artificial. All these symbols built in gfc_build_class_symbol,
generate_finalization_wrapper, gfc_find_derived_vtab etc.
Looking at the history it seems the artificial bit often was forgotten.
And most importantly i think it is not correct to ignore artificial in
gfc_check_conflict!

I'm attaching my notes on this to illustrate what i mean.
Not a patch, even if it regtests cleanly..

The hunk in gfc_match_derived_decl() plugs another leak by first
checking if the max extension level is reached before adding the
component. Maybe i should split that hunk out.
Similar to the removal of *head in gfc_match_derived_decl, there's
another spot in gfc_match_decl_type_spec which should get rid of the
*head and just wire the interface up as usual. Just cosmetics.

Several tests do exercise this code: alloc_comp_class_1.f90,
class_19.f03 and 62, unlimited_polymorphic_8.f90 and others.

> >> The rest of the changes (appart from class.c) are mostly OK with the nit
> >> below and should be put in their own commit.
> >>  
> >>   >>> @@ -3826,10 +3828,8 @@ free_tb_tree (gfc_symtree *t)
> >>   >>>
> >>   >>> free_tb_tree (t->left);
> >>   >>> free_tb_tree (t->right);
> >>   >>> -
> >>   >>> -  /* TODO: Free type-bound procedure structs themselves; probably  
> >> needs some  
> >>   >>> - sort of ref-counting mechanism.  */
> >>   >>> free (t->n.tb);  
> >>
> >> Please keep a comment; it remains somehow valid but could be updated
> >> maybe: gfc_typebound_proc’s u.generic field for example is nowhere freed
> >> as far as I know.  
> > 
> > Well that's a valid point, not sure where they are freed indeed.
> > Do you have a specific testcase in mind that leaks tbp's u.generic (or
> > specific for that matter) for me to look at?
> >   
> Any testcase with generic typebound procedures, I guess.
> typebound_generic_3.f03 for example seems like a good candidate.

I'll have a look at these later, thanks for the pointer.
> 
> > I'm happy to change the comment to
> > TODO: Free type-bound procedure u.generic and u.specific fields
> > to reflect the current state. Ok?
> >  
> I don’t think specific leaks because it’s one of gfc_namespace’s 
> sym_root sub-nodes, and it’s freed with gfc_namespace

Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]

2021-11-06 Thread Bernhard Reutner-Fischer via Fortran
On Sat, 6 Nov 2021 20:22:53 +0100
Harald Anlauf  wrote:

> Hi Bernhard,
> 
> I cannot comment on the gcc/ parts, but
> 

> > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
> > index e86386c8b17..04fe8fe460b 100644
> > --- a/gcc/fortran/cpp.c
> > +++ b/gcc/fortran/cpp.c
> > @@ -728,12 +728,20 @@ gfc_cpp_done (void)
> > cpp_clear_file_cache (cpp_in);
> >   }  
> 
> why do you introduce a wrapper for something outside of fortran
> that is used only once,
> 
> > -/* PATH must be malloc-ed and NULL-terminated.  */
> > +/* Free all cpp include dirs.  */
> > +void
> > +gfc_cpp_free_cpp_dirs (void)
> > +{
> > +  free_cpp_dirs ();
> > +}

> > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
> > index 44644a2a333..963b9a9c89e 100644
> > --- a/gcc/fortran/cpp.h
> > +++ b/gcc/fortran/cpp.h
> > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
> >   bool gfc_cpp_preprocess (const char *source_file);
> >
> >   void gfc_cpp_done (void);
> > +void gfc_cpp_free_cpp_dirs (void);
> >
> >   void gfc_cpp_add_include_path (char *path, bool user_supplied);
> >   void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
> > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
> > index 58dcaf01d75..ec4c2cf01d9 100644
> > --- a/gcc/fortran/f95-lang.c
> > +++ b/gcc/fortran/f95-lang.c
> > @@ -275,7 +275,7 @@ gfc_finish (void)
> > gfc_cpp_done ();
> > gfc_done_1 ();
> > gfc_release_include_path ();
> > -  return;  
> 
> namely here?
> 
> > +  gfc_cpp_free_cpp_dirs ();
> >   }  
> 
> Why not call free_cpp_dirs () here directly, omit all unnecessary
> stuff, and maybe only add a brief comment here?

cpp.c includes incpath.h, f95-lang.c does not and should not.
So the cleanest thing is to keep the cpp handling in cpp.[ch] and have
the language frontend call into it's cpp bits.

It would be rather rogue to
extern void free_cpp_dirs (void);
in f95-lang.c and directly call it in gfc_finish, i'd say?

thanks,