Patchwork [fortran] Fix PR 38536

login
register
mail settings
Submitter Tobias Burnus
Date Jan. 22, 2011, 6:03 p.m.
Message ID <4D3B1C08.2050709@net-b.de>
Download mbox | patch
Permalink /patch/80012/
State New
Headers show

Comments

Tobias Burnus - Jan. 22, 2011, 6:03 p.m.
On 01/15/2011 02:45 PM, Thomas Koenig wrote:
>>> here is an update of the patch.  This one just warns about array
>>> sections if they are followed by component references.

s/warns/prints an error/

> Revision 169130 ├╝bertragen.

How about the following patch on top of it? I think the crucial part 
(for F2008 at least) is rather the contiguity. The patch looks longer 
than it is due to white space changes.

Tobias
Thomas Koenig - Jan. 23, 2011, 11:27 a.m.
Hi Tobias,

> On 01/15/2011 02:45 PM, Thomas Koenig wrote:
>>>> here is an update of the patch.  This one just warns about array
>>>> sections if they are followed by component references.
> 
> s/warns/prints an error/
> 
>> Revision 169130 ├╝bertragen.
> 
> How about the following patch on top of it? I think the crucial part
> (for F2008 at least) is rather the contiguity. The patch looks longer
> than it is due to white space changes.

Shouldn't the test on gfc_is_simply_contiguous be reversed?  The test
case in c_loc_tests_16.f90 would then have to be changed.

	Thomas
Thomas Koenig - Jan. 23, 2011, 1:28 p.m.
Hi Tobias,

>> > On 01/15/2011 02:45 PM, Thomas Koenig wrote:
>>>>> >>>> here is an update of the patch.  This one just warns about array
>>>>> >>>> sections if they are followed by component references.
>> > 
>> > s/warns/prints an error/
>> > 
>>> >> Revision 169130 ├╝bertragen.
>> > 
>> > How about the following patch on top of it? I think the crucial part
>> > (for F2008 at least) is rather the contiguity. The patch looks longer
>> > than it is due to white space changes.
> Shouldn't the test on gfc_is_simply_contiguous be reversed?  The test
> case in c_loc_tests_16.f90 would then have to be changed.

I applied your patch with the test reversed and tested it with the
following source:

ig25@linux-fd1f:~/Krempel/Ref-c> cat cloc.f90
! { dg-do compile }
! PR 38536 - array sections as arguments to c_loc are illegal.
program main
  use iso_c_binding
  implicit none
  interface
     subroutine fun(arg) BIND(C)
       use iso_c_binding
       type(c_ptr) :: arg
     end subroutine fun
  end interface

  integer(c_int), target :: n(3)

  type(C_PTR) :: p

  n(1) = 3
  n(2) = 5
  n(3) = 7
  p = c_loc(n)
  call fun(p)
  p = c_loc(n(2:3))  ! { dg-warning "Array section" }
  call fun(p)
  print *,n
end program main
ig25@linux-fd1f:~/Krempel/Ref-c> cat fun.c
#include <stdio.h>

void fun(void *);

void fun(void *p)
{
  int *pp;
  pp = p;
  printf("%p %d %d\n",p, pp[0], pp[1]);
}
ig25@linux-fd1f:~/Krempel/Ref-c> gfortran cloc.f90 fun.c
ig25@linux-fd1f:~/Krempel/Ref-c> ./a.out
0x7fff4e19b508 1310307600 32767
0x7fff4e19b508 1310307604 32767
           3           5           7

Either my test program is wrong (which is entirely possible because I
don't use ISO C binding myself, much), or there are at least two
problems with using c_loc like this - the first version should have
worked in any case.

	Thomas
Tobias Burnus - Jan. 23, 2011, 2:13 p.m.
Dear Thomas,

Thomas Koenig wrote:
> I applied your patch with the test reversed and tested it with the
> following source:
>
> [...]
>    interface
>       subroutine fun(arg) BIND(C)
>         use iso_c_binding
>         type(c_ptr) :: arg
[...]
> void fun(void *);

The interface does not really match. On the Fortran side, you have "void 
**" on the C side you only have "void *". Try:

        type(c_ptr), VALUE :: arg


The result is then:

0x7fffc66564c0 3 5
0x7fffc66564c4 5 7
            3           5           7

C Binding is described at 
http://gcc.gnu.org/onlinedocs/gfortran/Mixed_002dLanguage-Programming.html 
-- and the pointer handling in particular at 
http://gcc.gnu.org/onlinedocs/gfortran/Working-with-Pointers.html

Side remark: If it were not for a C_LOC example, I had used "int *" on 
the C side and "integer(c_int) :: arg(*)" on the Fortran side.

Tobias
Thomas Koenig - Jan. 23, 2011, 2:52 p.m.
Hi Tobias,

> Dear Thomas,
> 
> Thomas Koenig wrote:
>> I applied your patch with the test reversed and tested it with the
>> following source:
>>
>> [...]
>>    interface
>>       subroutine fun(arg) BIND(C)
>>         use iso_c_binding
>>         type(c_ptr) :: arg
> [...]
>> void fun(void *);
> 
> The interface does not really match. On the Fortran side, you have "void
> **" on the C side you only have "void *".

You learn something new every day...

Your patch is OK with the test for gfc_is_simply_contiguous reversed,
regression-testing and an appropriate test case and change to
c_loc_tests_16.f90.

	Thomas

Patch

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 9f0d675..f458cbe 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -2700,7 +2700,6 @@  gfc_iso_c_func_interface (gfc_symbol *sym, gfc_actual_arglist *args,
       else if (sym->intmod_sym_id == ISOCBINDING_LOC)
         {
 	  gfc_ref *ref;
-	  bool seen_section;
 
           /* Make sure we have either the target or pointer attribute.  */
 	  if (!arg_attr.target && !arg_attr.pointer)
@@ -2722,35 +2721,29 @@  gfc_iso_c_func_interface (gfc_symbol *sym, gfc_actual_arglist *args,
 
 	  /* Follow references to make sure there are no array
 	     sections.  */
-	  seen_section = false;
 
 	  for (ref=args->expr->ref; ref; ref = ref->next)
 	    {
 	      if (ref->type == REF_ARRAY)
-		{
-		  if (ref->u.ar.type == AR_SECTION)
-		    seen_section = true;
-
-		  if (ref->u.ar.type != AR_ELEMENT)
-		    {
-		      gfc_ref *r;
-		      for (r = ref->next; r; r=r->next)
-			if (r->type == REF_COMPONENT)
-			  {
+		if (ref->u.ar.type != AR_ELEMENT)
+		  {
+		    gfc_ref *r;
+		    for (r = ref->next; r; r=r->next)
+		      if (r->type == REF_COMPONENT)
+			{
 			    gfc_error_now ("Array section not permitted"
 					   " in '%s' call at %L", name,
 					   &(args->expr->where));
 			    retval = FAILURE;
 			    break;
-			  }
-		    }
-		}
+			}
+		  }
 	    }
 
-	  if (seen_section && retval == SUCCESS)
-	    gfc_warning ("Array section in '%s' call at %L", name,
+	  if (retval == SUCCESS && gfc_is_simply_contiguous (args->expr, false))
+	    gfc_warning ("Array might be not contiguous in '%s' call at %L", name,
 			 &(args->expr->where));
-			 
+
           /* See if we have interoperable type and type param.  */
           if (verify_c_interop (arg_ts) == SUCCESS
               || gfc_check_any_c_kind (arg_ts) == SUCCESS)