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

[FX Coudert]

Hi Jerry,

Sorry again for taking that much time to review your patch.  
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:

     _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:

+  /* If we're storing a UNIT number, we need to check it first.  */
+  if (type == IOPARM_common_unit && e->ts.kind != 4)
+    {
+      tree max, cond;
+      ioerror_codes bad_unit;
+      int i;
+
+      bad_unit = IOERROR_BAD_UNIT;
+
+      /* Don't evaluate the UNIT number multiple times.  */
+      se.expr = gfc_evaluate_now (se.expr, &se.pre);
+
+      /* UNIT numbers should be nonnegative.  */
+      cond = fold_build2 (LT_EXPR, boolean_type_node, se.expr,
+                         build_int_cst (TREE_TYPE (se.expr),0));
+      gfc_trans_io_runtime_check (cond, var, bad_unit,
+                              "Negative unit number in I/O statement",
+                              &se.pre);
+
+      /* UNIT numbers should be less than the max.  */
+      i = gfc_validate_kind (BT_INTEGER, 4, false);
+      max = gfc_conv_mpz_to_tree (gfc_integer_kinds[i].huge, 4);
+
+      cond = fold_build2 (GT_EXPR, boolean_type_node, se.expr,
+                         fold_convert (TREE_TYPE (se.expr), max));
+      gfc_trans_io_runtime_check (cond, var, bad_unit,
+                              "Unit number in I/O statement too large",
+                              &se.pre);
+
+    }