Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
On Tue, Mar 05, 2024 at 08:06:10PM -0800, Jerry D wrote:
> On 3/5/24 1:51 PM, Harald Anlauf wrote:
> > Hi Jerry,
> >
> > on further thought, do we sanitize 'child_iomsg'?
> > We pass it to snprintf as format.
> >
> > Wouldn't a strncpy be sufficient?
> >
> > Harald
> >
> >
>
> Just to be safe I will bump char message[IOMSG_LEN] to char
> message[IOMSG_LEN + 1]
>
> This is like a C string vs a Fortran string length situation. snprintf
> guarantees we don't exceed the child_iomsg_len and null terminates it.
>
> I added 1 to:
> child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1
>
string_len_trim substracts 1 from the passed in argument.
gfc_charlen_type
string_len_trim (gfc_charlen_type len, const CHARTYPE *s)
{
if (len <= 0)
return 0;
const size_t long_len = sizeof (unsigned long);
size_t i = len - 1;
Does this account for the NULL?
--
steve
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
On 3/5/24 1:51 PM, Harald Anlauf wrote: Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald Just to be safe I will bump char message[IOMSG_LEN] to char message[IOMSG_LEN + 1] This is like a C string vs a Fortran string length situation. snprintf guarantees we don't exceed the child_iomsg_len and null terminates it. I added 1 to: child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1 Because snprintf was chopping off the last character of the fortran string to put the null in. (zero based vs one based char array). I test this with a very long string which exceeded the length and then reduced it until I could see the desired end. I have not tried running a test case with sanitize. I did check with valgrind. I will try the sanitize flags to see if we get a problem. If not will push. Thanks for comments, Jerry - On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (>common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (>common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (>common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
On 5 March 2024 04:15:12 CET, Jerry D wrote: > >Attached is the revised patch using the already available string_len_trim >function. > >This hunk is only executed if a user has not passed an iostat or iomsg >variable in the parent I/O statement and an error is triggered which >terminates execution of the program. In this case, the iomsg string is >provided in the usual error message in a "processor defined" way. > >(F2023): > >12.6.4.8.3 Executing defined input/output data transfers >--- >11 If the iostat argument of the defined input/output procedure has a nonzero >value when that procedure returns, and the processor therefore terminates >execution of the program as described in 12.11, the processor shall make the >value of the iomsg argument available in a processor-dependent manner. >--- > >OK for trunk? LGTM. thanks!
[PATCH] Fortran: error recovery while simplifying expressions [PR103707,PR106987]
Dear all,
error recovery on arithmetic errors during simplification has bugged
me for a long time, especially since the occurence of ICEs depended
on whether -frange-check is specified or not, whether array ctors
were involved, etc.
I've now come up with the attached patch that classifies the arithmetic
result codes into "hard" and "soft" errors.
A "soft" error means that it is an overflow or other exception (e.g. NaN)
that is ignored with -fno-range-check. After the patch, a soft error
will not stop simplification (a hard one will), and error status will be
passed along.
I took this opportunity to change the emitted error for division by zero
for real and complex division dependent on whether the numerator is
regular or not. This makes e.g. (0.)/0 a NaN and now says so, in
accordance with some other brands.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Other comments?
Thanks,
Harald
From d9b87bea6af77fbc794e1f21cfecb0468c68cb72 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Tue, 5 Mar 2024 21:54:26 +0100
Subject: [PATCH] Fortran: error recovery while simplifying expressions
[PR103707,PR106987]
When an exception is encountered during simplification of arithmetic
expressions, the result may depend on whether range-checking is active
(-frange-check) or not. However, the code path in the front-end should
stay the same for "soft" errors for which the exception is triggered by the
check, while "hard" errors should always terminate the simplification, so
that error recovery is independent of the flag. Separation of arithmetic
error codes into "hard" and "soft" errors shall be done consistently via
is_hard_arith_error().
PR fortran/103707
PR fortran/106987
gcc/fortran/ChangeLog:
* arith.cc (is_hard_arith_error): New helper function to determine
whether an arithmetic error is "hard" or not.
(check_result): Use it.
(gfc_arith_divide): Set "Division by zero" only for regular
numerators of real and complex divisions.
(reduce_unary): Use is_hard_arith_error to determine whether a hard
or (recoverable) soft error was encountered. Terminate immediately
on hard error, otherwise remember code of first soft error.
(reduce_binary_ac): Likewise.
(reduce_binary_ca): Likewise.
(reduce_binary_aa): Likewise.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr99350.f90:
* gfortran.dg/arithmetic_overflow_3.f90: New test.
---
gcc/fortran/arith.cc | 134 --
.../gfortran.dg/arithmetic_overflow_3.f90 | 48 +++
gcc/testsuite/gfortran.dg/pr99350.f90 | 2 +-
3 files changed, 143 insertions(+), 41 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/arithmetic_overflow_3.f90
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index d17d1aaa1d9..b373c25e5e1 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -130,6 +130,30 @@ gfc_arith_error (arith code)
}
+/* Check if a certain arithmetic error code is severe enough to prevent
+ further simplification, as opposed to errors thrown by the range check
+ (e.g. overflow) or arithmetic exceptions that are tolerated with
+ -fno-range-check. */
+
+static bool
+is_hard_arith_error (arith code)
+{
+ switch (code)
+{
+case ARITH_OK:
+case ARITH_OVERFLOW:
+case ARITH_UNDERFLOW:
+case ARITH_NAN:
+case ARITH_DIV0:
+case ARITH_ASYMMETRIC:
+ return false;
+
+default:
+ return true;
+}
+}
+
+
/* Get things ready to do math. */
void
@@ -579,10 +603,10 @@ check_result (arith rc, gfc_expr *x, gfc_expr *r, gfc_expr **rp)
val = ARITH_OK;
}
- if (val == ARITH_OK || val == ARITH_OVERFLOW)
-*rp = r;
- else
+ if (is_hard_arith_error (val))
gfc_free_expr (r);
+ else
+*rp = r;
return val;
}
@@ -792,23 +816,26 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
break;
case BT_REAL:
- if (mpfr_sgn (op2->value.real) == 0 && flag_range_check == 1)
- {
- rc = ARITH_DIV0;
- break;
- }
+ /* Set "Division by zero" only for regular numerator. */
+ if (flag_range_check == 1
+ && mpfr_zero_p (op2->value.real)
+ && mpfr_regular_p (op1->value.real))
+ rc = ARITH_DIV0;
mpfr_div (result->value.real, op1->value.real, op2->value.real,
GFC_RND_MODE);
break;
case BT_COMPLEX:
- if (mpc_cmp_si_si (op2->value.complex, 0, 0) == 0
- && flag_range_check == 1)
- {
- rc = ARITH_DIV0;
- break;
- }
+ /* Set "Division by zero" only for regular numerator. */
+ if (flag_range_check == 1
+ && mpfr_zero_p (mpc_realref (op2->value.complex))
+ && mpfr_zero_p (mpc_imagref (op2->value.complex))
+ && ((mpfr_regular_p (mpc_realref (op1->value.complex))
+ && mpfr_number_p (mpc_imagref (op1->value.complex)))
+ || (mpfr_regular_p (mpc_imagref (op1->value.complex))
+ && mpfr_number_p (mpc_realref (op1->value.complex)
+ rc = ARITH_DIV0;
gfc_set_model (mpc_realref (op1->value.complex));
if
