Bug 34640

*** Bug 34643 has been marked as a duplicate of this bug. ***

Valgrind:

==3611== Invalid read of size 8
==3611==    at 0x4A4029: gfc_trans_pointer_assignment (trans-expr.c:3756)
==3611==    by 0x485AC7: gfc_trans_code (trans.c:1002)
==3611==    by 0x49B7B9: gfc_generate_function_code (trans-decl.c:3298)

Reduced test; has nothing to do with BIND(C). I am not 100% sure it is valid. (Actually, glancing at the code, I had said it is invalid). However, it gives no error/warning (or ICE) with other compilers such as NAG f95, g95, openf95, ifort, which implies that it is presumably valid.

MODULE test
  IMPLICIT NONE
  TYPE :: my_type
    INTEGER :: value
  END TYPE
CONTAINS
  SUBROUTINE get_values(values)
    INTEGER,POINTER :: values(:)
    TYPE(my_type),POINTER :: d(:)
    values => d(:)%value
  END SUBROUTINE
END MODULE

(In reply to comment #3)
> Reduced test; has nothing to do with BIND(C). I am not 100% sure it is valid.
> (Actually, glancing at the code, I had said it is invalid). However, it gives
> no error/warning (or ICE) with other compilers such as NAG f95, g95, openf95,
> ifort, which implies that it is presumably valid.

It is valid and runs OK, as long as 'values' is not a dummy. This is because gfc_get_symbol_decl does not set GFC_DECL_SPAN for pointers.... I forgot about it, apparently.

I'm taking a look at how it might be done.

Paul 

I'm taking a look at how it might be done.

This allows compilation to proceed:

Index: gcc/fortran/trans-decl.c
===================================================================
*** gcc/fortran/trans-decl.c    (revision 131237)
--- gcc/fortran/trans-decl.c    (working copy)
*************** gfc_get_symbol_decl (gfc_symbol * sym)
*** 951,956 ****
--- 951,970 ----
          sym->backend_decl = decl;
        }

