[patch, fortran] Fix pointers not escaping via C_PTR

[patch, fortran] Fix pointers not escaping via C_PTR

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

Hello world,

the attached patch fixes a wrong-code bug for gfortran where pointers
were not marked as escaping.  A C_PTR can be stashed away and reused
later (at least as long as the variable it points to remains active).

This is not a regression, but IMHO a bad wrong-code bug. The chances
of this patch introducing regressions are really, really low.

Regression-tested.  OK for trunk?

Regards

	Thomas

2019-02-29  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/71544
         * trans-types.c (gfc_typenode_for_spec) Set ts->is_c_interop of 
C_PTR and
         C_FUNPTR.
         (create_fn_spec): Mark argument as escaping if ts->is_c_interop 
is set.

2019-02-29  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/71544
         * gfortran.dg/c_ptr_tests_19.f90: New test.

[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 1021 bytes --]

Index: trans-types.c
===================================================================
--- trans-types.c	(Revision 269260)
+++ trans-types.c	(Arbeitskopie)
@@ -1176,7 +1176,8 @@ gfc_typenode_for_spec (gfc_typespec * spec, int co
         {
           spec->type = BT_INTEGER;
           spec->kind = gfc_index_integer_kind;
-          spec->f90_type = BT_VOID;
+	  spec->f90_type = BT_VOID;
+	  spec->is_c_interop = 1;  /* Mark as escaping later.  */
         }
       break;
     case BT_VOID:
@@ -2957,7 +2958,8 @@ create_fn_spec (gfc_symbol *sym, tree fntype)
 		    || f->sym->ts.u.derived->attr.pointer_comp))
 	    || (f->sym->ts.type == BT_CLASS
 		&& (CLASS_DATA (f->sym)->ts.u.derived->attr.proc_pointer_comp
-		    || CLASS_DATA (f->sym)->ts.u.derived->attr.pointer_comp)))
+		    || CLASS_DATA (f->sym)->ts.u.derived->attr.pointer_comp))
+	    || (f->sym->ts.type == BT_INTEGER && f->sym->ts.is_c_interop))
 	  spec[spec_len++] = '.';
 	else if (f->sym->attr.intent == INTENT_IN)
 	  spec[spec_len++] = 'r';

[-- Attachment #3: c_ptr_tests_19.f90 --]
[-- Type: text/x-fortran, Size: 820 bytes --]

! { dg-do run }

! PR 71544 - this failed with some optimization options due to a
! pointer not being marked as escaping.

module store_cptr
    use, intrinsic :: iso_c_binding
    implicit none
    public
    type(c_ptr), save :: cptr
end module store_cptr

subroutine init()
    use, intrinsic :: iso_c_binding
    implicit none
    integer(c_int), pointer :: a
    allocate(a)
    call save_cptr(c_loc(a))
    a = 100
end subroutine init

subroutine save_cptr(cptr_in)
    use store_cptr
    implicit none
    type(c_ptr), intent(in) :: cptr_in
    cptr = cptr_in
end subroutine save_cptr

program init_fails
    use store_cptr
    implicit none
    integer(c_int), pointer :: val
    call init()
    call c_f_pointer(cptr,val)
    print *,'The following line should print 100'
    print *,val
end program init_fails

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Steve Kargl]

On Thu, Feb 28, 2019 at 09:14:48PM +0100, Thomas Koenig wrote:
> 
> the attached patch fixes a wrong-code bug for gfortran where pointers
> were not marked as escaping.  A C_PTR can be stashed away and reused
> later (at least as long as the variable it points to remains active).
> 
> This is not a regression, but IMHO a bad wrong-code bug. The chances
> of this patch introducing regressions are really, really low.
> 
> Regression-tested.  OK for trunk?
> 

Is this a wrong code bug or a clever user wandering off into
undefined behavior?

> subroutine init()
>     use, intrinsic :: iso_c_binding
>     implicit none
>     integer(c_int), pointer :: a
>     allocate(a)
>     call save_cptr(c_loc(a))
>     a = 100
> end subroutine init

Of particular note, 'a' is an unsaved local variable.  When init() 
returns, 'a' goes out-of-scope.  Fortran normally deallocates 
an unsaved allocatable entity, but 'a' is an unsaved local variable
with the pointer attribute.  For lack of a better term, 'allocate(a)'
allocates an anonymous target and associates the anonymous target
with the pointer 'a'.  save_ptr() caches the address of the anonymous
target.  When init() returns does the anonymous target get deallocated
or does the program leak memory?  In addition, when init() returns,
what happens to the pointer association of 'a'?  I think it becomes
undefined, which means the anonymous memory is inaccessible at least
by Fortran means as there are no pointers associated with the 
anonymous target.  The Fortran standard does not what happens to
leaked memory.  In particular, the Fortran standard does not
guarantee that leaked memory is not reaped by some garbage collection
or that it must retain value 100.

