[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #19 from Steve Kargl  ---
On Sun, Jul 19, 2020 at 02:42:24PM +, arjen.markus895 at gmail dot com
wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372
> 
> --- Comment #17 from Arjen Markus  ---
> As UMASK has two arguments, should it be possible to combine integer arguments
> of different kinds? That is:
> 
> integer(kind=1) :: mask
> integer(kind=8) :: old
> 
> call umask( mask, old )
> 
> (Probably a bit overdoing it, but I guess it should be documented either way)
> 

Comment 12 makes UMASK a generic function.
Comment 13 makes UMASK a generic subroutine.

The patch for the generic subroutine UMASK converts
MASK to integer(4) and, if present, OLD to integer(4).
It then creates a call to _gfortran_umask_i4_sub. 
Note umask(2) takes a mode_t in C, which at least on
FreeBSD is an unsigned int.

So, in a sense it does not matter if the kind types
differ for MASK and OLD.  OTOH, the Fortran standard
for most intrinsic routines require types to be the
same.  gfortran should probably have a check for the
same type.

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 280157)
+++ gcc/fortran/check.c (working copy)
@@ -7230,10 +7230,7 @@ gfc_check_umask_sub (gfc_expr *mask, gfc_expr *old)
   if (old == NULL)
 return true;

-  if (!scalar_check (old, 1))
-return false;
-
-  if (!type_check (old, 1, BT_INTEGER))
+  if (!same_type_check (mask, 0, old, 1, false))
 return false;

   return true;

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

Thomas Koenig  changed:

   What|Removed |Added

 CC||tkoenig at gcc dot gnu.org

--- Comment #18 from Thomas Koenig  ---
mode_t has at least 12 bits, so an 8-bit integer argument for umask
makes little sense (and should probably be rejected).  On Linux,
mode_t is an unsigned int, 32 bits.