+       if (sym->attr.subref_array_pointer)
+       {
+         tree span;
+         GFC_DECL_SUBREF_ARRAY_P (sym->backend_decl) = 1;
+         span = build_decl (VAR_DECL, create_tmp_var_name ("span"),
+                            gfc_array_index_type);
+         gfc_allocate_lang_decl (sym->backend_decl);
+         gfc_finish_var_decl (span, sym);
+         TREE_STATIC (span) = 1;
+         DECL_INITIAL (span) = build_int_cst (NULL_TREE, 0);
+
+         GFC_DECL_SPAN (sym->backend_decl) = span;
+       }
+
        TREE_USED (sym->backend_decl) = 1;
        if (sym->attr.assign && GFC_DECL_ASSIGN (sym->backend_decl) == 0)
        {

but the span is not passed back to the actual argument, as this demonstrates:

MODULE test
  IMPLICIT NONE
  TYPE :: my_type
    INTEGER :: value = 99
    INTEGER :: spacer = 199
  END TYPE
CONTAINS
  SUBROUTINE get_values(values, d)
    INTEGER,POINTER :: values(:)
    TYPE(my_type),POINTER :: d(:)
    values => d(:)%value
    print *, "in get_values  ", values
  END SUBROUTINE
END MODULE

  use test
  TYPE(my_type),POINTER :: d(:)
  INTEGER,POINTER :: values(:)
  allocate (d(2))
  call get_values (values, d)
  print *, "in MAIN        ", values
  deallocate (d)
end

I'll have to figure out how this can be done.  No doubt 'span' will have to be added to the parent scope and the assignment performed on function entry and return.  Hmmm!!  Better still would be to copy in and copy out.

Back to PR34431 and friends - I'll do this next.

Paul

Jacques,

Now that 4.3 is out of the door, I have no excuse.  It's in the queue behind completing my move to Barcelona, memory leaks in allocatable components + some associated bugs and adding procedure pointers.  Thus, don't hold your breath but it's on the way.

Paul

(In reply to comment #6)
> Jacques,
> 
> Now that 4.3 is out of the door, I have no excuse.  It's in the queue behind
> completing my move to Barcelona, memory leaks in allocatable components + some
> associated bugs and adding procedure pointers.  Thus, don't hold your breath
> but it's on the way.
> 

We have had some discussion on the list about reforming array descriptors, which is what is needed here.  However, this is a big job because the early gfortran developers made some unfortunate choices and left no 'reserved' fields.  Quite aside from the coding job, it will wreck the gfortran API.

Paul 

Wait until 4.5.0 since it needs the array descriptor reform to be fixed.

Unassigning self for present until we decide who will be responsible for the work.

Paul

It helps to mark it correctly!

As fj pointed out: PR 38471 might be a duplicate of this PR.

Enfin, j'aborde le boulot....

Paul

*** Bug 43091 has been marked as a duplicate of this bug. ***

As pointed out in comment #10 pr38471 is a duplicate of this one, as well as pr42851 (see comment #1 of pr43091). They all give the same backtrace:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000018
0x00000001000d072a in gfc_trans_pointer_assignment (expr1=0x14182ba40, expr2=0x14182be20) at ../../work/gcc/fortran/trans-expr.c:4711
4711		      gfc_add_modify (&lse.post, GFC_DECL_SPAN(decl), tmp);
(gdb) bt
#0  0x00000001000d072a in gfc_trans_pointer_assignment (expr1=0x14182ba40, expr2=0x14182be20) at ../../work/gcc/fortran/trans-expr.c:4711
#1  0x00000001000aa9a6 in trans_code (code=0x14182c1e0, cond=0x0) at ../../work/gcc/fortran/trans.c:1093
#2  0x00000001000c783f in gfc_generate_function_code (ns=<value temporarily unavailable, due to optimizations>) at ../../work/gcc/fortran/trans-decl.c:4422
#3  0x00000001000aadeb in gfc_generate_module_code (ns=<value temporarily unavailable, due to optimizations>) at ../../work/gcc/fortran/trans.c:1392
#4  0x000000010006b6df in gfc_parse_file () at ../../work/gcc/fortran/parse.c:4303
#5  0x00000001000a5bbc in gfc_be_parse_file (set_yydebug=<value temporarily unavailable, due to optimizations>) at ../../work/gcc/fortran/f95-lang.c:239
#6  0x00000001006e447b in toplev_main (argc=2, argv=0x7fff5fbfd960) at ../../work/gcc/toplev.c:1053
#7  0x0000000100000c74 in start ()

*** Bug 42851 has been marked as a duplicate of this bug. ***

*** Bug 38471 has been marked as a duplicate of this bug. ***

The dupes PR38471 and PR42851 have more testcases, the former an equally lengthy discussion as this PR.

*** Bug 46339 has been marked as a duplicate of this bug. ***

Paul,

In comment #5 you mentioned this:

"I'll have to figure out how this can be done.  No doubt 'span' will have to be
added to the parent scope and the assignment performed on function entry and
return.  Hmmm!!  Better still would be to copy in and copy out."

I too began to think of this as a fix for pre 4.7.  I do think it would be nice if we could provide a fix or informative error message rather than ice in 4.6. In the pr46339 which I have marked as a duplicate of this PR, I have provided some patches that fix the ICE part, but wrong code remains. What do you think?

The patches apply to fortran-dev branch, shall I commit them there?

Other potential dupes: PR40737, PR45128.

Created attachment 24403 [details]
Example program that causes an ICE

I attached a small example program that is causing an internal compiler error for me with gfortran 4.6. I was going to submit a new bug report, but a search suggests that I am probably running into this bug, so I'll submit my small example code here. I have a few codes with pointers to derived type components that don't work with gfortran 4.6, so this bug is the only thing that is keeping me from using gfortran regularly.

Still affects trunk 4.9.0

     values => d(:)%value
 ^
0x99687f crash_signal
        ../../gcc/gcc/toplev.c:332
0x610d2b gfc_trans_pointer_assignment(gfc_expr*, gfc_expr*)
        ../../gcc/gcc/fortran/trans-expr.c:6337
0x5e0d4a trans_code
        ../../gcc/gcc/fortran/trans.c:1439
0x6083bd gfc_generate_function_code(gfc_namespace*)
        ../../gcc/gcc/fortran/trans-decl.c:5392
0x5e19c1 gfc_generate_module_code(gfc_namespace*)
        ../../gcc/gcc/fortran/trans.c:1755
0x59efc8 translate_all_program_units
        ../../gcc/gcc/fortran/parse.c:4453
0x59efc8 gfc_parse_file()
        ../../gcc/gcc/fortran/parse.c:4663
0x5dc355 gfc_be_parse_file
        ../../gcc/gcc/fortran/f95-lang.c:189
Please submit a full bug report,

*** Bug 39304 has been marked as a duplicate of this bug. ***

*** Bug 57733 has been marked as a duplicate of this bug. ***

*** Bug 45128 has been marked as a duplicate of this bug. ***

*** Bug 41627 has been marked as a duplicate of this bug. ***

Created attachment 38089 [details]
workaround (of sorts)

This is how I get around this issue.

Note that I don't usually have to explicitly provide the dimensions
in my application.  The workaround is shown applied to the demo case.

I simply wrote a "do nothing" function, "remap" that does the pointer assignment.

I hope someone can make use of this workaround to figure out a real solution!

Below is a reduced version of attachment 38089 [details] from the Comment 27.  gfortran 7.0.1 20170205 produces an ICE on whatever is the first line unless the penultimate line is removed.

$ cat chester.f90 
  type var_tables
     real, pointer :: rvar(:)
  end type 
  type real_vars
     real r
  end type 
  type(var_tables) vtab_r
  type(real_vars),  target :: x(1)
  vtab_r%rvar => x%r ! deleting this pointer association eliminates the ICE
end 

$ gfortran chester.f90 
chester.f90:1:0:

   type var_tables
 
internal compiler error: Segmentation fault: 11

chester.f90:1:0: internal compiler error: Abort trap: 6
gfortran: internal compiler error: Abort trap: 6 (program f951)

$ $ gfortran --version
GNU Fortran (MacPorts gcc7 7-20170205_0) 7.0.1 20170205 (experimental)

Created attachment 41534 [details]
An unpolished patch for the bug

Dear All,

Since I am not going to have any time to fix the array descriptor reform on fortran-dev anytime soon, I have developed the attached. An extra 'span' field has been added to the array descriptor, which is used for non-OOP pointer operations.

It needs a lot of cleaning up and thorough testing. It passes the regtests  except for: goacc/kernels-alias-4.f95; and graphite/pr14741.f90

I can see why the former might fail but the latter has me completely flummoxed.

One of the most important parts of the cleanup will be to rework trans.c:gfc_build_array_ref and some of the calls to it.I should include class pointers in the scheme as well, just for completeness.

Any feedback that you can give me would gratefully received. Let's at least finally implement this last missing F95 feature.

Cheers

Paul

> Le 10 juin 2017 à 19:40, pault at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> a écrit :
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34640
> 
> --- Comment #29 from Paul Thomas <pault at gcc dot gnu.org> ---
> Created attachment 41534 [details]
>  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41534&action=edit
> An unpolished patch for the bug
> 
> Dear All,
> 
> Since I am not going to have any time to fix the array descriptor reform on
> fortran-dev anytime soon, I have developed the attached. An extra 'span' field
> has been added to the array descriptor, which is used for non-OOP pointer
> operations.
> 
> It needs a lot of cleaning up and thorough testing. It passes the regtests 
> except for: goacc/kernels-alias-4.f95; and graphite/pr14741.f90

The failure of graphite/pr14741.f90 is  not related to your patch.

Dominique

> I can see why the former might fail but the latter has me completely flummoxed.
> 
> One of the most important parts of the cleanup will be to rework
> trans.c:gfc_build_array_ref and some of the calls to it.I should include class
> pointers in the scheme as well, just for completeness.
> 
> Any feedback that you can give me would gratefully received. Let's at least
> finally implement this last missing F95 feature.
> 
> Cheers
> 
> Paul
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

Hi Dominique,

I had suspected that. Thanks for the confirmation!

Cheers

Paul

On 10 June 2017 at 18:46, dominiq at lps dot ens.fr
<gcc-bugzilla@gcc.gnu.org> wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34640
>
> --- Comment #30 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
>> Le 10 juin 2017 à 19:40, pault at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> a écrit :
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34640
>>
>> --- Comment #29 from Paul Thomas <pault at gcc dot gnu.org> ---
>> Created attachment 41534 [details]
>>  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41534&action=edit
>> An unpolished patch for the bug
>>
>> Dear All,
>>
>> Since I am not going to have any time to fix the array descriptor reform on
>> fortran-dev anytime soon, I have developed the attached. An extra 'span' field
>> has been added to the array descriptor, which is used for non-OOP pointer
>> operations.
>>
>> It needs a lot of cleaning up and thorough testing. It passes the regtests
>> except for: goacc/kernels-alias-4.f95; and graphite/pr14741.f90
>
> The failure of graphite/pr14741.f90 is  not related to your patch.
>
> Dominique
>
>> I can see why the former might fail but the latter has me completely flummoxed.
>>
>> One of the most important parts of the cleanup will be to rework
>> trans.c:gfc_build_array_ref and some of the calls to it.I should include class
>> pointers in the scheme as well, just for completeness.
>>
>> Any feedback that you can give me would gratefully received. Let's at least
>> finally implement this last missing F95 feature.
>>
>> Cheers
>>
>> Paul
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.

*** Bug 59093 has been marked as a duplicate of this bug. ***

*** Bug 57116 has been marked as a duplicate of this bug. ***

AFAICT the patch fixes all the issues of this PR and its duplicates (I have found two more). It also compiles the tests in pr55763, but gives a wrong output

   0.00000000       1.10000002      0.200000003    
  -1.00000000    

instead of

   0.00000000      0.200000003      0.400000006    
  -1.00000000    

for two of them.

Author: pault
Date: Sun Sep 10 17:02:53 2017
New Revision: 251949

URL: https://gcc.gnu.org/viewcvs?rev=251949&root=gcc&view=rev
Log:
2017-09-10  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/34640
	PR fortran/40737
	PR fortran/55763
	PR fortran/57019
	PR fortran/57116

	* expr.c (is_subref_array): Add class pointer array dummies
	to the list of expressions that return true.
	* trans-array.c: Add SPAN_FIELD and update indices for
	subsequent fields.
	(gfc_conv_descriptor_span, gfc_conv_descriptor_span_get,
	gfc_conv_descriptor_span_set, is_pointer_array,
	get_array_span): New functions.
	(gfc_get_descriptor_offsets_for_info): New function to preserve
	API for access to descriptor fields for trans-types.c.
	(gfc_conv_scalarized_array_ref): If the expression is a subref
	array, make sure that info->descriptor is a descriptor type.
	Otherwise, if info->descriptor is a pointer array, set 'decl'
	and fix it if it is a component reference.
	(build_array_ref): Simplify handling of class array refs by
	passing the vptr to gfc_build_array_ref rather than generating
	the pointer arithmetic in this function.
	(gfc_conv_array_ref): As in gfc_conv_scalarized_array_ref, set
	'decl'.
	(gfc_array_allocate): Set the span field if this is a pointer
	array. Use the expr3 element size if it is available, so that
	the dynamic type element size is used.
	(gfc_conv_expr_descriptor): Set the span field for pointer
	assignments.
	* trans-array.h: Prototypes for gfc_conv_descriptor_span_get
	gfc_conv_descriptor_span_set and
	gfc_get_descriptor_offsets_for_info added.
	trans-decl.c (gfc_get_symbol_decl): If a non-class pointer
	array, mark the declaration as a GFC_DECL_PTR_ARRAY_P. Remove
	the setting of GFC_DECL_SPAN.
	(gfc_trans_deferred_vars): Set the span field to zero in thge
	originating scope.
	* trans-expr.c (gfc_conv_procedure_call): Do not use copy-in/
	copy-out to pass subref expressions to a pointer dummy.
	(gfc_trans_pointer_assignment): Remove code for setting of
	GFC_DECL_SPAN. Set the 'span' field for non-class pointers to
	class function results. Likewise for rank remap. In the case
	that the target is not a whole array, use the target array ref
	for remap and, since the 'start' indices are missing, set the
	lbounds to one, as required by the standard.
	* trans-intrinsic.c (conv_expr_ref_to_caf_ref): Pick up the
	'token' offset from the field decl in the descriptor.
	(conv_isocbinding_subroutine): Set the 'span' field.
	* trans-io.c (gfc_trans_transfer): Always scalarize pointer
	array io.
	* trans-stmt.c (trans_associate_var): Set the 'span' field.
	* trans-types.c (gfc_get_array_descriptor_base): Add the 'span'
	field to the array descriptor.
	(gfc_get_derived_type): Pointer array components are marked as
	GFC_DECL_PTR_ARRAY_P.
	(gfc_get_array_descr_info): Replaced API breaking code for
	descriptor offset calling gfc_get_descriptor_offsets_for_info.
	* trans.c (get_array_span): New function.
	(gfc_build_array_ref): Simplify by calling get_array_span and
	obtain 'span' if 'decl' or 'vptr' present.
	* trans.h : Rename DECL_LANG_FLAG_6, GFC_DECL_SUBREF_ARRAY_P,
	as GFC_DECL_PTR_ARRAY_P.


2017-09-10  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/34640
	* gfortran.dg/associate_24.f90: New test.
	* gfortran.dg/assumed_type_2.f90: Adjust some of the tree dump
	checks.
	* gfortran.dg/no_arg_check_2.f90: Likewise.
	* gfortran.dg/pointer_array_1.f90: New test.
	* gfortran.dg/pointer_array_2.f90: New test.
	* gfortran.dg/pointer_array_7.f90: New test.
	* gfortran.dg/pointer_array_8.f90: New test.
	* gfortran.dg/pointer_array_component_1.f90: New test.
	* gfortran.dg/pointer_array_component_2.f90: New test.
	* gfortran.dg/goacc/kernels-alias-4.f95: Bump up both tree scan
	counts by 1.

	PR fortran/40737
	* gfortran.dg/pointer_array_3.f90: New test.

	PR fortran/57116
	* gfortran.dg/pointer_array_4.f90: New test.

	PR fortran/55763
	* gfortran.dg/pointer_array_5.f90: New test.

	PR fortran/57019
	* gfortran.dg/pointer_array_6.f90: New test.

2017-09-10  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/34640
	* libgfortran/libgfortran.h: Add span field to descriptor.
	* libgfortran/libtool-version : Bump up version number to 5:0:0.

Added:
    trunk/gcc/testsuite/gfortran.dg/associate_24.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_1.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_2.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_3.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_4.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_5.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_6.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_7.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_8.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_component_1.f90
    trunk/gcc/testsuite/gfortran.dg/pointer_array_component_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/expr.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-array.h
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/fortran/trans-io.c
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/fortran/trans.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/assumed_type_2.f90
    trunk/gcc/testsuite/gfortran.dg/goacc/kernels-alias-4.f95
    trunk/gcc/testsuite/gfortran.dg/no_arg_check_2.f90
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/libgfortran.h
    trunk/libgfortran/libtool-version

Fixed on trunk.

Sorry that it has taken so long.

Paul

Bravo!

Paul,

THANK YOU!

Thanks! I'll keep an eye out for this in an upcoming release so I can test with GEOS.

Happy to learn that this old bug is solved now. 9 years : just enough to get good wine !

Most of the tests give an ICE after r251949:

pr34640.f90:32:0:

     values => d(:)%value
 
internal compiler error: Segmentation fault: 11

pr34640_5.f90:1:0:

 type var_tables
 
internal compiler error: Segmentation fault: 11

These ICEs were fixed by the latest patch in the mailing list.

> Most of the tests give an ICE after r251949:
> ...

Looking more carefully to what I did, it turns out that I saw the ICEs on r251946!-(

Sorry for the noise.