[Bug libfortran/114304] New: [14 Regression] Rejects lapack test

[Bug libfortran/114304] New: [14 Regression] Rejects lapack test

[rguenth at gcc dot gnu.org]

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

            Bug ID: 114304
           Summary: [14 Regression] Rejects lapack test
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

I see

[  208s] make[1]: Entering directory
'/home/abuild/rpmbuild/BUILD/lapack-3.9.0/TESTING'
[  208s] DGEBAL: Testing the balancing of a DOUBLE PRECISION general matrix
[  208s] ./EIG/xeigtstd < dbal.in > dbal.out 2>&1
[  208s] make[1]: *** [Makefile:427: dbal.out] Error 2

> cat dbal.out 
At line 119 of file dchkbl.f (unit = 5, file = 'stdin')
Fortran runtime error: Semicolon not allowed as separator with DECIMAL='point'

Error termination. Backtrace:
#0  0x7efcc2c38b42 in ???
#1  0x7efcc2c3961e in ???
#2  0x7efcc2c3a17f in ???
#3  0x7efcc2e8dc75 in ???
#4  0x7efcc2e906f1 in ???
#5  0x7efcc2e9144d in ???
#6  0x7efcc2e921aa in ???
#7  0x7efcc2e97a00 in ???
#8  0x55b5f7e0a993 in dchkbl_.constprop.0
        at /home/abuild/rpmbuild/BUILD/lapack-3.9.0/TESTING/EIG/dchkbl.f:119

which does

      READ( NIN, FMT = * )( SCALIN( I ), I = 1, N )

and the input has at its end

  2.494800386918399765D+291  1.582914569427869018D+175 
1.004336277661868922D+59  3.186183822264904554D-58  5.053968264940243633D-175 
0.40083367200179455560D-291;

0

[Bug libfortran/114304] [14 Regression] Rejects lapack test

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'm not sure it's desirable/valid to reject these?

[Bug libfortran/114304] [14 Regression] Rejects lapack test

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Has been long fixed upstream.  Guess it's to be expectd.

[Bug libfortran/114304] [14 Regression] Rejects lapack test

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---
                 CC|                            |burnus at gcc dot gnu.org,
                   |                            |jvdelisle at gcc dot gnu.org

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
I think the semicolon is not permitted as item separator but if I have as input
  '1.234 134.23 abc'
or likewise:
  '1.234, 134.23 abc'
and
  read (..., * ) x(1:2)

it works – i.e. only reading two floats and then stopping before reading 'abc'.

But if I do the very same but replace ' abc' by ' ;', I get the error, which
seems to be rather inconsistent — what's the difference between 'abc' and ';'
in this case?

* * *

The message is new since
  r14-9050-ga71d87431d0c4e
  libgfortran: [PR105473] Fix checks for decimal='comma'.

Jerry, can you check?

[Bug libfortran/114304] [14 Regression] Rejects lapack test

[burnus at gcc dot gnu.org]

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 57668
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57668&action=edit
Testcase

Testresults of the attached testcase:
   See also https://godbolt.org/z/q4rG61EvW

The attached testcase shows with ifort and flang:
   1.234350       1243.240       13.24000     a
   1.234350       1243.240       13.24000     a
   1.234350       1243.240       13.24000    
   1.234350       1243.240       13.24000    
   1.234350       1243.240       13.24000     ;
   1.234350       1243.240       13.24000     ;

With GCC mainline:
   1.23434997       1243.23999       13.2399998     a
   1.23434997       1243.23999       13.2399998     a
Fortran runtime error: Semicolon not allowed as separator with DECIMAL='point'

With GCC 13 (and 4.9):
   1.23434997       1243.23999       13.2399998     a
   1.23434997       1243.23999       13.2399998     a
   1.23434997       1243.23999       13.2399998    
   1.23434997       1243.23999       13.2399998    
Fortran runtime error: End of file