Personally, I would probably reject anything that is not a default integer,
but part of the fun of being a gfortran developer is that you can
chose yourself (within reason :-).

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[arjen.markus895 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #17 from Arjen Markus  ---
As UMASK has two arguments, should it be possible to combine integer arguments
of different kinds? That is:

integer(kind=1) :: mask
integer(kind=8) :: old

call umask( mask, old )

(Probably a bit overdoing it, but I guess it should be documented either way)

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[arjen.markus895 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #16 from Arjen Markus  ---
To get more involved in gfortran, I thought this issue might be a good starting
point (it is the first of a number that Steve Kargl posted some time ago for
this purpose). I added a small test program to check for the support of all
integer kinds for the various legacy routines. The issue has already been
solved for kill, but not yet for the other routines mentioned here.

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[arjen.markus895 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

Arjen Markus  changed:

   What|Removed |Added

 CC||arjen.markus895 at gmail dot 
com

--- Comment #15 from Arjen Markus  ---
Created attachment 48893
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48893=edit
Test program for legacy routines with various integer kinds

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #14 from Steve Kargl  ---
This patch brings UNLINK subroutine into agreement with
its documentation.

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 280157)
+++ gcc/fortran/check.c (working copy)
@@ -7267,6 +7275,9 @@ gfc_check_unlink_sub (gfc_expr *name, gfc_expr *status
 return false;

   if (!type_check (status, 1, BT_INTEGER))
+return false;
+
+  if (!kind_value_check (status, 1, gfc_default_integer_kind))
 return false;

   return true;

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #13 from kargl at gcc dot gnu.org ---
This patch makes the UMASK subroutine a generic subprogram.  This is
accomplished by converting its arguments to INTEGER(4), call
_gfortran_umask_i4_sub, and converting the OLD argument back to an appropriate
integer kind.  Don't know if it works with -fdefault-real-8, but who cares. 
That option should be removed from gfortran.

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 280157)
+++ gcc/fortran/check.c (working copy)
@@ -7236,6 +7236,14 @@ gfc_check_umask_sub (gfc_expr *mask, gfc_expr *old)
   if (!type_check (old, 1, BT_INTEGER))
 return false;

+  if (old->expr_type != EXPR_VARIABLE
+  || (old->symtree && old->symtree->n.sym
+ && old->symtree->n.sym->attr.intent == INTENT_IN))
+{
+  gfc_error ("OLD at %L shall be an INTENT(OUT) variable", >where);
+  return false;
+}
+
   return true;
 }

Index: gcc/fortran/intrinsic.texi
===
--- gcc/fortran/intrinsic.texi  (revision 280157)
+++ gcc/fortran/intrinsic.texi  (working copy)
@@ -14795,9 +14795,10 @@ corresponding to the lower cobound of the array along 

 @table @asis
 @item @emph{Description}:
-Sets the file creation mask to @var{MASK}. If called as a function, it
-returns the old value. If called as a subroutine and argument @var{OLD}
-if it is supplied, it is set to the old value. See @code{umask(2)}.
+Sets the file creation mask to @var{MASK}.
+If called as a function, it returns the old value.
+If called as a subroutine, and @var{OLD} is present, 
+it sets @var{OLD} to the previous value.

 @item @emph{Standard}:
 GNU extension
Index: gcc/fortran/trans-decl.c
===
--- gcc/fortran/trans-decl.c(revision 280157)
+++ gcc/fortran/trans-decl.c(working copy)
@@ -212,12 +212,13 @@ tree gfor_fndecl_convert_char4_to_char1;


 /* Other misc. runtime library functions.  */
-tree gfor_fndecl_size0;
-tree gfor_fndecl_size1;
 tree gfor_fndecl_iargc;
+tree gfor_fndecl_is_contiguous0;
 tree gfor_fndecl_kill;
 tree gfor_fndecl_kill_sub;
-tree gfor_fndecl_is_contiguous0;
+tree gfor_fndecl_size0;
+tree gfor_fndecl_size1;
+tree gfor_fndecl_umask_sub;


 /* Intrinsic functions implemented in Fortran.  */
@@ -3635,6 +3636,10 @@ gfc_build_intrinsic_function_decls (void)
   gfor_fndecl_iargc = gfc_build_library_function_decl (
get_identifier (PREFIX ("iargc")), gfc_int4_type_node, 0);
   TREE_NOTHROW (gfor_fndecl_iargc) = 1;
+
+  gfor_fndecl_umask_sub = gfc_build_library_function_decl (
+   get_identifier (PREFIX ("umask_i4_sub")), void_type_node,
+   2, gfc_pint4_type_node, gfc_pint4_type_node);

   gfor_fndecl_kill_sub = gfc_build_library_function_decl (
get_identifier (PREFIX ("kill_sub")), void_type_node,
Index: gcc/fortran/trans-intrinsic.c
===
--- gcc/fortran/trans-intrinsic.c   (revision 280157)
+++ gcc/fortran/trans-intrinsic.c   (working copy)
@@ -9138,7 +9138,51 @@ conv_intrinsic_kill_sub (gfc_code *code)
 }


+/* Translate a call to the UMASK subroutine.  */

+static tree
+conv_intrinsic_umask_sub (gfc_code *code)
+{
+  stmtblock_t block;
+  gfc_se se, se_old;
+  tree int4_type_node = gfc_get_int_type (4);
+  tree mask;
+  tree oldp = NULL_TREE;
+  tree tmp;
+
+  /* Make the function call.  */
+  gfc_init_block ();
+  gfc_init_se (, NULL);
+
+  /* Convert mask to an INTEGER(4) entity.  */
+  gfc_conv_expr (, code->ext.actual->expr);
+  gfc_add_block_to_block (, );
+  mask = fold_convert (int4_type_node, gfc_evaluate_now (se.expr, ));
+  gfc_add_block_to_block (, );
+
+  /* Deal with an optional OLD.  */
+  if (code->ext.actual->next->expr)
+{
+  gfc_init_se (_old, NULL);
+  gfc_conv_expr (_old, code->ext.actual->next->expr);
+  oldp = gfc_create_var (int4_type_node, "_oldp");
+}
+
+  tmp = build_call_expr_loc (input_location, gfor_fndecl_umask_sub, 2,
+gfc_build_addr_expr (NULL_TREE, mask),
+oldp ? gfc_build_addr_expr (NULL_TREE, oldp)
+   : null_pointer_node);
+
+  gfc_add_expr_to_block (, tmp);
+
+  if (oldp && oldp != se_old.expr)
+gfc_add_modify (, se_old.expr,
+   fold_convert (TREE_TYPE (se_old.expr), oldp));
+
+  return gfc_finish_block ();
+}
+
+
 /* The loc intrinsic returns the address of its argument as
gfc_index_integer_kind integer.  */

@@ -11899,6 +11943,10 @@ gfc_conv_intrinsic_subroutine (gfc_code *code)

 case GFC_ISYM_KILL:
   res = conv_intrinsic_kill_sub (code);
+  break;
+
+case GFC_ISYM_UMASK:
+  res = conv_intrinsic_umask_sub (code);
   break;

 case GFC_ISYM_SYSTEM_CLOCK:
Index: gcc/fortran/trans.h

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #12 from kargl at gcc dot gnu.org ---
This makes the UMASK intrinsic function generic by converting the input
argument to default integer kind and calling the relevant libgfortran function.

Index: gcc/fortran/iresolve.c
===
--- gcc/fortran/iresolve.c  (revision 280157)
+++ gcc/fortran/iresolve.c  (working copy)
@@ -3314,6 +3314,14 @@ gfc_resolve_ucobound (gfc_expr *f, gfc_expr *array, gf
 void
 gfc_resolve_umask (gfc_expr *f, gfc_expr *n)
 {
+  if (n->ts.kind != gfc_default_integer_kind)
+{
+  gfc_typespec ts;
+  ts.type = BT_INTEGER;
+  ts.kind = gfc_default_integer_kind;
+  gfc_convert_type (n, , 2);
+}
+
   f->ts.type = BT_INTEGER;
   f->ts.kind = n->ts.kind;
   f->value.function.name = gfc_get_string (PREFIX ("umask_i%d"), n->ts.kind);

The UMASK subroutine is completely broken and requires a much different
approach to fix the problem.  Namely, the subroutine has an optional 
intent(out) argument.  This needs to be dealt with in trans-intrinsic.c

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #11 from kargl at gcc dot gnu.org ---
(In reply to kargl from comment #10)
> The non-standard SLEEP intrinsic subroutine is documented as taking a
> default integer argument.  This patch enforces this restriction.
> 
> Index: gcc/fortran/check.c
> ===
> --- gcc/fortran/check.c   (revision 280157)
> +++ gcc/fortran/check.c   (working copy)
> @@ -5588,6 +5588,9 @@ gfc_check_sleep_sub (gfc_expr *seconds)
>if (!scalar_check (seconds, 0))
>  return false;
>  
> +  if (!kind_value_check (seconds, 0, gfc_default_integer_kind))
> +   return false;
> +
>return true;
>  }

Perhaps, a better patch.  This effectively makes SLEEP a generic
procedure by converting the SECONDS argument to default integer
kind.

Index: gcc/fortran/iresolve.c
===
--- gcc/fortran/iresolve.c  (revision 280157)
+++ gcc/fortran/iresolve.c  (working copy)
@@ -3699,14 +3699,19 @@ void
 gfc_resolve_sleep_sub (gfc_code *c)
 {
   const char *name;
-  int kind;
+  gfc_typespec ts;
+  gfc_expr *n;
+  gfc_clear_ts ();

-  if (c->ext.actual->expr != NULL)
-kind = c->ext.actual->expr->ts.kind;
-  else
-kind = gfc_default_integer_kind;
+  /* The argument to SLEEP has to be of default kind.  If it is not,
+ we convert it.  */
+  ts.type = BT_INTEGER;
+  ts.kind = gfc_default_integer_kind;
+  n = c->ext.actual->expr;
+  if (n != NULL && n->ts.kind != ts.kind)
+gfc_convert_type (n, , 2);

-  name = gfc_get_string (PREFIX ("sleep_i%d_sub"), kind);
+  name = gfc_get_string (PREFIX ("sleep_i%d_sub"), ts.kind);
   c->resolved_sym = gfc_get_intrinsic_sub_symbol (name);
 }

Index: gcc/fortran/intrinsic.texi
===
--- gcc/fortran/intrinsic.texi  (revision 280157)
+++ gcc/fortran/intrinsic.texi  (working copy)
@@ -13557,7 +13557,8 @@ Subroutine

 @item @emph{Arguments}:
 @multitable @columnfractions .15 .70
-@item @var{SECONDS} @tab The type shall be of default @code{INTEGER}.
+@item @var{SECONDS} @tab @var{SECONDS} shall be scalar with an
+@code{INTEGER} type.
 @end multitable

 @item @emph{Example}:

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

kargl at gcc dot gnu.org changed:

   What|Removed |Added

 CC||kargl at gcc dot gnu.org

--- Comment #10 from kargl at gcc dot gnu.org ---
The non-standard SLEEP intrinsic subroutine is documented as taking a default
integer argument.  This patch enforces this restriction.

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 280157)
+++ gcc/fortran/check.c (working copy)
@@ -5588,6 +5588,9 @@ gfc_check_sleep_sub (gfc_expr *seconds)
   if (!scalar_check (seconds, 0))
 return false;

+  if (!kind_value_check (seconds, 0, gfc_default_integer_kind))
+   return false;
+
   return true;
 }

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

--- Comment #9 from kargl at gcc dot gnu.org ---
KILL has been at least reviewed for correctness.  See

r258436 | kargl | 2018-03-11 14:34:40 -0700 (Sun, 11 Mar 2018) | 14 lines

2018-03-11  Steven G. Kargl  

* check.c (gfc_check_kill):  Check pid and sig are scalar.
(gfc_check_kill_sub): Restrict kind to 4 and 8.
* intrinsic.c (add_function): Sort keyword list.  Add pid and sig
keywords for KILL.  Remove redundant *back="back" in favor of the
original *bck="back".
(add_subroutines): Sort keyword list.  Add pid and sig keywords
for KILL.
* intrinsic.texi: Fix documentation to consistently use pid and sig.
* iresolve.c (gfc_resolve_kill): Kind can only be 4 or 8.  Choose the
correct function.
(gfc_resolve_rename_sub): Add comment.

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

kargl at gcc dot gnu.org changed:

   What|Removed |Added

   Priority|P3  |P4
 CC||kargl at gcc dot gnu.org

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[burnus at gcc dot gnu dot org]

--- Comment #7 from burnus at gcc dot gnu dot org  2007-02-23 20:42 ---
 various intrinsics do not diagnose invalid argument kinds
The question is what is the right solution:
a) Only allow certain kinds
b) Allowing all kinds and doing the conversion/providing the needed functions.

