[Patch, fortran] PR117434 - [F08] gfortran rejects actual argument corresponding to procedure pointer dummy argument
Hi All,
There is not much to say about the attached patch other than it is minimal
:-) The testcases are probably a bit more than is strictly needed since the
interface tests (proc_ptr_55.f90) are already tested elsewhere. However, it
is as well to check in this context.
OK for mainline and 14-branch after a week or two?
The issue with the executable stack on some platforms should have its own
PR to ensure that it has the required visibility. I can make
proc_ptr_64.f90 compile-only until it is fixed.
Regards
Paul
Change.Logs
Description: Binary data
diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index 69519fe3168..61c506bfdb5 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -3513,12 +3513,17 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
skip_size_check:
- /* Satisfy F03:12.4.1.3 by ensuring that a procedure pointer actual
- argument is provided for a procedure pointer formal argument. */
+ /* Satisfy either: F03:12.4.1.3 by ensuring that a procedure pointer
+ actual argument is provided for a procedure pointer formal argument;
+ or: F08:12.5.2.9 (F18:15.5.2.10) by ensuring that the effective
+ argument shall be an external, internal, module, or dummy procedure.
+ The interfaces are checked elsewhere. */
if (f->sym->attr.proc_pointer
&& !((a->expr->expr_type == EXPR_VARIABLE
&& (a->expr->symtree->n.sym->attr.proc_pointer
|| gfc_is_proc_ptr_comp (a->expr)))
+ || (a->expr->ts.type == BT_PROCEDURE
+ && f->sym->ts.interface)
|| (a->expr->expr_type == EXPR_FUNCTION
&& is_procptr_result (a->expr
{
diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_54.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_54.f90
new file mode 100644
index 000..348b73b9dad
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/proc_ptr_54.f90
@@ -0,0 +1,95 @@
+! { dg-do run }
+!
+! Test the fix for pr117434, in which the F2008 addition of being permitted to
+! pass an external, internal or module procedure to a dummy procedure pointer
+! gave the error "Expected a procedure pointer for argument ‘’ at (1).
+!
+! This testcase checks for correct results.
+!
+! Contributed by Damian Rouson
+!
+module julienne_test_description_m
+ implicit none
+
+ abstract interface
+logical function test_function_i(arg)
+ integer, intent(in) :: arg
+end function
+ end interface
+
+ type test_description_t
+procedure(test_function_i), pointer, nopass :: test_function_
+ end type
+
+
+contains
+
+ type(test_description_t) function new_test_description(test_function)
+procedure(test_function_i), intent(in), pointer :: test_function
+new_test_description%test_function_ => test_function
+ end function
+
+end module
+
+module test_mod
+
+contains
+
+ logical function mod_test(arg)
+integer, intent(in) :: arg
+if (arg == 1) then
+ mod_test = .true.
+else
+ mod_test = .false.
+endif
+ end function
+
+end
+
+logical function ext_test(arg)
+ integer, intent(in) :: arg
+ if (arg == 2) then
+ext_test = .true.
+ else
+ext_test = .false.
+ endif
+end function
+
+ use julienne_test_description_m
+ use test_mod
+ implicit none
+ type(test_description_t) test_description
+
+ interface
+logical function ext_test(arg)
+ integer, intent(in) :: arg
+end function
+ end interface
+
+ test_description = new_test_description(test)
+ if (test_description%test_function_(1) &
+ .or. test_description%test_function_(2) &
+ .or. .not.test_description%test_function_(3)) stop 1
+
+ test_description = new_test_description(mod_test)
+ if (test_description%test_function_(2) &
+ .or. test_description%test_function_(3) &
+ .or. .not.test_description%test_function_(1)) stop 2
+
+ test_description = new_test_description(ext_test)
+ if (test_description%test_function_(1) &
+ .or. test_description%test_function_(3) &
+ .or. .not.test_description%test_function_(2)) stop 3
+
+contains
+
+ logical function test(arg)
+integer, intent(in) :: arg
+if (arg == 3) then
+ test = .true.
+else
+ test = .false.
+endif
+ end function
+
+end
diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_55.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_55.f90
new file mode 100644
index 000..7028634b54e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/proc_ptr_55.f90
@@ -0,0 +1,87 @@
+! { dg-do compile }
+!
+! Test the fix for pr117434, in which the F2008 addition of being permitted to
+! pass an external, internal or module procedure to a dummy procedure pointer
+! gave the error "Expected a procedure pointer for argument ‘’ at (1).
+!
+! This testcase tests that interface checking is OK in this situation.
+!
+! Contributed by Damian Rouson
+!
+module julienne_test_description_m
+ implicit none
+
+ abstract interface
+logical function test_function_i(arg)
+ integer, intent(in) :: arg
+end function
+ e
Re: [Patch, fortran] PR117434 - [F08] gfortran rejects actual argument corresponding to procedure pointer dummy argument
Hi Paul,
Am 05.11.24 um 16:24 schrieb Paul Richard Thomas:
Hi All,
There is not much to say about the attached patch other than it is minimal
:-) The testcases are probably a bit more than is strictly needed since the
interface tests (proc_ptr_55.f90) are already tested elsewhere. However, it
is as well to check in this context.
the fix is fine, but testcase gfortran.dg/proc_ptr_56.f90 should be
! { dg-do compile }
instead of
! { dg-do run }
You'll notice that you get
# of unresolved testcases 6
when running it under the testsuite harness, because
PASS: gfortran.dg/proc_ptr_56.f90 -O0 (test for excess errors)
UNRESOLVED: gfortran.dg/proc_ptr_56.f90 -O0 compilation failed to
produce executable
the first line is expected, but not the second.
OK for mainline and 14-branch after a week or two?
OK with the testcase fixed.
The issue with the executable stack on some platforms should have its own
PR to ensure that it has the required visibility. I can make
proc_ptr_64.f90 compile-only until it is fixed.
I do not have any preference here. If any of the testers are affected,
then having it compile-only might be the right choice.
Regards
Paul
Thanks for the patch!
Harald
Re: [Patch, fortran] PR117434 - [F08] gfortran rejects actual argument corresponding to procedure pointer dummy argument
Hi Folks.
> On 5 Nov 2024, at 19:23, Harald Anlauf wrote:
>
> Hi Paul,
>
> Am 05.11.24 um 16:24 schrieb Paul Richard Thomas:
>> Hi All,
>>
>> There is not much to say about the attached patch other than it is minimal
>> :-) The testcases are probably a bit more than is strictly needed since the
>> interface tests (proc_ptr_55.f90) are already tested elsewhere. However, it
>> is as well to check in this context.
>
> the fix is fine, but testcase gfortran.dg/proc_ptr_56.f90 should be
>
> ! { dg-do compile }
>
> instead of
>
> ! { dg-do run }
>
> You'll notice that you get
>
> # of unresolved testcases 6
>
> when running it under the testsuite harness, because
>
> PASS: gfortran.dg/proc_ptr_56.f90 -O0 (test for excess errors)
> UNRESOLVED: gfortran.dg/proc_ptr_56.f90 -O0 compilation failed to
> produce executable
You can most likely add target-specific additional options like “-z execstack”
to
suppress the warning on those platforms affected (and therefore get a test at
O0).
Note that, in the test case in PR117434 the code was completely elided
for O > 0 and therefore that’s also a consideration as to whether execute
testing is useful (at least someone should verify the revised testcase actually
does something)
Iain
>
> the first line is expected, but not the second.
>
>> OK for mainline and 14-branch after a week or two?
>
> OK with the testcase fixed.
>
>> The issue with the executable stack on some platforms should have its own
>> PR to ensure that it has the required visibility. I can make
>> proc_ptr_64.f90 compile-only until it is fixed.
>
> I do not have any preference here. If any of the testers are affected,
> then having it compile-only might be the right choice.
>
>> Regards
>>
>> Paul
>>
>
> Thanks for the patch!
>
> Harald
>
[pushed: r15-4969] fortran: dynamically allocate error_buffer [PR117442]
PR fortran/117442 reports a crash on exit of f951 when configured
with --enable-gather-detailed-mem-stats.
The crash happens if any diagnostics were ever buffered into
error_buffer. The root cause is that error_buffer is statically
allocated and thus has a non-trivial destructor called at exit.
If error_buffer's diagnostic_buffer ever buffered anything, then
a diagnostic_per_format_buffer will have been created for the
buffer per-output-sink, and the destructors for these call
into the mem-stats subsystem, which has already beeen cleaned up.
The simplest fix is to allocate error_buffer on the heap, rather
that statically, which fixes the crash.
There's a comment about error_buffer:
/* pp_error_buffer is statically allocated. This simplifies memory
management when using gfc_push/pop_error. */
added by Manu in r6-1748-g5862c189c2c3c2 while fixing PR fortran/66528.
The comment appears to be out of date. I've tested maxerrors.f90 under
valgrind, and it's clean with the patch.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r15-4969-g8c4184682d1cdf.
gcc/fortran/ChangeLog:
PR fortran/117442
* error.cc (error_buffer): Convert to a pointer so it can be
heap-allocated.
(gfc_error_now): Update for error_buffer being heap-allocated.
(gfc_clear_error): Likewise.
(gfc_error_flag_test): Likewise.
(gfc_error_check): Likewise.
(gfc_push_error): Likewise.
(gfc_pop_error): Likewise.
(gfc_diagnostics_init): Allocate error_buffer on the heap, rather
than statically.
(gfc_diagnostics_finish): Delete error_buffer.
Signed-off-by: David Malcolm
---
gcc/fortran/error.cc | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/gcc/fortran/error.cc b/gcc/fortran/error.cc
index 050a8f286efd..1445ebcbecd8 100644
--- a/gcc/fortran/error.cc
+++ b/gcc/fortran/error.cc
@@ -43,7 +43,7 @@ static bool warnings_not_errors = false;
/* True if the error/warnings should be buffered. */
static bool buffered_p;
-static gfc_error_buffer error_buffer;
+static gfc_error_buffer *error_buffer;
static diagnostic_buffer *pp_error_buffer, *pp_warning_buffer;
gfc_error_buffer::gfc_error_buffer ()
@@ -707,7 +707,7 @@ gfc_error_now (const char *gmsgid, ...)
diagnostic_info diagnostic;
rich_location rich_loc (line_table, UNKNOWN_LOCATION);
- error_buffer.flag = true;
+ error_buffer->flag = true;
va_start (argp, gmsgid);
diagnostic_set_info (&diagnostic, gmsgid, &argp, &rich_loc, DK_ERROR);
@@ -842,7 +842,7 @@ gfc_internal_error (const char *gmsgid, ...)
void
gfc_clear_error (void)
{
- error_buffer.flag = false;
+ error_buffer->flag = false;
warnings_not_errors = false;
gfc_clear_diagnostic_buffer (pp_error_buffer);
}
@@ -853,7 +853,7 @@ gfc_clear_error (void)
bool
gfc_error_flag_test (void)
{
- return (error_buffer.flag
+ return (error_buffer->flag
|| !pp_error_buffer->empty_p ());
}
@@ -864,10 +864,10 @@ gfc_error_flag_test (void)
bool
gfc_error_check (void)
{
- if (error_buffer.flag
+ if (error_buffer->flag
|| ! pp_error_buffer->empty_p ())
{
- error_buffer.flag = false;
+ error_buffer->flag = false;
global_dc->flush_diagnostic_buffer (*pp_error_buffer);
return true;
}
@@ -903,7 +903,7 @@ gfc_move_error_buffer_from_to (gfc_error_buffer *
buffer_from,
void
gfc_push_error (gfc_error_buffer *err)
{
- gfc_move_error_buffer_from_to (&error_buffer, err);
+ gfc_move_error_buffer_from_to (error_buffer, err);
}
@@ -912,7 +912,7 @@ gfc_push_error (gfc_error_buffer *err)
void
gfc_pop_error (gfc_error_buffer *err)
{
- gfc_move_error_buffer_from_to (err, &error_buffer);
+ gfc_move_error_buffer_from_to (err, error_buffer);
}
@@ -955,9 +955,8 @@ gfc_diagnostics_init (void)
global_dc->m_source_printing.caret_chars[0] = '1';
global_dc->m_source_printing.caret_chars[1] = '2';
pp_warning_buffer = new diagnostic_buffer (*global_dc);
- /* pp_error_buffer is statically allocated. This simplifies memory
- management when using gfc_push/pop_error. */
- pp_error_buffer = &(error_buffer.buffer);
+ error_buffer = new gfc_error_buffer ();
+ pp_error_buffer = &(error_buffer->buffer);
}
void
@@ -970,4 +969,7 @@ gfc_diagnostics_finish (void)
diagnostic_text_finalizer (global_dc) = gfc_diagnostic_text_finalizer;
global_dc->m_source_printing.caret_chars[0] = '^';
global_dc->m_source_printing.caret_chars[1] = '^';
+ delete error_buffer;
+ error_buffer = nullptr;
+ pp_error_buffer = nullptr;
}
--
2.26.3
Re: [Patch, fortran] PR117434 - [F08] gfortran rejects actual argument corresponding to procedure pointer dummy argument
Hi Harald,
Yes indeed about proc_ptr_56.f90 :-( That was a slip that occurred in the
preparation of the patch for the list. I will indeed make proc_ptr_54.f90
compile-only for the time being. The latter was not elided from my platform
for any level of optimization for the simple reason that system updates
have been failing for a while because I have run out of disk space. I have
been putting off doing anything about it until I can put up with the
disruption.
Thanks for the review
Paul
On Tue, 5 Nov 2024 at 19:23, Harald Anlauf wrote:
> Hi Paul,
>
> Am 05.11.24 um 16:24 schrieb Paul Richard Thomas:
> > Hi All,
> >
> > There is not much to say about the attached patch other than it is
> minimal
> > :-) The testcases are probably a bit more than is strictly needed since
> the
> > interface tests (proc_ptr_55.f90) are already tested elsewhere. However,
> it
> > is as well to check in this context.
>
> the fix is fine, but testcase gfortran.dg/proc_ptr_56.f90 should be
>
> ! { dg-do compile }
>
> instead of
>
> ! { dg-do run }
>
> You'll notice that you get
>
> # of unresolved testcases 6
>
> when running it under the testsuite harness, because
>
> PASS: gfortran.dg/proc_ptr_56.f90 -O0 (test for excess errors)
> UNRESOLVED: gfortran.dg/proc_ptr_56.f90 -O0 compilation failed to
> produce executable
>
> the first line is expected, but not the second.
>
> > OK for mainline and 14-branch after a week or two?
>
> OK with the testcase fixed.
>
> > The issue with the executable stack on some platforms should have its own
> > PR to ensure that it has the required visibility. I can make
> > proc_ptr_64.f90 compile-only until it is fixed.
>
> I do not have any preference here. If any of the testers are affected,
> then having it compile-only might be the right choice.
>
> > Regards
> >
> > Paul
> >
>
> Thanks for the patch!
>
> Harald
>
>