[Bug libfortran/114304] [14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
            Summary|[14 Regression] Rejects     |[14 Regression] libgfortran
                   |lapack test                 |I/O – bogus "Semicolon not
                   |                            |allowed as separator with
                   |                            |DECIMAL='point'"

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
I just noticed that the change
  libgfortran: [PR105473] Fix checks for decimal='comma'.
got also backported to
  r13-8411-g7ecea49245bc6aeb6c889a4914961f94417f16e5 
on Thu Mar 7, 2024.

Thus, GCC 13 is now affected as well!

[Disclaimer: I have not checked the spec, but it seems very much like a
wrong-code bug.]

[Bug libfortran/114304] [14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> ---
[For completeness: The LAPACK testsuite change Richard mentioned in comment 2
is
https://github.com/Reference-LAPACK/lapack/commit/64e8a7500d817869e5fcde35afd39af8bc7a8086
- That's for g95 and was applied 2020.]

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-03-11
     Ever confirmed|0                           |1

--- Comment #7 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
I suppose we can put this error behind a -std= option. Just my luck it is
discovered right after I backport to 13. The semi-colon in this case is
intended to indicate the end of the line I think.

I will have to did through the standards a but.  If one is desperate to fix
this and we need to revert on 13, let me know.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jvdelisle at gcc dot gnu.org

--- Comment #8 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
This gets around the bogus error and makes sense to me.  However with your
reduced test case I get and EOF error later.  I also get this EOF witt gfortran
9. I have not checked 10, 11, or 12 yet.

If I can build lapack cleanly I will push this as sort of obvious.

$ git diff
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index e38e9a84976..c23c2bb2048 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -481,10 +481,10 @@ eat_separator (st_parameter_dt *dtp)
       break;

     case ';':
-      if (dtp->u.p.current_unit->decimal_status == DECIMAL_POINT)
+     if (dtp->u.p.current_unit->decimal_status == DECIMAL_COMMA)
        {
          generate_error (&dtp->common, LIBERROR_READ_VALUE,
-          "Semicolon not allowed as separator with DECIMAL='point'");
+          "Semicolon not allowed as separator with DECIMAL='comma'");
          unget_char (dtp, c);
          break;
        }

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #9 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Patch on comment #8 breaks all sorts of things. Not so obvious. I will try
reverting the original hunk from pr105473 and then work from there.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenther at suse dot de]

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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
> Am 11.03.2024 um 20:03 schrieb jvdelisle at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114304
> 
> --- Comment #9 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
> Patch on comment #8 breaks all sorts of things. Not so obvious. I will try
> reverting the original hunk from pr105473 and then work from there.

Just to add, I think rejecting something we accepted before and when this
doesn’t fix a rejects-valid shouldn’t be done on branches and given it affects
the standard library, when it’s SONAME is not altered as it might affect
programs compiled with older libgfortran (maybe there’s the argument for
something like a LIBGFORTRAN_STRICT environment to control such if really
needed?)

> --
> You are receiving this mail because:
> You reported the bug.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #11 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Created attachment 57676
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57676&action=edit
Proposed patch

This patch fixes the issue by reverting the troublesome hunk and regression
tests OK on x86_64. The test case added for pr105473 still passes. I will add
the test attached already to this PR to our test suite.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jerry DeLisle <jvdelisle@gcc.gnu.org>:

https://gcc.gnu.org/g:0c179654c3170749f3fb3232f2442fcbc99bffbb

commit r14-9432-g0c179654c3170749f3fb3232f2442fcbc99bffbb
Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
Date:   Mon Mar 11 15:15:34 2024 -0700

    libgfortran: [PR114304] Revert portion of PR105347 change.

            PR libfortran/105437
            PR libfortran/114304

    libgfortran/ChangeLog:

            * io/list_read.c (eat_separator): Remove check for decimal
            point mode and semicolon used as a seprator. Removes
            the regression.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pr105473.f90: Add additional checks to address
            the case of semicolon at the end of a line.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jerry DeLisle
<jvdelisle@gcc.gnu.org>:

https://gcc.gnu.org/g:824a71f609b37a8121793075b175e2bbe14fdb82

commit r13-8417-g824a71f609b37a8121793075b175e2bbe14fdb82
Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
Date:   Mon Mar 11 15:15:34 2024 -0700

    libgfortran: [PR114304] Revert portion of PR105347 change.

            PR libfortran/105437
            PR libfortran/114304

    libgfortran/ChangeLog:

            * io/list_read.c (eat_separator): Remove check for decimal
            point mode and semicolon used as a seprator. Removes
            the regression.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pr105473.f90: Add additional checks to address
            the case of semicolon at the end of a line.

    (cherry picked from commit 0c179654c3170749f3fb3232f2442fcbc99bffbb)

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

--- Comment #14 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 57680
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57680&action=edit
Testcase with decimal=COMMA, passes with ifort/ifx/flang - fails with gfortran

> commit r14-9432-g0c179654c3170749f3fb3232f2442fcbc99bffbb
> commit r13-8417-g824a71f609b37a8121793075b175e2bbe14fdb82

Thanks for the fix.

We are now back to the GCC 13 result → comment 4

Namely, attachment 57668 now gives:

   1.23434997       1243.23999       13.2399998     a
   1.23434997       1243.23999       13.2399998     a
   1.23434997       1243.23999       13.2399998    
   1.23434997       1243.23999       13.2399998    
At line 33 of file foo.f90 (unit = 99, file = 'foo.inp')

* * *

The question is whether the following show give an error as shown above:
  real :: x(3)
  character(len=1) :: s
  ...
  write(99, '(a)') '1.23435 1243.24 13.24 ;'
  read(99, *) x, s

Or whether reading this line should work, i.e. reading ';' as character – as it
does with ifort and flang.

Or in other words:
* Does ';' count as character, readable by list-directed formatted I/O? (ifort,
ifx, flang)
* Or doesn't it? (gfortran since at least 4.9)

* * *

In F2023 (23-007r1), "13.10.2 Values and value separators":

"A value separator is
 • a comma optionally preceded by one or more contiguous blanks and
   optionally followed by one or more contiguous blanks,
   unless the decimal edit mode is COMMA, in which case a semicolon is
   used in place of the comma,
 • a slash optionally preceded by one or more contiguous blanks and
   optionally followed by one or more contiguous blanks, or
 • one or more contiguous blanks between two nonblank values
   or following the last nonblank value, where a nonblank value
   is a constant, an r*c form, or an r* form."

(where 'r' is an positive integer and 'c' is a literal constant [with ...].)


To me it reads as if the semicolon should be read just fine.

* * *

I now have tried another testcase with decimal=COMMA, which works just fine
with ifort / ifx /flang as shown at
  https://godbolt.org/z/ajeTjzEfY

But with GCC it fails with:
  Fortran runtime error: Comma not allowed as separator with DECIMAL='comma'

See godbolt link above for gfortran vs. ifort vs. ifx. vs. flang
or the attached testcase.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #15 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #14)
--- snip ---
> The question is whether the following show give an error as shown above:
>   real :: x(3)
>   character(len=1) :: s
>   ...
>   write(99, '(a)') '1.23435 1243.24 13.24 ;'
>   read(99, *) x, s
> 
> Or whether reading this line should work, i.e. reading ';' as character – as
> it does with ifort and flang.
> 
> Or in other words:
> * Does ';' count as character, readable by list-directed formatted I/O?
> (ifort, ifx, flang)
> * Or doesn't it? (gfortran since at least 4.9)
> 

This error caught my attention right away as being odd. With the regression
part out of the way at the moment I will study this further. My initial thought
is we have an eat_seprator out of place where we only need an eat_spaces.
Regardless, I am looking into it.

[Bug libfortran/114304] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13/14 Regression]          |libgfortran I/O – bogus
                   |libgfortran I/O – bogus     |"Semicolon not allowed as
                   |"Semicolon not allowed as   |separator with
                   |separator with              |DECIMAL='point'"
                   |DECIMAL='point'"            |
                 CC|                            |law at gcc dot gnu.org