-- 
Steve

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Thomas Koenig]

Hi Steve,

> On Thu, Feb 28, 2019 at 09:14:48PM +0100, Thomas Koenig wrote:
>>
>> the attached patch fixes a wrong-code bug for gfortran where pointers
>> were not marked as escaping.  A C_PTR can be stashed away and reused
>> later (at least as long as the variable it points to remains active).
>>
>> This is not a regression, but IMHO a bad wrong-code bug. The chances
>> of this patch introducing regressions are really, really low.
>>
>> Regression-tested.  OK for trunk?
>>
> 
> Is this a wrong code bug or a clever user wandering off into
> undefined behavior?
> 
>> subroutine init()
>>      use, intrinsic :: iso_c_binding
>>      implicit none
>>      integer(c_int), pointer :: a
>>      allocate(a)
>>      call save_cptr(c_loc(a))
>>      a = 100
>> end subroutine init
> 
> Of particular note, 'a' is an unsaved local variable.  When init()
> returns, 'a' goes out-of-scope.  Fortran normally deallocates
> an unsaved allocatable entity, but 'a' is an unsaved local variable
> with the pointer attribute.  For lack of a better term, 'allocate(a)'
> allocates an anonymous target and associates the anonymous target
> with the pointer 'a'.  save_ptr() caches the address of the anonymous
> target.  When init() returns does the anonymous target get deallocated
> or does the program leak memory?

Neither.  If there were no C_PTR pointing to the memory, it would
leak memory.

> In addition, when init() returns,
> what happens to the pointer association of 'a'? 

'a' becomes undefined, hence it is no longer associated with the
memory.

> I think it becomes
> undefined, which means the anonymous memory is inaccessible at least
> by Fortran means as there are no pointers associated with the
> anonymous target.

That is not correct.  Looking at F2018, 18.2.3.3, by using C_F_POINTER,
you can

" Associate a data pointer with the target of a C pointer and specify 
its shape. "

First, this talks about a C pointer having a target.  Second, you can
re-estabilsh the association to a different pointer to the Fortran
program.

Regards

	Thomas

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Thomas Koenig]

I wrote:

> First, this talks about a C pointer having a target.  Second, you can
> re-estabilsh the association to a different pointer to the Fortran
> program.

There is another point to consider: This is interoperability with C
we are dealing with, so we also have to follow C semantics.
And, love it or hate it, C pointers escape.

So, OK for trunk?

Regards

	Thomas

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Jerry DeLisle]

On 2/28/19 12:14 PM, Thomas Koenig wrote:
> Hello world,
> 
> the attached patch fixes a wrong-code bug for gfortran where pointers
> were not marked as escaping.  A C_PTR can be stashed away and reused
> later (at least as long as the variable it points to remains active).
> 
> This is not a regression, but IMHO a bad wrong-code bug. The chances
> of this patch introducing regressions are really, really low.
> 
> Regression-tested.  OK for trunk?
> 

This is Ok except the test case. You are using dg-run but are not testing for 
any condition. I think you want to to IF .... STOP instead of printing some test 
values or dou you just want dg-conpile?

Fix test case and OK for trunk.

Jerry

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

Hi Jerry,

> I think you want to to IF .... STOP instead of printing some test values 
> or dou you just want dg-conpile?

You're correct, I wanted a dg-do run.

Here is the version of the test case that I committed.

Thanks for the review (and the other one).

Regards

	Thomas

[-- Attachment #2: c_ptr_tests_19.f90 --]
[-- Type: text/x-fortran, Size: 781 bytes --]

! { dg-do run }

! PR 71544 - this failed with some optimization options due to a
! pointer not being marked as escaping.

module store_cptr
    use, intrinsic :: iso_c_binding
    implicit none
    public
    type(c_ptr), save :: cptr
end module store_cptr

subroutine init()
    use, intrinsic :: iso_c_binding
    implicit none
    integer(c_int), pointer :: a
    allocate(a)
    call save_cptr(c_loc(a))
    a = 100
end subroutine init

subroutine save_cptr(cptr_in)
    use store_cptr
    implicit none
    type(c_ptr), intent(in) :: cptr_in
    cptr = cptr_in
end subroutine save_cptr

program init_fails
    use store_cptr
    implicit none
    integer(c_int), pointer :: val
    call init()
    call c_f_pointer(cptr,val)
    if (val /= 100) stop 1
end program init_fails