Re: [patch, fortran] New fix: PR31201 Too large unit number generates wrong code

[Jerry DeLisle <jvdelisle at verizon dot net>]

FX Coudert wrote:
> Hi Jerry,
>
> Sorry again for taking that much time to review your patch.

No apology needed and I appreciate your careful review.  I have to work these 
things in between other stuff just like you. :)

> Unfortunately, it's still not OK due to the ERR specifier. If you take 
> code using a unit number that is too large, but wraps to a positive int4 
> value, you can generate wrong code:
>
> $ cat a.f90
>   integer :: i
>   character(len=60) :: s
>
>   open(2_8*huge(0)+20_8,file="foo",iostat=i)
>   print *, i
>   open(2_8*huge(0)+20_8,file="foo",err=99)
>   99 print *, "ERR worked ok"
>
>   write(18,*) "test"
>   end
> $ cat foo
> cat: foo: No such file or directory
> $ ./bin/gfortran -static a.f90 && ./a.out
>         5005
> ERR worked ok
> $ cat foo
> test
>
>
> It so happens that the unit number in this code (2_8*huge(0)+20_8) wraps 
> into an integer(kind=4) with the value 18. Thus, writing later to unit 
> 18 actually writes for foo (instead of opening a fort.18 file). This 
> shows that the OPEN statement is executed after your call to 
> generate_error. The code generated for this example show that:

I was aware of this problem, but I concluded it is up to the user to actually do 
something with the error code and avoid any operations on the bogus unit. 
However, like you said, it is relatively easy to not actually perform the open.
st_open calls library_start.  We can do the check there and return that the 
library is not OK.  I believe that is what this feature was intended for.

I will have a few minutes today to give this a go.

>     _gfortran_generate_error (&open_parm.2, 5005, "Unit number in I/O 
> statement too large");
>     open_parm.2.common.unit = 18;
>     _gfortran_st_open (&open_parm.2);
>     switch (open_parm.2.common.flags & 3)
>       {
>         case 1:;
>         goto __label_000099;
>       }
>
> The problem and the solution are clear: we should, after 
> _gfortran_generate_error is called, check the value of 
> (open_parm.2.common.flags & 3) exactly in the same way it is done after 
> the call to _gfortran_st_open. This is done with io_result(), which 
> should probably be called from gfc_trans_io_runtime_check().
>
> I think this time, it's really the last hurdle. I have checked the 
> standard and thought carefully about IOMSG, EOR and END, to conclude 
> that they cannot be a problem in this precise case. I know that you will 
> be travelling next week, so if you submit a final patch, I'll review it 
> and commit under your name during that time. If you don't have time to 
> put up an updated patch before leaving, I'll try to do it myself, as I 
> think it's only a matter of a few code lines.
>
> Thanks again for your work,
> FX
>
>
>
> PS: One minor remark: I think MAX_UNIT_NUMBER should be replaced by the 
> code I sent you to generate the maximal value of the unit type 
> (integer(kind=4)). The following code works fine for me:

I tried this once and it did not work.  It appeared to result in the condition 
for the max unit being optimized away completely.  So I went with what worked at 
the time.  I will check that again, I must have had something else wrong at the 
time.  :)

Thanks,

Jerry