(a) is in the spirit of Fortran 95, which allowed only the default kind for a
lot of arguments to intrinsic procedures
(b) is in the spirit of Fortran 2003 which lifted a lot of those restrictions.

For legacy intrinsics we have the choice, but I favour option (b).


-- 

burnus at gcc dot gnu dot org changed:

   What|Removed |Added

 CC||burnus at gcc dot gnu dot
   ||org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[sgk at troutmask dot apl dot washington dot edu]

--- Comment #8 from sgk at troutmask dot apl dot washington dot edu  
2007-02-23 21:02 ---
Subject: Re:  various intrinsics do not diagnose invalid argument kinds

On Fri, Feb 23, 2007 at 08:42:03PM -, burnus at gcc dot gnu dot org wrote:
 
 
  various intrinsics do not diagnose invalid argument kinds
 The question is what is the right solution:
 a) Only allow certain kinds
 b) Allowing all kinds and doing the conversion/providing the needed functions.
 
 (a) is in the spirit of Fortran 95, which allowed only the default kind for a
 lot of arguments to intrinsic procedures
 (b) is in the spirit of Fortran 2003 which lifted a lot of those restrictions.
 
 For legacy intrinsics we have the choice, but I favour option (b).
 

I agree that (b) is perhaps the best option.  Afterall, we are 
moving forward in time so gfortran will chase F2003 (and F2008).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[dfranke at gcc dot gnu dot org]