--- Comment #16 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Per c#12, c#13, c#14 & c#15, dropping the regression marker, but leaving open.

[Bug libfortran/114304] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #17 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Jeffrey A. Law from comment #16)
> Per c#12, c#13, c#14 & c#15, dropping the regression marker, but leaving
> open.

Interestingly, the remaining part of this bug is also a regression, it just
does not break LAPACK. Reverting this change fixes it which means the new test
for pr105473 will fail. I have an idea where to put this check in
read_complex() but I have not finished this and tested it.

Jeffrey, if you would like me to push this, let me know. We can mark
pr105473.f90 in the test suite to XFAIL or comment out the one check there that
fails.

diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index fb3f7dbc34d..c178acd61a5 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -471,8 +471,6 @@ eat_separator (st_parameter_dt *dtp)
     case ',':
       if (dtp->u.p.current_unit->decimal_status == DECIMAL_COMMA)
        {
-         generate_error (&dtp->common, LIBERROR_READ_VALUE,
-          "Comma not allowed as separator with DECIMAL='comma'");
          unget_char (dtp, c);
          break;
        }

[Bug libfortran/114304] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[law at gcc dot gnu.org]

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

--- Comment #18 from Jeffrey A. Law <law at gcc dot gnu.org> ---
I don't have an opinion on the Fortran patch -- I think it's up to the Fortran
front-end maintainers to make that decision.

