## g95 ##
In file foo.f90:2

CALL TST(I)
          1
Error: Array argument at (1) is smaller than the dummy size
## Intel ##
fortcom: Error: foo.f90, line 2: The storage extent of the dummy argument exceeds that of the actual argument.   [I]
CALL TST(I)
---------^
fortcom: Info: foo.f90, line 4: This variable has not been used.   [I]
 SUBROUTINE TST(I)
----------------^
compilation aborted for foo.f90 (code 1)
## Portland ##
## Sun ##

CALL TST(I)
         ^
"foo.f90", Line = 2, Column = 10: ERROR: The overall size of the dummy argument array is greater than the size of this actual argument.

Analogously for character lengths:

        program test
           character(len=10) :: x
           call foo(x)
           write(*,*) 'X=',x
        contains
        subroutine foo(y)
           character(len=20) :: y
           y = 'hello world'
        end subroutine
       end

Taken from: http://ftp.aset.psu.edu/pub/ger/fortran/test/ test17.f90 (slightly modified).

g95: Actual character argument at (1) is shorter in length than the formal argument

NAG f95: Character length too short for arg Y (no. 1) of FOO

ifort: Character length argument mismatch. [X]

Subject: Bug number PR25071

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00186.html

Subject: Bug 25071

Author: burnus
Date: Fri May  4 07:54:06 2007
New Revision: 124411

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124411
Log:
2007-05-04  Tobias Burnus  <burnus@net-b.de>

        PR fortran/25071
        * interface.c (compare_actual_formal): Check character length.

2007-05-04  Tobias Burnus  <burnus@net-b.de>

        PR fortran/25071
        * gfortran.dg/char_length_3.f90: New test.
        * gfortran.dg/char_result_2.f90: Fix test.


Added:
    trunk/gcc/testsuite/gfortran.dg/char_length_3.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/interface.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/char_result_2.f90

Note: Only the string length problem is fixed; the array storage extend still needs to be fixed.

I believe this is fixed by PR30940.

The first example gives:
Warnung: Actual argument contains too few elements for dummy argument 'i' (4/6) at (1)

The second example:
Warnung: Character length of actual argument shorter than of dummy argument 'y' (10/20) at (1)

It currently gives only warnings since I failed to get any comments when an error and when only a warning should be given.

Missing is the check for array element designators: PR32616.

Reopening..

(In reply to comment #6)
> I believe this is fixed by PR30940.
> 
> The first example gives:
> Warnung: Actual argument contains too few elements for dummy argument 'i' (4/6)
> at (1)
> 
> The second example:
> Warnung: Character length of actual argument shorter than of dummy argument 'y'
> (10/20) at (1)
> 
> It currently gives only warnings since I failed to get any comments when an
> error and when only a warning should be given.

Sorry for being 'a bit' late with comments, but IMHO this should be an error and not just a warning, because

1) The standard says so

2) Other compilers reject it (so we can't argue that we must support this common extension)

3) Not rejecting it makes it really easy to corrupt memory. Consider

program x
  character(len=10) :: a, b, c
  a = "1234567890"
  b = a
  c = a
  call xx2(b)
  print *, '::', a, '::', b, '::', c, '::'
contains
  subroutine xx2(name)
    character(len=20), intent(inout) :: name
    name = 'hi'
  end subroutine xx2
end program

This prints (gfortran 4.4, x86_64 Linux):

$ ./chardummy2
 ::    567890::hi        ::1234567890::

That is, blanking out the remaining 18 characters at the end of the character b passed to xx2 overwrites part of the character a (why are only 4 characters overwritten and not all 10? because they are allocated 4/8/16? byte aligned on the stack). Note that neither bounds checking nor valgrind detects this error.

> 
> Missing is the check for array element designators: PR32616.

I haven't looked, but maybe PR30940 and PR32616 would need to be fixed as well?

