diff mbox

[Fortran,committed] PR 46459: ICE (segfault): Invalid read in compare_actual_formal [error recovery]

Message ID CAKwh3qibgtTrKJhb1XxFKHhRkoOEUZD7z3mHx7t4UGOjYGs8tg@mail.gmail.com
State New
Headers show

Commit Message

Janus Weil Nov. 9, 2016, 8:42 p.m. UTC
Hi all,

I have committed yet another obvious ice-on-invalid fix:

https://gcc.gnu.org/viewcvs?rev=242020&root=gcc&view=rev

Cheers,
Janus

Comments

Andre Vehreschild Nov. 10, 2016, 9:11 a.m. UTC | #1
Hi Janus,

well, is it really that obvious? It fixes the ICE, correct. But what about
cases where the actual has an explicit interface, but is not a variable? Like
in:

function f()
  integer :: f
end function

call sub(f())

! sub() defined as in the pr.

I haven't checked whether that is valid Fortran.

Another thought, because I fell on my nose about this once too often, what
about the actual argument being a component of a derived type. I known this is
not the bug you fixes, but quite close. Think about:

type t
  real :: r(:)
end type

type(t) :: foo

call sub(foo%r)

I assume the check for C1232 does not address this correctly. Can you comment
having thought about this somewhat?

- Andre

On Wed, 9 Nov 2016 21:42:15 +0100
Janus Weil <janus@gcc.gnu.org> wrote:

> Hi all,
> 
> I have committed yet another obvious ice-on-invalid fix:
> 
> https://gcc.gnu.org/viewcvs?rev=242020&root=gcc&view=rev
> 
> Cheers,
> Janus
Janus Weil Nov. 10, 2016, 1:35 p.m. UTC | #2
Hi Andre,

> well, is it really that obvious?

well ... what can I say. If you wanna be strict about it, I guess
there is no such thing as an "obvious patch". There is basically
always something that you can miss, or that can be improved. Mikael's
patch was obvious to me in the sense that it is clear and simple and
fixes the ICE without any side effects. Thus it's a clear improvement,
and it has been rotting in bugzilla for over five years.

Are you implying that it was premature to commit it as 'obvious'?


> It fixes the ICE, correct. But what about
> cases where the actual has an explicit interface, but is not a variable? Like
> in:
> [...]
> Can you comment
> having thought about this somewhat?

In fact I have not thought about any further cases. Since you're not
giving full examples, I can only guess what you mean: The cases in the
attachment are working as expected. Anything else?

Cheers,
Janus




> On Wed, 9 Nov 2016 21:42:15 +0100
> Janus Weil <janus@gcc.gnu.org> wrote:
>
>> Hi all,
>>
>> I have committed yet another obvious ice-on-invalid fix:
>>
>> https://gcc.gnu.org/viewcvs?rev=242020&root=gcc&view=rev
>>
>> Cheers,
>> Janus
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
Andre Vehreschild Nov. 11, 2016, 11:31 a.m. UTC | #3
Hi Janus,

sorry, when I stepped on your toes. That was not my intention. While looking at
your patch and its environment those thoughts came to me. Good that you could
destroy my doubts. Thank you very much.

> In fact I have not thought about any further cases. Since you're not
> giving full examples, I can only guess what you mean: The cases in the
> attachment are working as expected. Anything else?

That was what I've thought about. Thanks for testing that I am was mistaken.

- Andre
Janus Weil Nov. 11, 2016, 12:57 p.m. UTC | #4
Hi Andre,

> sorry, when I stepped on your toes. That was not my intention.

well, I kind of got the impression that me committing 'obvious'
patches was somehow getting in conflict with _your_ toes (though I
don't quite understand why). I care as much about the quality of
gfortran as you do, trust me. Sorry if I was sounding too harsh ...

Cheers,
Janus
diff mbox

Patch

Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(Revision 241993)
+++ gcc/fortran/interface.c	(Arbeitskopie)
@@ -3190,6 +3190,7 @@  compare_actual_formal (gfc_actual_arglist **ap, gf
 	 shape array, if the dummy argument has the VOLATILE attribute.  */
 
       if (f->sym->attr.volatile_
+	  && a->expr->expr_type == EXPR_VARIABLE
 	  && a->expr->symtree->n.sym->as
 	  && a->expr->symtree->n.sym->as->type == AS_ASSUMED_SHAPE
 	  && !(f->sym->as && f->sym->as->type == AS_ASSUMED_SHAPE))
@@ -3219,6 +3220,7 @@  compare_actual_formal (gfc_actual_arglist **ap, gf
 	 dummy argument has the VOLATILE attribute.  */
 
       if (f->sym->attr.volatile_
+	  && a->expr->expr_type == EXPR_VARIABLE
 	  && a->expr->symtree->n.sym->attr.pointer
 	  && a->expr->symtree->n.sym->as
 	  && !(f->sym->as