Given there's still a regression here, I'll put the marker back.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

--- Comment #19 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Regarding the LAPACK issue:

Actually, I am inclined to
* regard it as LAPACK bug
* that was also fixed upstream, see comment 6, to make g95 happy.
as ';' is not a value separator - while ' ;' is fine, where the blank is a
value separator.

My testcase of comment 4 therefore always used a space before the ',' / ';'.

* * *

I have now created an extended testcase, attached to PR105473 as attachment
57695. (Only testing integer/real parsing, not reading the char afterward as in
comment 4.)

The same testcase can also be found at https://godbolt.org/z/14h48167W and
shows the result with gfortran, ifort, ifx and flag. I used this result to add
comments to the testcases.

* * *

For some F2023 wording, see comment 14 above.

And I have to admit that I am rather confused by the results as there does not
seem to be any consistent pattern; there are cases where I agree with
gfortran's error even though neither ifort nor flang show one, while for
others, I think gfortran gets it wrong.

In particular, I think for the following cases:

  call t('point', ';') ! gfortran: no error, others: error
→ IMHO invalid: not a value separator and not an integer.

  call t('point', '5;') ! gfortran: no error shown, others: error
→ This is the LAPACK example but for integers.
  I think ';' is invalid as it is not part of the integer but also not a value
separator.

  call t('comma', '7 ,') ! gfortran: error; others: no error
→ IMHO valid - I think the ' ' as value separator is sufficient.

  call t('point', '3.3,', .true.) ! gfortran/flag: error shown; ifort: no error
→ What's wrong with a comma as value separator?

  call t('comma', '3,3;', .true.) ! gfortran: error shown; others: no error
→ Same, except that ';' is now the value separator


But in the following cases, I think gfortran is *right*:
  call t('point', '5.') ! gfortran/flang: Error shown, ifort: no error
→ '.' is not part of an integer nor a value separator

  call t('comma', '5,') ! gfortran: error; others: no error
→ Likewise for ',' - the ',' is not part of an integer nor a value separator


Disclaimer: I might have easily overlooked some fine print.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #20 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Some additional information from the 2023 standard.

13.10.3.1List-directed input forms

8 If the next effective item is default, ASCII, or ISO 10646 character and
    • the character sequence does not contain value separators,
    • the character sequence does not cross a record boundary,
    • the first nonblank character is not a quotation mark or an apostrophe,
    • the leading characters are not digits followed by an asterisk, and
    • the character sequence contains at least one character,

  the delimiting apostrophes or quotation marks are not required. If the
delimiters are
  omitted, the character sequence is terminated by the first blank, comma (if
the decimal
  edit mode is POINT), semicolon (if the decimal edit mode is COMMA), slash, or
end of
  record; in this case apostrophes and quotation marks within the datum are not
to be
  doubled.

---------------------------------------------------

I am still digesting this and should have something sorted out in the next day
or so.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #21 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
The following may be a helpful read.

https://www.ibm.com/docs/en/openxl-fortran-aix/17.1.2?topic=formatting-value-separators

I am auditing our list_read.c code for the various types.  The NULL read plays
into this.

The process is complicated further by the use of repeat counts in front of
values.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #22 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jerry DeLisle <jvdelisle@gcc.gnu.org>:

https://gcc.gnu.org/g:93adf88cc6744aa2c732b765e1e3b96e66cb3300

commit r14-9822-g93adf88cc6744aa2c732b765e1e3b96e66cb3300
Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
Date:   Fri Apr 5 19:25:13 2024 -0700

    libfortran: Fix handling of formatted separators.

            PR libfortran/114304
            PR libfortran/105473

    libgfortran/ChangeLog:

            * io/list_read.c (eat_separator): Add logic to handle spaces
            preceding a comma or semicolon such that that a 'null' read
            occurs without error at the end of comma or semicolon
            terminated input lines. Add check and error message for ';'.
            (list_formatted_read_scalar): Treat comma as a decimal point
            when specified by the decimal mode on the first item.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pr105473.f90: Modify to verify new error message.
            * gfortran.dg/pr114304.f90: New test.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[chenglulu at loongson dot cn]

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

chenglulu <chenglulu at loongson dot cn> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chenglulu at loongson dot cn

--- Comment #23 from chenglulu <chenglulu at loongson dot cn> ---
(In reply to GCC Commits from comment #22)
> The master branch has been updated by Jerry DeLisle <jvdelisle@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:93adf88cc6744aa2c732b765e1e3b96e66cb3300
> 
> commit r14-9822-g93adf88cc6744aa2c732b765e1e3b96e66cb3300
> Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
> Date:   Fri Apr 5 19:25:13 2024 -0700
> 
>     libfortran: Fix handling of formatted separators.
>     
>             PR libfortran/114304
>             PR libfortran/105473
>     
>     libgfortran/ChangeLog:
>     
>             * io/list_read.c (eat_separator): Add logic to handle spaces
>             preceding a comma or semicolon such that that a 'null' read
>             occurs without error at the end of comma or semicolon
>             terminated input lines. Add check and error message for ';'.
>             (list_formatted_read_scalar): Treat comma as a decimal point
>             when specified by the decimal mode on the first item.
>     
>     gcc/testsuite/ChangeLog:
>     
>             * gfortran.dg/pr105473.f90: Modify to verify new error message.
>             * gfortran.dg/pr114304.f90: New test.

Hi,
This patch causes spec2017 527 and 627 tests to fail.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenth at gcc dot gnu.org]

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

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to chenglulu from comment #23)
> (In reply to GCC Commits from comment #22)
> > The master branch has been updated by Jerry DeLisle <jvdelisle@gcc.gnu.org>:
> > 
> > https://gcc.gnu.org/g:93adf88cc6744aa2c732b765e1e3b96e66cb3300
> > 
> > commit r14-9822-g93adf88cc6744aa2c732b765e1e3b96e66cb3300
> > Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
> > Date:   Fri Apr 5 19:25:13 2024 -0700
> > 
> >     libfortran: Fix handling of formatted separators.
> >     
> >             PR libfortran/114304
> >             PR libfortran/105473
> >     
> >     libgfortran/ChangeLog:
> >     
> >             * io/list_read.c (eat_separator): Add logic to handle spaces
> >             preceding a comma or semicolon such that that a 'null' read
> >             occurs without error at the end of comma or semicolon
> >             terminated input lines. Add check and error message for ';'.
> >             (list_formatted_read_scalar): Treat comma as a decimal point
> >             when specified by the decimal mode on the first item.
> >     
> >     gcc/testsuite/ChangeLog:
> >     
> >             * gfortran.dg/pr105473.f90: Modify to verify new error message.
> >             * gfortran.dg/pr114304.f90: New test.
> 
> Hi,
> This patch causes spec2017 527 and 627 tests to fail.

This is the speed and rate versions (same source) of cam4, which is

       The Community Earth System Model version 1.0.2 (CESM1.0.2)

                     http://www.cesm.ucar.edu

       See the CESM web site for documentation and information.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenth at gcc dot gnu.org]

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

--- Comment #25 from Richard Biener <rguenth at gcc dot gnu.org> ---
 (t_initf) Read in prof_inparm namelist from: drv_in
 (shr_sys_abort) ERROR: (T_INITF) :: namelist read returns an error condition
for prof_inparm
(shr_sys_abort) WARNING: calling mpi_abort() and stopping
MPI_Abort: error code = 0