(In reply to comment #7)
> Reopening..

actually, I think this is a kind of error that should be caught at run-time with -fcheck=arguments (not to say bounds). I guess that's not so easy to implement, as it seem to imply that additional hidden subroutine arguments need to be passed around.

(In reply to comment #8)
> actually, I think this is a kind of error that should be caught at run-time
> with -fcheck=arguments (not to say bounds). I guess that's not so easy to
> implement, as it seem to imply that additional hidden subroutine arguments need
> to be passed around.

Well, that assumes that one compiles all files with the checking option. I like the idea better to have a global (static, SAVE) derived-type (struct) variable which contains the arguments and a function pointer. If loc(current function) == loc(function pointer) then the argument checking is done - else it is for a different function and no checking is done.

(In reply to comment #7)
> Sorry for being 'a bit' late with comments, but IMHO this should be an error
> and not just a warning, because
>
> 2) Other compilers reject it (so we can't argue that we must support this
> common extension)

Well, ifort and pathscale compile w/o warning and g95 with just a warning the example at:  gfortran.fortran-torture/execute/st_function.f90

Thus, for compare_actual_formal I would make a distinction between:

      /* Special case for character arguments.  For allocatable, pointer
         and assumed-shape dummies, the string length needs to match
         exactly.  */

where I agree that at least for pointer and allocatable an error should be printed - but probably also for assumed-shape dummies.

And to
      if (actual_size != 0
            && actual_size < formal_size
            && a->expr->ts.type != BT_PROCEDURE)

which can be less problematic and where a warning might be sufficient.

I actually vaguely recall why this should be a warning, and not be checked for at runtime. This is for legacy codes using real :: a(1) instead of real :: a(*). Something like

SUBROUTINE S1(a)
  INTEGER :: a(10)
  a(10)=0
END SUBROUTINE

SUBROUTINE S2(a)
  INTEGER :: a(1)
  CALL S1(a)
END SUBROUTINE

INTEGER :: a(10)
CALL S2(a)
END

'works just fine'...

No activity for more than four years. AFAICT everything is fixed, but for comment 7 preferring an error instead of the warning. Since warnings can be turned into errors with -Werror, I don't think this PR should stay opened.

(In reply to Dominique d'Humieres from comment #12)
> AFAICT everything is fixed, but for
> comment 7 preferring an error instead of the warning. Since warnings can be
> turned into errors with -Werror, I don't think this PR should stay opened.

I disagree: Code like comment #0 should give an error by default. That's also what other compilers do (e.g. ifort & sunf95) and I don't see how this can be a useful 'extension'. It is certainly problematic code.

Legacy code like the one in comment #11 should be allowed with -std=legacy only (if at all).

> > AFAICT everything is fixed, but for
> > comment 7 preferring an error instead of the warning. Since warnings can be
> > turned into errors with -Werror, I don't think this PR should stay opened.
>
> I disagree: Code like comment #0 should give an error by default.
> That's also what other compilers do (e.g. ifort & sunf95) and I don't see
> how this can be a useful 'extension'. It is certainly problematic code.

The errors introduced at r124411 have been changed to warnings by Tobias Burnus at r126271, see the rationale at https://gcc.gnu.org/ml/fortran/2007-06/msg00519.html.

> Legacy code like the one in comment #11 should be allowed with -std=legacy
> only (if at all).

If there is an agreement about that, this is something that I can handle.

IMO this PR should be closed as FIXED and a new one should be opened for the warning->error conversion.

(In reply to Dominique d'Humieres from comment #15)
> The errors introduced at r124411 have been changed to warnings by Tobias
> Burnus at r126271

.. but only for the CHARACTER case. For non-character arrays there were never any errors, and a warning has been added only in the second commit you mention.


> > Legacy code like the one in comment #11 should be allowed with -std=legacy
> > only (if at all).
> 
> If there is an agreement about that, this is something that I can handle.

I'm not sure if there will be any agreement about it, but in any case here is a patch proposal:


Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(Revision 233007)
+++ gcc/fortran/interface.c	(Arbeitskopie)
@@ -2787,10 +2787,10 @@
 			 f->sym->name, actual_size, formal_size,
 			 &a->expr->where);
           else if (where)
-	    gfc_warning (0, "Actual argument contains too few "
-			 "elements for dummy argument %qs (%lu/%lu) at %L",
-			 f->sym->name, actual_size, formal_size,
-			 &a->expr->where);
+	    gfc_notify_std (GFC_STD_LEGACY, "Actual argument contains too few "
+	                    "elements for dummy argument %qs (%lu/%lu) at %L",
+			    f->sym->name, actual_size, formal_size,
+			    &a->expr->where);
 	  return  0;
 	}
 

On comment 0 this gives a hard error with -std=f95/f2003/f2008, a warning (as before) with -std=gnu and no diagnostic at all with -std=legacy.

For comment 11 the behavior is almost unchanged, giving a warning for all -std levels except legacy.

The patch does not trigger any failures in the testsuite, which is certainly a good sign. It might still break some legacy codes, but they can just use -std=legacy (which they probably already do). Everyone else will rather benefit from the stricter diagnostics, I guess. It can catch some rather bad errors.

Any opinions on this?

> Any opinions on this?

It's fine for me.

(In reply to janus from comment #16)
> Any opinions on this?

+1

> Any opinions on this?

So far 2 for, 0 against. May be the patch can be committed?

(In reply to Dominique d'Humieres from comment #19)
> > Any opinions on this?
> 
> So far 2 for, 0 against. May be the patch can be committed?

Please do if you can, after testing on trunk.

> Please do if you can, after testing on trunk.

I can do it if Janus unassign himself.

On the current trunk, the patch produces one failure:

FAIL: gfortran.dg/warn_argument_mismatch_1.f90   -O  (test for excess errors)

Another problem is that it removes the warning for -std=legacy.

Do with it what you will ...

> On the current trunk, the patch produces one failure:
>
> FAIL: gfortran.dg/warn_argument_mismatch_1.f90   -O  (test for excess errors)
>
> Another problem is that it removes the warning for -std=legacy.
>
> Do with it what you will ...

Thanks for the pointers, they will save me some time. I think I can handle the problems myself.

Taking.

Patch at https://gcc.gnu.org/ml/fortran/2017-09/msg00126.html.

Author: dominiq
Date: Fri Sep 29 13:15:26 2017
New Revision: 253286

URL: https://gcc.gnu.org/viewcvs?rev=253286&root=gcc&view=rev
Log:
2017-09-29  Dominique d'Humieres  <dominiq@lps.ens.fr>

	PR fortran/25071
	* interface.c (compare_actual_formal): Change warnings to errors
	when "Actual argument contains too few elements for dummy
	argument", unless -std=legacy is used.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/interface.c

Author: dominiq
Date: Fri Sep 29 13:19:21 2017
New Revision: 253287

URL: https://gcc.gnu.org/viewcvs?rev=253287&root=gcc&view=rev
Log:
2017-09-29  Dominique d'Humieres  <dominiq@lps.ens.fr>

	PR fortran/25071
	* gfortran.dg/argument_checking_3.f90: Change warnings to errors.
	* gfortran.dg/argument_checking_4.f90: Likewise.
	* gfortran.dg/argument_checking_5.f90: Likewise.
	* gfortran.dg/argument_checking_6.f90: Likewise.
	* gfortran.dg/argument_checking_10.f90: Likewise.
	* gfortran.dg/argument_checking_13.f90: Likewise.
	* gfortran.dg/argument_checking_15.f90: Likewise.
	* gfortran.dg/argument_checking_18.f90: Likewise.
	* gfortran.dg/gomp/udr8.f90: Likewise.
	* gfortran.dg/warn_argument_mismatch_1.f90: Add -std=legacy to
	the dg-options.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/argument_checking_10.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_13.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_15.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_18.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_3.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_4.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_5.f90
    trunk/gcc/testsuite/gfortran.dg/argument_checking_6.f90
    trunk/gcc/testsuite/gfortran.dg/gomp/udr8.f90
    trunk/gcc/testsuite/gfortran.dg/warn_argument_mismatch_1.f90

Closing.