--- Comment #5 from dfranke at gcc dot gnu dot org  2007-02-11 21:51 ---
Also affected:
chmod, exit, getcwd, hostnm, link, rename, sleep, system_clock, unlink, umask 
(maybe others).

SYSTEM_CLOCK accepts INTEGER(1) if exactly one or all of its optional arguments
are of that type.

UMASK(NEW[,OLD]) also accepts INTEGER(1) in its NEW argument if OLD is not
present.


-- 

dfranke at gcc dot gnu dot org changed:

   What|Removed |Added

Summary|kill intrinsic doesn't  |various intrinsics do not
   |diagnose invalid argument   |diagnose invalid argument
   |kinds   |kinds


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372

[Bug fortran/30372] various intrinsics do not diagnose invalid argument kinds

[kargl at gcc dot gnu dot org]

--- Comment #6 from kargl at gcc dot gnu dot org  2007-02-11 22:26 ---
daniel,

It's just an inconsistency in implementations.  First, note that
FX and myself implemented a lot of the g77 intrinsics procedures
as our first foray into gfortran, so we may have missed some of the 
finer details.  For example, EXIT() actually accepts any integer
kind. I don't know if it's documented or not.  I'll note

laptop:kargl[210] cat  e.f90
program e
  integer(1) i1
  integer(2) i2
  integer(4) i4
  integer(8) i8
  call exit(i1)
  call exit(i2)
  call exit(i3)
  call exit(i8)
end program e
laptop:kargl[211] gfc4x -o z -fdump-tree-original e.f90
/usr/tmp/ccA5UbZ9.o(.text+0x1e): In function `MAIN__':
: undefined reference to `_gfortran_exit_i1'
collect2: ld returned 1 exit status
laptop:kargl[212] more e.f90.003t.original 
MAIN__ ()
{
  int2 i2;
  int1 i1;
  int8 i8;
  int4 i3;

  _gfortran_set_std (70, 127, 0, 0);
  _gfortran_exit_i1 (i1);
  _gfortran_exit_i2 (i2);
  _gfortran_exit_i4 (i3);
  _gfortran_exit_i8 (i8);
}

So, I think we need to audit the intrinsics to see were we need to 
fix up the inconsistencies and documentation.


-- 

kargl at gcc dot gnu dot org changed:

   What|Removed |Added

 CC||kargl at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30372