There's code appearantly matching

   logical profile_disable
   logical profile_barrier
   logical profile_single_file
   logical profile_global_stats
   integer profile_depth_limit
   integer profile_detail_limit
   integer profile_outpe_num
   integer profile_outpe_stride
   integer profile_timer
   logical profile_papi_enable 
   namelist /prof_inparm/ profile_disable, profile_barrier, &
                          profile_single_file, profile_global_stats, &
                          profile_depth_limit, &
                          profile_detail_limit, profile_outpe_num, &
                          profile_outpe_stride, profile_timer, &
                          profile_papi_enable
...
       write(p_logunit,*) '(t_initf) Read in prof_inparm namelist from:
'//trim(NLFilename)
       unitn = shr_file_getUnit()

       ierr = 1
       open( unitn, file=trim(NLFilename), status='old', iostat=ierr )
       if (ierr .eq. 0) then

          ! Look for prof_inparm group name in the input file.  
          ! If found, leave the file positioned at that namelist group.
          call find_group_name(unitn, 'prof_inparm', status=ierr)

          if (ierr == 0) then  ! found prof_inparm
             read(unitn, nml=prof_inparm, iostat=ierr)
             if (ierr /= 0) then
                call shr_sys_abort( subname//':: namelist read returns an'// &
                                    ' error condition for prof_inparm' ) 

and the input file has

&prof_inparm
 profile_single_file            = .true.
/
&pio_inparm
/

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenth at gcc dot gnu.org]

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

--- Comment #26 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 57895
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57895&action=edit
drv_in input file

This is the complete input file.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenth at gcc dot gnu.org]

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

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
And find_group_name does

   ios = 0
   do while (ios <= 0)
      read(unit, '(a)', iostat=ios, end=102) inrec
...
         inrec2 = to_lower(adjustl(inrec))

         ! check for leading '&'
         if (inrec2(1:1) == '&') then

            ! check for case insensitive group name
            if (trim(lc_group) == inrec2(2:len_grp+1)) then

               ! found group name.  backspace to leave file position at this
record
               backspace(unit)
               status = 0
               return

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

--- Comment #28 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 57896
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57896&action=edit
Testcase

It seems as if 'tabs' cause problems, e.g. for:

 profile_single_file            = .true.

where there are two tabs before '='.

* * *

The problem seems to be that the new code uses:

-  eat_spaces (dtp);
   dtp->u.p.comma_flag = 0;
