[PATCH] OpenMP/Fortran: Add missing pop_state in parse_omp_dispatch
When the ST_NONE case is taken, the function returns immediately. Not calling
pop_state causes a dangling pointer.
gcc/fortran/ChangeLog:
* parse.cc (parse_omp_dispatch): Add missing pop_state.
---
gcc/fortran/parse.cc | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 00cd23d7729..5094d9d3ead 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -6375,7 +6375,10 @@ parse_omp_dispatch (void)
st = next_statement ();
if (st == ST_NONE)
-return st;
+{
+ pop_state ();
+ return st;
+}
if (st == ST_CALL || st == ST_ASSIGNMENT)
accept_statement (st);
else
--
2.47.2
Re: [PATCH] OpenMP/Fortran: Add missing pop_state in parse_omp_dispatch
Pushed to master as obvious. This should fix PR118714.
On 31/01/2025 11:46, Paul-Antoine Arras wrote:
When the ST_NONE case is taken, the function returns immediately. Not calling
pop_state causes a dangling pointer.
gcc/fortran/ChangeLog:
* parse.cc (parse_omp_dispatch): Add missing pop_state.
---
gcc/fortran/parse.cc | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 00cd23d7729..5094d9d3ead 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -6375,7 +6375,10 @@ parse_omp_dispatch (void)
st = next_statement ();
if (st == ST_NONE)
-return st;
+{
+ pop_state ();
+ return st;
+}
if (st == ST_CALL || st == ST_ASSIGNMENT)
accept_statement (st);
else
--
PA
Re: [PATCH] Fortran: host association issue with symbol in COMMON block [PR108454]
On 1/30/25 1:44 PM, Harald Anlauf wrote: Dear all, analyzing the the PR (by Gerhard) turned out to two slightly related issues. The first one, where a variable in a COMMON block is falsely resolved to a derived type declared in the host, leads to a false freeing of the symbol, resulting in memory corruption and ICE. If we already know that the symbol is in a common block, we may just skip that interface search. The other issue is a resolution issue, where the derived type declared in the host is used in a variable declaration in the procedure (as type or class), and a variable of the same name as the derived type is used in a common block but later resolves to a basic type, without a proper detection of the conflict. But as this issue is found to be independent of the presence of a COMMON block, I have opened a separate issue (pr118709) for it. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Looks good to go Harald. As always, thanks for the fix. Jerry
Re: [PATCH] Fortran: host association issue with symbol in COMMON block [PR108454]
Am 31.01.25 um 18:37 schrieb Jerry D: On 1/30/25 1:44 PM, Harald Anlauf wrote: Dear all, analyzing the the PR (by Gerhard) turned out to two slightly related issues. The first one, where a variable in a COMMON block is falsely resolved to a derived type declared in the host, leads to a false freeing of the symbol, resulting in memory corruption and ICE. If we already know that the symbol is in a common block, we may just skip that interface search. The other issue is a resolution issue, where the derived type declared in the host is used in a variable declaration in the procedure (as type or class), and a variable of the same name as the derived type is used in a common block but later resolves to a basic type, without a proper detection of the conflict. But as this issue is found to be independent of the presence of a COMMON block, I have opened a separate issue (pr118709) for it. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Looks good to go Harald. As always, thanks for the fix. Jerry Thanks, Jerry! This is now r15-7308-gd6418fe22684f9 .
[PATCH] OpenMP: Improve Fortran metadirective diagnostics [PR107067]
The Fortran front end was giving an ICE instead of a user-friendly
diagnostic when variants of a metadirective variant had different
statement associations. The particular test case reported in the issue
also involved invalid placement of the "omp end metadirective" which
was not being diagnosed either.
gcc/fortran/ChangeLog
PR middle-end/107067
* parse.cc (parse_omp_do): Diagnose missing "OMP END METADIRECTIVE"
after loop.
(parse_omp_structured_block): Likewise for strictly structured block.
(parse_omp_metadirective_body): Use better test for variants ending
at different places. Issue a user diagnostic at the end if any
were inconsistent, instead of calling gcc_assert.
gcc/testsuite/ChangeLog
PR middle-end/107067
* gfortran.dg/gomp/metadirective-11.f90: Remove the dg-ice, update
for current behavior, and add more tests to exercise the new error
code.
---
gcc/fortran/parse.cc | 53 ---
.../gfortran.dg/gomp/metadirective-11.f90 | 67 +--
2 files changed, 105 insertions(+), 15 deletions(-)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 00cd23d7729..933cfe8c58f 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -5804,9 +5804,20 @@ do_end:
/* If handling a metadirective variant, treat 'omp end metadirective'
as the expected end statement for the current construct. */
- if (st == ST_OMP_END_METADIRECTIVE
- && gfc_state_stack->state == COMP_OMP_BEGIN_METADIRECTIVE)
-st = omp_end_st;
+ if (gfc_state_stack->state == COMP_OMP_BEGIN_METADIRECTIVE)
+{
+ if (st == ST_OMP_END_METADIRECTIVE)
+ st = omp_end_st;
+ else
+ {
+ /* We have found some extra statements between the loop
+and the "end metadirective" which is required in a
+"begin metadirective" construct, or perhaps the
+"end metadirective" is missing entirely. */
+ gfc_error_now ("Expected OMP END METADIRECTIVE at %C");
+ return st;
+ }
+}
if (st == omp_end_st)
{
@@ -6294,6 +6305,14 @@ parse_omp_structured_block (gfc_statement omp_st, bool
workshare_stmts_only)
accept_statement (st);
st = next_statement ();
}
+ else if (omp_end_st == ST_OMP_END_METADIRECTIVE)
+ {
+ /* We have found some extra statements between the END BLOCK
+and the "end metadirective" which is required in a
+"begin metadirective" construct, or perhaps the
+"end metadirective" is missing entirely. */
+ gfc_error_now ("Expected OMP END METADIRECTIVE at %C");
+ }
return st;
}
else if (st != omp_end_st || block_construct)
@@ -6406,10 +6425,12 @@ parse_omp_metadirective_body (gfc_statement omp_st)
gfc_omp_variant *variant
= new_st.ext.omp_variants;
locus body_locus = gfc_current_locus;
+ bool saw_error = false;
accept_statement (omp_st);
gfc_statement next_st = ST_NONE;
+ locus next_loc;
while (variant)
{
@@ -6467,8 +6488,24 @@ parse_omp_metadirective_body (gfc_statement omp_st)
reject_statement ();
st = next_statement ();
}
+
finish:
+ /* Sanity-check that each variant finishes parsing at the same place. */
+ if (next_st == ST_NONE)
+ {
+ next_st = st;
+ next_loc = gfc_current_locus;
+ }
+ else if (st != next_st
+ || next_loc.nextc != gfc_current_locus.nextc
+ || next_loc.u.lb != gfc_current_locus.u.lb)
+ {
+ saw_error = true;
+ next_st = st;
+ next_loc = gfc_current_locus;
+ }
+
gfc_in_omp_metadirective_body = old_in_metadirective_body;
if (gfc_state_stack->head)
@@ -6480,15 +6517,13 @@ parse_omp_metadirective_body (gfc_statement omp_st)
if (variant->next)
gfc_clear_new_st ();
- /* Sanity-check that each variant finishes parsing at the same place. */
- if (next_st == ST_NONE)
- next_st = st;
- else
- gcc_assert (st == next_st);
-
variant = variant->next;
}
+ if (saw_error)
+gfc_error_now ("Variants in a metadirective at %L have "
+ "different associations", &body_locus);
+
return next_st;
}
diff --git a/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
b/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
index e7de70e6259..15aba210ce8 100644
--- a/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
@@ -1,33 +1,88 @@
! { dg-do compile }
-! { dg-ice "Statements following a block in a metadirective" }
! PR fortran/107067
program metadirectives
implicit none
logical :: UseDevice
+ integer :: n, v
!$OMP begin metadirective &
!$OMP when ( user = { condition ( UseDevice ) } &
Re: [patch, libfortran] PR114618 Version 2 Format produces incorrect output when contains 1x, ok when uses " "
Hi Jerry,
Am 30.01.25 um 21:50 schrieb Jerry D:
On 1/29/25 10:30 AM, Jerry D wrote:
Follow-up:
On 1/28/25 1:33 PM, Harald Anlauf wrote:
Jerry,
while I haven't read your actual patch yet, I think the testcase
is slightly incorrect. In fact, Intel, NAG, Nvidia and AMD flang
disagree with it.
--- snip ---
The following adjustment to the patch puts this right.
case FMT_X:
case FMT_TR:
consume_data_flag = 0;
dtp->u.p.skips += f->u.n;
tab_pos = bytes_used + dtp->u.p.skips - 1;
dtp->u.p.pending_spaces = tab_pos - dtp->u.p.max_pos + 1;
dtp->u.p.pending_spaces = dtp->u.p.pending_spaces < 0
? f->u.n : dtp->u.p.pending_spaces;
//if (t == FMT_X && tab_pos < dtp->u.p.max_pos)
//{
//write_x (dtp, dtp->u.p.skips, dtp->u.p.pending_spaces);
//dtp->u.p.skips = dtp->u.p.pending_spaces = 0;
//}
Interestingly, it also fixes a floating point exception I ran into
while setting up another test case for part 2 of this effort. I
suspect this may be what was detected by the auto patch tester.
I will clean this up, adjust the test case for this part and re-submit.
Regards,
Jerry
Here is version 2 of the patch cleaned up and with the test case revised
accordingly.
Thank you Herald for helping with my blindness.
Regressions tested on x86_64. I will wait a bit to see if the auto patch
tester reports anything.
Otherwise, OK for trunk?
this looks mostly good. There is only one (minor?) issue I saw:
writing output to a file, there is a discrepancy between stream-
and non-stream I/O. Adding
write (*, fmt1) 'RADIX', radix(pi)
write (*, fmt2) 'RADIX', radix(pi)
open (10, form="formatted")
write(10, fmt1) 'RADIX', radix(pi)
write(10, fmt2) 'RADIX', radix(pi)
close(10)
open (11, form="formatted", access="stream")
write(11, fmt1) 'RADIX', radix(pi)
write(11, fmt2) 'RADIX', radix(pi)
close(11)
shows agreement between stdout and fort.10, while fort.11 differs:
% head fort.10 fort.11
==> fort.10 <==
RADIX.. 2
RADIX . 2
==> fort.11 <==
RADIX 2
RADIX 2...
Other brands do not show this discrepancy.
Regards,
Jerry
PS working on part 2 still.
Are you addressing this in the second part of your work?
As the test program crashes with current HEAD and earlier
anyway, your patch is already progress, but we don't want
to leave it that way.
So either commit the current version and track this issue in
a PR if not yet done, or have a look if there is a quick fix.
Thanks for the work!
Harald
Re: [patch, libfortran] PR114618 Version 2 Format produces incorrect output when contains 1x, ok when uses " "
On 1/31/25 11:30 AM, Harald Anlauf wrote:
Hi Jerry,
Am 30.01.25 um 21:50 schrieb Jerry D:
On 1/29/25 10:30 AM, Jerry D wrote:
Follow-up:
On 1/28/25 1:33 PM, Harald Anlauf wrote:
Jerry,
while I haven't read your actual patch yet, I think the testcase
is slightly incorrect. In fact, Intel, NAG, Nvidia and AMD flang
disagree with it.
--- snip ---
The following adjustment to the patch puts this right.
case FMT_X:
case FMT_TR:
consume_data_flag = 0;
dtp->u.p.skips += f->u.n;
tab_pos = bytes_used + dtp->u.p.skips - 1;
dtp->u.p.pending_spaces = tab_pos - dtp->u.p.max_pos + 1;
dtp->u.p.pending_spaces = dtp->u.p.pending_spaces < 0
? f->u.n : dtp->u.p.pending_spaces;
//if (t == FMT_X && tab_pos < dtp->u.p.max_pos)
//{
//write_x (dtp, dtp->u.p.skips, dtp->u.p.pending_spaces);
//dtp->u.p.skips = dtp->u.p.pending_spaces = 0;
//}
Interestingly, it also fixes a floating point exception I ran into
while setting up another test case for part 2 of this effort. I
suspect this may be what was detected by the auto patch tester.
I will clean this up, adjust the test case for this part and re-submit.
Regards,
Jerry
Here is version 2 of the patch cleaned up and with the test case revised
accordingly.
Thank you Herald for helping with my blindness.
Regressions tested on x86_64. I will wait a bit to see if the auto patch
tester reports anything.
Otherwise, OK for trunk?
this looks mostly good. There is only one (minor?) issue I saw:
writing output to a file, there is a discrepancy between stream-
and non-stream I/O. Adding
write (*, fmt1) 'RADIX', radix(pi)
write (*, fmt2) 'RADIX', radix(pi)
open (10, form="formatted")
write(10, fmt1) 'RADIX', radix(pi)
write(10, fmt2) 'RADIX', radix(pi)
close(10)
open (11, form="formatted", access="stream")
write(11, fmt1) 'RADIX', radix(pi)
write(11, fmt2) 'RADIX', radix(pi)
close(11)
shows agreement between stdout and fort.10, while fort.11 differs:
% head fort.10 fort.11
==> fort.10 <==
RADIX.. 2
RADIX . 2
==> fort.11 <==
RADIX 2
RADIX 2...
Other brands do not show this discrepancy.
Regards,
Jerry
PS working on part 2 still.
Are you addressing this in the second part of your work?
Yes, part 2 which I am still working on.
As the test program crashes with current HEAD and earlier
anyway, your patch is already progress, but we don't want
to leave it that way.
So either commit the current version and track this issue in
a PR if not yet done, or have a look if there is a quick fix.
Thanks for the work!
Harald
To make sure I will add the example with 'stream' to an existing or new
PR. I need to look to see what this is doing. I have not heard back
from the arm auto-patch tester so I hope the patch here is an improvement.
Thanks,
Jerry
