[PATCHv3, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi, The patch escalates the failure when Hollerith constant to real conversion fails in native_interpret_expr. It finally reports an "Cannot simplify expression" error in do_simplify method. The patch of pr95450 added a verification for decoding/encoding checking in native_interpret_expr. native_interpret_expr may fail on real type conversion and returns a NULL tree then. But upper layer calls don't handle the failure so that an ICE is reported when the verification fails. IBM long double is an example. It doesn't have a unique memory presentation for some real values. So it may not pass the verification. The new test case shows the problem. errorcount is used to check if an error is already reported or not when getting a bad expr. Buffered errors need to be excluded as they don't increase error count either. The patch passed regression test on Power and x86 linux platforms. Gui Haochen Thanks ChangeLog 2023-03-07 Haochen Gui gcc/ PR target/103628 * fortran/target-memory.cc (gfc_interpret_float): Return FAIL when native_interpret_expr gets a NULL tree. * fortran/arith.cc (gfc_hollerith2real): Return NULL when gfc_interpret_float fails. * fortran/error.cc (gfc_buffered_p): Define. * fortran/gfortran.h (gfc_buffered_p): Declare. * fortran/intrinsic.cc: Add diagnostic.h to include list. (do_simplify): Save errorcount and check it at finish. Report a "Cannot simplify expression" error on a bad result if error count doesn't change and no other errors buffered. gcc/testsuite/ PR target/103628 * gfortran.dg/pr103628.f90: New. Co-Authored-By: Tobias Burnus patch.diff diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index c0d12cfad9d..d3d38c7eb6a 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -2752,10 +2752,12 @@ gfc_hollerith2real (gfc_expr *src, int kind) result = gfc_get_constant_expr (BT_REAL, kind, &src->where); hollerith2representation (result, src); - gfc_interpret_float (kind, (unsigned char *) result->representation.string, - result->representation.length, result->value.real); - - return result; + if (gfc_interpret_float (kind, + (unsigned char *) result->representation.string, + result->representation.length, result->value.real)) +return result; + else +return NULL; } /* Convert character to real. The constant will be padded or truncated. */ diff --git a/gcc/fortran/error.cc b/gcc/fortran/error.cc index 214fb78ba7b..872d42e731e 100644 --- a/gcc/fortran/error.cc +++ b/gcc/fortran/error.cc @@ -49,6 +49,13 @@ static gfc_error_buffer error_buffer; static output_buffer *pp_error_buffer, *pp_warning_buffer; static int warningcount_buffered, werrorcount_buffered; +/* Return buffered_p. */ +bool +gfc_buffered_p (void) +{ + return buffered_p; +} + /* Return true if there output_buffer is empty. */ static bool diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 219ef8c7612..edfe11796a6 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3328,6 +3328,7 @@ void gfc_internal_error (const char *, ...) ATTRIBUTE_NORETURN ATTRIBUTE_GCC_GFC void gfc_clear_error (void); bool gfc_error_check (void); bool gfc_error_flag_test (void); +bool gfc_buffered_p (void); notification gfc_notification_std (int); bool gfc_notify_std (int, const char *, ...) ATTRIBUTE_GCC_GFC(2,3); diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index e89131f5a71..9d049001a51 100644 --- a/gcc/fortran/intrinsic.cc +++ b/gcc/fortran/intrinsic.cc @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see #include "options.h" #include "gfortran.h" #include "intrinsic.h" +#include "diagnostic.h" /* For errorcount. */ /* Namespace to hold the resolved symbols for intrinsic subroutines. */ static gfc_namespace *gfc_intrinsic_namespace; @@ -4620,6 +4621,7 @@ do_simplify (gfc_intrinsic_sym *specific, gfc_expr *e) { gfc_expr *result, *a1, *a2, *a3, *a4, *a5, *a6; gfc_actual_arglist *arg; + int old_errorcount = errorcount; /* Max and min require special handling due to the variable number of args. */ @@ -4708,7 +4710,12 @@ do_simplify (gfc_intrinsic_sym *specific, gfc_expr *e) finish: if (result == &gfc_bad_expr) -return false; +{ + if (errorcount == old_errorcount + && (gfc_buffered_p () && !gfc_error_flag_test ())) + gfc_error ("Cannot simplify expression at %L", &e->where); + return false; +} if (result == NULL) resolve_intrinsic (specific, e); /* Must call at run-time */ diff --git a/gcc/fortran/target-memory.cc b/gcc/fortran/target-memory.cc index 7ce7d736629..0c47aa6b842 100644 --- a/gcc/fortran/target-memory.cc +++ b/gcc/fortran/target-memory.cc @@ -416,11 +416,14 @@ gfc_interpret_float (int kind, unsigned char *buffer, size_t buffer_size, mp
Re: [Patch, fortran] PR37336 finalization
Paul, first of all, thank you very much indeed for the hard work you put into this! This is a great step for gfortran. I can hurry this along to get the patch into 13-branch or I can wait until 14-branch opens. Personally, I think that this fixes so many bugs, and makes the compiler so much better, that I would prefer having it in gcc-13. Finalization was only of very limited use before, and the risk of meaningful regressions (short of a build failure) is therefore very low. Again, thanks a lot! Best regards Thomas
Re: [Patch, fortran] PR37336 finalization
On Tue, Mar 07, 2023 at 03:58:32PM +0100, Thomas Koenig via Fortran wrote: > Paul, > > first of all, thank you very much indeed for the hard work you put into > this! This is a great step for gfortran. Ditto**2 > > I can hurry this along to get the patch > > into 13-branch or I can wait until 14-branch opens. > > Personally, I think that this fixes so many bugs, and makes > the compiler so much better, that I would prefer having it > in gcc-13. Finalization was only of very limited use before, > and the risk of meaningful regressions (short of a build > failure) is therefore very low. > I agree with Thomas. The main branch is in stage 4, which is regression and documentation fixing mode. I would think the number of bugs fixed by your patch can be argued as fixing regressions. I can set aside some time on Saturday to help with a review (if required). -- Steve
F77 indexed file support
All, I'm getting back into Fortran after many years away and was going to start with my goto kick the tires application. Basically the lottery tracking system found in this book. https://www.theminimumyouneedtoknow.com/app_book.html The first cut was Fortran 77 on the VAX. There we could create/use indexed files with OPEN statements much like this: 210 OPEN (UNIT=K_DRAW_CHAN, 1 FILE=DRAWING_DATA, 2 STATUS='OLD', 3 ORGANIZATION='INDEXED', 4 ACCESS='KEYED', 5 RECORDTYPE='FIXED', 6 FORM='UNFORMATTED', 7 RECL=K_DRAWING_RECORD_SIZE/4, 8 CARRIAGECONTROL='FORTRAN', 9 KEY=(1:8:CHARACTER), 1 DISP='KEEP', 2 IOSTAT=L_DRAW_ERR, 3 ERR=999) The ORGANIZATION='INDEXED' is key. GnuCOBOL https://gnucobol.sourceforge.io/ uses the BerkleyDB (sp?) library so the standard COBOL indexed file support from the big computers can at least be mimicked. I'm searching everywhere and I cannot find Gnu Fortran (any flavor) having an ORGANIZATION clause in the OPEN(). Is this true? Are we forced to use some SQLite wrapper library when porting VAX/VMS FORTRAN to Linux? Btw, is there a "search" utility for the archives or do I have to pull down all of the zip files, unzip into directory, and grep to look for stuff like this? I'm guessing it has come up before. Thanks, Roland -- Roland Hughes, President Logikal Solutions (630)-205-1593 http://www.theminimumyouneedtoknow.com http://www.infiniteexposure.net http://www.johnsmith-book.com http://www.logikalblog.com http://www.interestingauthors.com/blog
Re: F77 indexed file support
Hi Roland, 210 OPEN (UNIT=K_DRAW_CHAN, 1 FILE=DRAWING_DATA, 2 STATUS='OLD', 3 ORGANIZATION='INDEXED', I'd never heard of that one up to now. 4 ACCESS='KEYED', 5 RECORDTYPE='FIXED', 6 FORM='UNFORMATTED', 7 RECL=K_DRAWING_RECORD_SIZE/4, 8 CARRIAGECONTROL='FORTRAN', 9 KEY=(1:8:CHARACTER), 1 DISP='KEEP', 2 IOSTAT=L_DRAW_ERR, 3 ERR=999) The ORGANIZATION='INDEXED' is key. GnuCOBOL https://gnucobol.sourceforge.io/ uses the BerkleyDB (sp?) library so the standard COBOL indexed file support from the big computers can at least be mimicked. I'm searching everywhere and I cannot find Gnu Fortran (any flavor) having an ORGANIZATION clause in the OPEN(). ORGANIZATION is not an extension that gfortran supports. ifort, which traces its lineage back to VMS Fortran, supports ORGANIZATION, but not 'INDEXED', according to https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/file-operation-i-o-statements/open-statement-specifiers/open-organization-specifier.html This is likely a Fortran interface to a VMS speciality; the older operating systems had stuff like that. UNIX did away with all the record-orientation (I also remember VSAM and ISAM data sets on old IBM mainframes) and UNIX and derivatives, and Windows, now just offers the "stream of bytes" model. So, if you need the functionality, you will have to implement it yourself, possibly via a database. Best regards Thomas
Re: F77 indexed file support
I have never worked much with VAXes, but I do remember that VAX used a file system where you made a new version of a file and the older versions were automatically kept. I guess that is the purpose of the INDEXED organisation. It is not so much a limitation of gfortran that it does not support this, but a consequence of the operating system's completely different view on files and file management. Regards, Arjen Op di 7 mrt 2023 om 23:58 schreef Thomas Koenig via Fortran < fortran@gcc.gnu.org>: > Hi Roland, > > > 210 OPEN (UNIT=K_DRAW_CHAN, > > 1FILE=DRAWING_DATA, > > 2STATUS='OLD', > > 3ORGANIZATION='INDEXED', > > I'd never heard of that one up to now. > > > 4ACCESS='KEYED', > > 5RECORDTYPE='FIXED', > > 6FORM='UNFORMATTED', > > 7RECL=K_DRAWING_RECORD_SIZE/4, > > 8CARRIAGECONTROL='FORTRAN', > > 9KEY=(1:8:CHARACTER), > > 1DISP='KEEP', > > 2IOSTAT=L_DRAW_ERR, > > 3ERR=999) > > > > The ORGANIZATION='INDEXED' is key. > > > > GnuCOBOL > > > > https://gnucobol.sourceforge.io/ > > > > uses the BerkleyDB (sp?) library so the standard COBOL indexed file > > support from the big computers can at least be mimicked. > > > > I'm searching everywhere and I cannot find Gnu Fortran (any flavor) > > having an ORGANIZATION clause in the OPEN(). > > ORGANIZATION is not an extension that gfortran supports. > ifort, which traces its lineage back to VMS Fortran, supports > ORGANIZATION, but not 'INDEXED', according to > > > https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/file-operation-i-o-statements/open-statement-specifiers/open-organization-specifier.html > > This is likely a Fortran interface to a VMS speciality; the older > operating systems had stuff like that. UNIX did away with all > the record-orientation (I also remember VSAM and ISAM data sets > on old IBM mainframes) and UNIX and derivatives, and Windows, now > just offers the "stream of bytes" model. > > So, if you need the functionality, you will have to implement it > yourself, possibly via a database. > > Best regards > > Thomas >
Re: [Patch, fortran] PR37336 finalization
Hi Thomas and Steve, Last night, I scoped out the work required to get the patch ready to commit. Sorting out the testcases will be the main load since they have grown "organically". I propose to change over to one test for each paragraph of F2018 7.5.6.2/7.5.6.3 and to verify them against the other brands. I suspect that this will allow a weeding out of the existing tests. It will take me a couple of weeks. Could you please review the patch on that self same time scale? I would be happy to have feedback one file at a time. Salvatore alerted me to an ICE on valid, which occurs with -fcheck=all. Eliminating the assert at trans.cc:1101 fixes it. I am not convinced that the line does anything useful now and so consider it to be erased. I suspect that future bugs will result from components that are finalizable and allocatable and so I will be paying close attention to it. In the course of the last few months, I have found that the associate construct causes a lot of niggles. Looking at pr87477, I see that I fixed a number of the dependencies a while since, on a branch other than the development branch of the time. As a result, the fixes got lost. I will turn to pr87477 just as soon as pr37336 is done and dusted. Cheers Paul On Tue, 7 Mar 2023 at 14:58, Thomas Koenig wrote: > Paul, > > first of all, thank you very much indeed for the hard work you put into > this! This is a great step for gfortran. > > > I can hurry this along to get the patch > > into 13-branch or I can wait until 14-branch opens. > > Personally, I think that this fixes so many bugs, and makes > the compiler so much better, that I would prefer having it > in gcc-13. Finalization was only of very limited use before, > and the risk of meaningful regressions (short of a build > failure) is therefore very low. > > Again, thanks a lot! > > Best regards > > Thomas > > > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: F77 indexed file support
On 7 March 2023 23:18:58 CET, Roland Hughes via Fortran wrote: [ snip namelist IO ] >Btw, is there a "search" utility for the archives or do I have to pull down >all of the zip files, unzip into directory, and grep to look for stuff like >this? I'm guessing it has come up before. Indeed we have https://inbox.sourceware.org/fortran/ along the traditional pipermail ml interface. thanks,