[PATCHv2, 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 "Unclassifiable statement" error. 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. Compared to last version, this version moves the mpfr_init after NULL tree test and fixes the format problem according to Tobias's advice. Thanks a lot. Gui Haochen Thanks ChangeLog 2023-03-01 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. gcc/testsuite/ PR target/103628 * gfortran.dg/pr103628.f90: New. 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/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, mpfr_t real) { gfc_set_model_kind (kind); - mpfr_init (real); - gfc_conv_tree_to_mpfr (real, -native_interpret_expr (gfc_get_real_type (kind), - buffer, buffer_size)); + tree source = native_interpret_expr (gfc_get_real_type (kind), buffer, + buffer_size); + if (!source) +return 0; + + mpfr_init (real); + gfc_conv_tree_to_mpfr (real, source); return size_float (kind); } diff --git a/gcc/testsuite/gfortran.dg/pr103628.f90 b/gcc/testsuite/gfortran.dg/pr103628.f90 new file mode 100644 index 000..e49aefc18fd --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103628.f90 @@ -0,0 +1,14 @@ +! { dg-do compile { target powerpc*-*-* } } +! { dg-options "-O2 -mabi=ibmlongdouble" } + +! Test to ensure that it reports an "Unclassifiable statement" error +! instead of throwing an ICE when the memory represent of the HOLLERITH +! string is not unique with ibm long double encoding. + +program main + integer, parameter :: k = 16 + real(kind = k):: b = 4h1234 +end program main + +! { dg-warning "Conversion from HOLLERITH" "warning" { target powerpc*-*-* } 10 } +! { dg-error "Unclassifiable statement" "error" { target powerpc*-*-* } 10 }
Re: [PATCHv2, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi, The patch passed regression test on Power linux platforms. Sorry for missing the information. Gui Haochen 在 2023/3/3 17:12, HAO CHEN GUI via Gcc-patches 写道: > Hi, > The patch escalates the failure when Hollerith constant to real conversion > fails in native_interpret_expr. It finally reports an "Unclassifiable > statement" error. > > 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. > > Compared to last version, this version moves the mpfr_init after NULL tree > test and fixes the format problem according to Tobias's advice. Thanks a lot. > > Gui Haochen > Thanks > > ChangeLog > 2023-03-01 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. > > gcc/testsuite/ > PR target/103628 > * gfortran.dg/pr103628.f90: New. > > 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/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, >mpfr_t real) > { >gfc_set_model_kind (kind); > - mpfr_init (real); > - gfc_conv_tree_to_mpfr (real, > - native_interpret_expr (gfc_get_real_type (kind), > - buffer, buffer_size)); > > + tree source = native_interpret_expr (gfc_get_real_type (kind), buffer, > +buffer_size); > + if (!source) > +return 0; > + > + mpfr_init (real); > + gfc_conv_tree_to_mpfr (real, source); >return size_float (kind); > } > > diff --git a/gcc/testsuite/gfortran.dg/pr103628.f90 > b/gcc/testsuite/gfortran.dg/pr103628.f90 > new file mode 100644 > index 000..e49aefc18fd > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/pr103628.f90 > @@ -0,0 +1,14 @@ > +! { dg-do compile { target powerpc*-*-* } } > +! { dg-options "-O2 -mabi=ibmlongdouble" } > + > +! Test to ensure that it reports an "Unclassifiable statement" error > +! instead of throwing an ICE when the memory represent of the HOLLERITH > +! string is not unique with ibm long double encoding. > + > +program main > + integer, parameter :: k = 16 > + real(kind = k):: b = 4h1234 > +end program main > + > +! { dg-warning "Conversion from HOLLERITH" "warning" { target powerpc*-*-* } > 10 } > +! { dg-error "Unclassifiable statement" "error" { target powerpc*-*-* } 10 }
Re: [PATCHv2, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi Tobias, 在 2023/3/3 17:29, Tobias Burnus 写道: > But could you also include the 'gcc/fortran/intrinsic.cc' change > proposed in > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613030.html (and > acknowledged by Steve)? Sure, I will merge it into the patch and do the regression test. Additionally, Kewen suggested: >> Since this test case is powerpc only, I think it can be moved to >> gcc/testsuite/gcc.target/powerpc/ppc-fortran. > > Which sounds reasonable. Test cases under gcc.target are tested by check-gcc-c. It greps "warning" and "error" (C style, lower case) from the output while check-gcc-fortran greps "Warning" and "Error" (upper case). As the test case needs to check the "Warning" and "Error" messages. I have to put it in gfortran.dg directory. What's your opinion? Gui Haochen Thanks
[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
Ping [PATCHv3, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi, Gently ping this: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613497.html Thanks Gui Haochen 在 2023/3/7 16:55, HAO CHEN GUI 写道: > 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 ==
[PATCHv4, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi, I refined the patch according to reviewer's advice. The main change is to check if buffer_p is set and buffered error exists. Also two regtests are fixed by catching the new error. I sent out the revised one for review due to my limited knowledge on Fortran front end. 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 for 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. Thanks Gui Haochen ChangeLog 2023-03-21 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/assumed_size_refs_2.f90: Catch "Cannot simplify expression" error. * gfortran.dg/unpack_field_1.f90: Likewise. * 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..2572b7a3448 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->wh