+  c = next_char (dtp);
+  if (c == ' ')
+    {
+      eat_spaces (dtp);

Thus, it explicitly checks for ' ' while eat_spaces handles:

  while (c != EOF && (c == ' ' || c == '\r' || c == '\t'));

Testcase attached.

I think we need at least an "|| c == '\t'"; I guess '\r' isn't really required
here, or is it?

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[rguenther at suse dot de]

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

--- Comment #29 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 8 Apr 2024, burnus at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114304
> 
> --- Comment #28 from Tobias Burnus <burnus at gcc dot gnu.org> ---
> Created attachment 57896
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57896&action=edit
> Testcase
> 
> It seems as if 'tabs' cause problems, e.g. for:
> 
>  profile_single_file            = .true.
> 
> where there are two tabs before '='.
> 
> * * *
> 
> The problem seems to be that the new code uses:
> 
> -  eat_spaces (dtp);
>    dtp->u.p.comma_flag = 0;
> +  c = next_char (dtp);
> +  if (c == ' ')
> +    {
> +      eat_spaces (dtp);
> 
> Thus, it explicitly checks for ' ' while eat_spaces handles:
> 
>   while (c != EOF && (c == ' ' || c == '\r' || c == '\t'));
> 
> Testcase attached.
> 
> I think we need at least an "|| c == '\t'"; I guess '\r' isn't really required
> here, or is it?

Might be for \r\n line endings?  I'd keep it for the sake of preserving
previous behavior.  isspace(3) tests for \f, \n, \r, \t, \v and space
(but of course all depends on the locale, not sure whether libgfortran
needs to care for locales)

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[burnus at gcc dot gnu.org]

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

--- Comment #30 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #29)
> Might be for \r\n line endings?

New lines are handled slightly differently – and \f and \v don't seem to be
handled at all.

Comparing the result with ifort/ifx/flang, they handle a bare '\r' (in contrast
to \r\n) at fewer places than gfortran – albeit from the code it looks as if a
\r not followed by \n is not handled consistently either.

> I'd keep it for the sake of preserving
> previous behavior.  isspace(3) tests for \f, \n, \r, \t, \v and space
> (but of course all depends on the locale, not sure whether libgfortran
> needs to care for locales)

I have added one example to the testcase, but that seems to be already handled
by the code further below which handles '\r' and '\n' - thus, the patch does
not handle it explicitly.

The Fortran standard does not seem to permit \f, \t, \v at all – at least I
only found those in the C interop section. The standard does not really define
what new line actually is, but: "A newline character is a nonblank character
returned by the intrinsic function NEW_LINE." – This handles different
character kinds, but always returns a single character (e.g. \r vs. \n would be
possible, but not \r\n).

* * *

Patch – which handles '\t':
https://gcc.gnu.org/pipermail/gcc-patches/2024-April/648950.html

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #31 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:477c8a82f38e353a8c6313b38197c70b12deea80

commit r14-9843-g477c8a82f38e353a8c6313b38197c70b12deea80
Author: Tobias Burnus <tburnus@baylibre.com>
Date:   Mon Apr 8 21:47:51 2024 +0200

    Fortran: Accept again tab as alternative to space as separator [PR114304]

    This fixes a side-effect of/regression caused by r14-9822-g93adf88cc6744a,
    which was for the same PR.

            PR libfortran/114304

    libgfortran/ChangeLog:

            * io/list_read.c (eat_separator): Accept tab as alternative to
space.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pr114304-2.f90: New test.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #32 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #28)
> Created attachment 57896 [details]
> Testcase
> 
--- snip ---

> I think we need at least an "|| c == '\t'"; I guess '\r' isn't really
> required here, or is it?

The particular snippit that broke the tab is only executed at the beginning
read of the first effective item so we dont need anythin else at this spot.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

--- Comment #33 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #30)
> (In reply to rguenther@suse.de from comment #29)
> > Might be for \r\n line endings?
> 
> New lines are handled slightly differently – and \f and \v don't seem to be
> handled at all.
> 
> Comparing the result with ifort/ifx/flang, they handle a bare '\r' (in
> contrast to \r\n) at fewer places than gfortran – albeit from the code it
> looks as if a \r not followed by \n is not handled consistently either.
> 
--- snip ---

Initially we primarily handled /n and I recall having to do a number of places
to take care of the \r\n because Microsoft allegedly chose to not follow the
standards of the day and did the \r/\n. One can argue about that since some of
the old teletypes needed the \n to roll the paper after the carriage return. ;)

We have never done anything with \f or \v .

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #34 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jerry DeLisle
<jvdelisle@gcc.gnu.org>:

https://gcc.gnu.org/g:b55a35bcc80a7402576556c2f9d161229fb220ef

commit r13-8640-gb55a35bcc80a7402576556c2f9d161229fb220ef
Author: Jerry DeLisle <jvdelisle@gcc.gnu.org>
Date:   Sun Apr 21 20:50:26 2024 -0700

    libfortran: Fix handling of formatted separators.

            Backport from mainline.

            PR libfortran/114304
            PR libfortran/105473

    libgfortran/ChangeLog:

            * io/list_read.c (eat_separator): Add logic to handle spaces
            preceding a comma or semicolon such that that a 'null' read
            occurs without error at the end of comma or semicolon
            terminated input lines. Add check and error message for ';'.
            Accept tab as alternative to space.
            (list_formatted_read_scalar): Treat comma as a decimal point
            when specified by the decimal mode on the first item.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pr105473.f90: Modify for revised checks.
            * gfortran.dg/pr114304-2.f90: New test.
            * gfortran.dg/pr114304.f90: New test.

[Bug libfortran/114304] [13/14 Regression] libgfortran I/O – bogus "Semicolon not allowed as separator with DECIMAL='point'"

[jvdelisle at gcc dot gnu.org]

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

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #35 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Fixed on 13 and mainline (14)