Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat

[Harald Anlauf <anlauf@gmx.de>]

Hi Jerry,

can you please replace the user message in e.g. your new testcase
pr105456-wf.f90 by say:

piomsg="The users message containing % and %% and %s and other stuff"

This behaves as expected with Intel, but dies horribly with gfortran
after your patch!

Cheers,
Harald


On 3/6/24 05:06, 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
>
> 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 (&dtp->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.nop@gmail.com wrote:
>>>>> Hi Jerry and Steve,
>>>>>
>>>>> On 29 February 2024 19:28:19 CET, Jerry D <jvdelisle2@gmail.com>
>>>>> 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
>>>>
>>>>
>>>
>>>
>>>
>>
>
>