[Patch, fortran] PR33664 - crash on invalid program

[Patch, fortran] PR33664 - crash on invalid program

[Paul Richard Thomas]

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

:ADDPATCH fortran:

This is a little embarrassing.  The testcase tells all - the symbol n
is interpreted to be an error.  Hozever, being referenced in a
specification expression, it should be pure, which it is not.  The fix
is self-explanatory - note the test for purity is redundant but
provides a belt and braces approach.  char_result_7.f90 was exposed by
this patch to have an illegal part, where the character langth was
determined by an impure function.  This has been eliminated.

Bootstrapped and regtested on x86_ia64/fc5 and certified tonto-2.3
proof.  OK for trunk?

Paul

2007-10-07  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33664
	* expr.c (gfc_specification_expr): If a function is not
	external, intrinsic or pure is an error.  Set the symbol pure
	to prevent repeat errors.

2007-10-07  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33664
	* gfortran.dg/impure_spec_expr_1.f90: New test.
	* gfortran.dg/char_result_7.f90: Remove illegal test.



-- 
The knack of flying is learning how to throw yourself at the ground and miss.
       --Hitchhikers Guide to the Galaxy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch; name="submit.diff", Size: 2532 bytes --]

Index: /svn/trunk/gcc/fortran/expr.c
===================================================================
*** /svn/trunk/gcc/fortran/expr.c	(revision 129068)
--- /svn/trunk/gcc/fortran/expr.c	(working copy)
*************** gfc_specification_expr (gfc_expr *e)
*** 2513,2518 ****
--- 2513,2530 ----
        return FAILURE;
      }
  
+   if (e->expr_type == EXPR_FUNCTION
+ 	  && !e->value.function.isym
+ 	  && !e->value.function.esym
+ 	  && !gfc_pure (e->symtree->n.sym))
+     {
+       gfc_error ("Function '%s' at %L must be PURE",
+ 		 e->symtree->n.sym->name, &e->where);
+       /* Prevent repeat error messages.  */
+       e->symtree->n.sym->attr.pure = 1;
+       return FAILURE;
+     }
+ 
    if (e->rank != 0)
      {
        gfc_error ("Expression at %L must be scalar", &e->where);
Index: /svn/trunk/gcc/testsuite/gfortran.dg/impure_spec_expr_1.f90
===================================================================
*** /svn/trunk/gcc/testsuite/gfortran.dg/impure_spec_expr_1.f90	(revision 0)
--- /svn/trunk/gcc/testsuite/gfortran.dg/impure_spec_expr_1.f90	(revision 0)
***************
*** 0 ****
--- 1,15 ----
+ ! { dg-do compile }
+ ! Checks the fix for PR33664, in which the apparent function reference
+ ! n(1) caused a seg-fault.
+ !
+ ! Contributed by Henrik Holst <holst@matmech.com>
+ !
+ module test
+ contains
+   subroutine func_1(u,n)
+     integer :: n
+     integer :: u(n(1))  ! { dg-error "must be PURE" }
+   end subroutine
+ end module test
+ ! { dg-final { cleanup-modules "test" } }
+ 
Index: /svn/trunk/gcc/testsuite/gfortran.dg/char_result_7.f90
===================================================================
*** /svn/trunk/gcc/testsuite/gfortran.dg/char_result_7.f90	(revision 129068)
--- /svn/trunk/gcc/testsuite/gfortran.dg/char_result_7.f90	(working copy)
*************** program main
*** 16,22 ****
    end interface
  
    call test (f1 (double, 100), 200)
-   call test (f2 (double, 70), 140)
  
    call indirect (double)
  contains
--- 16,21 ----
*************** contains
*** 31,42 ****
      f1 = ''
    end function f1
  
-   function f2 (fn, i)
-     integer :: i, fn
-     character (len = fn (i)) :: f2
-     f2 = ''
-   end function f2
- 
    subroutine indirect (fn)
      interface
        integer pure function fn (x)
--- 30,35 ----
*************** contains
*** 44,50 ****
        end function fn
      end interface
      call test (f1 (fn, 100), 200)
-     call test (f2 (fn, 70), 140)
    end subroutine indirect
  
    subroutine test (string, length)
--- 37,42 ----

Re: [Patch, fortran] PR33664 - crash on invalid program

[Tobias Schlüter]

Hi Paul,

Paul Richard Thomas wrote:
> This is a little embarrassing.  The testcase tells all - the symbol n
> is interpreted to be an error.  Hozever, being referenced in a
> specification expression, it should be pure, which it is not.  The fix
> is self-explanatory - note the test for purity is redundant but
> provides a belt and braces approach.  char_result_7.f90 was exposed by
> this patch to have an illegal part, where the character langth was
> determined by an impure function.  This has been eliminated.
> 
> Bootstrapped and regtested on x86_ia64/fc5 and certified tonto-2.3
> proof.  OK for trunk?

I'm embarassed to say that I don't understand the fix.  What is the 
purpose of checking .isym and .esym?  What is their purpose, anyway?  I 
understand that they're the actual functions being called, but under 
which circumstances.  I think this is a great time for you to bring some 
enlightenment to the obscure parts of gfortran :-)

Thanks,
- Tobi

Re: [Patch, fortran] PR33664 - crash on invalid program

[Paul Richard Thomas]

Tobias,

> I'm embarassed to say that I don't understand the fix.  What is the
> purpose of checking .isym and .esym?  What is their purpose, anyway?  I
> understand that they're the actual functions being called, but under
> which circumstances.  I think this is a great time for you to bring some
> enlightenment to the obscure parts of gfortran :-)

I am the last person that you should be asking for enlightenment!

The purpose of checking isym and esym is to establish whether or not
the function has been resolved.  I realise upon writing this that
e->value.function.name can substitute for both.  isym and esym are the
symbols for either intrinsic or "external"(I believe but have not
checked that it is actually everything that is not intrinsic.)
functions.  The reason that they are different is, of course, that
isym points to a gfc_intrinsic_sym, rather than a gfc_symbol.

For the purposes of this patch, the lack of resolution and the
impurity indicate that the symbol cannot be a function in a
specification expression.

Paul

Re: [Patch, fortran] PR33664 - crash on invalid program

[Tobias Schlüter]

Paul Richard Thomas wrote:
> The purpose of checking isym and esym is to establish whether or not
> the function has been resolved.  I realise upon writing this that
> e->value.function.name can substitute for both.  isym and esym are the
> symbols for either intrinsic or "external"(I believe but have not
> checked that it is actually everything that is not intrinsic.)
> functions.  The reason that they are different is, of course, that
> isym points to a gfc_intrinsic_sym, rather than a gfc_symbol.

Thanks, that makes a lot of sense.  It would perhaps be a worthwhile 
experiment to combine .isym and .esym into a single union, and to see 
all hell break loose because something assumed something different ;-)

> For the purposes of this patch, the lack of resolution and the
> impurity indicate that the symbol cannot be a function in a
> specification expression.

Why is that?  How does the lack of resolution indicate this?  (It could 
have its PURE attribute set during resolution, couldn't it?  Then the 
code would be legal.)

I'm also interested in these details out of self-interest,  My patch for 
PR 20851 broke resolution of some other permutations of valid objects. 
This problem is now PR33689.

Cheers,
- Tobi

Re: [Patch, fortran] PR33664 - crash on invalid program

[Paul Richard Thomas]

Tobias,

I am sorry to be so long in coming back to you - I've been a bit
pressed with daytime work.

> > For the purposes of this patch, the lack of resolution and the
> > impurity indicate that the symbol cannot be a function in a
> > specification expression.
>
> Why is that?  How does the lack of resolution indicate this?  (It could
> have its PURE attribute set during resolution, couldn't it?  Then the
> code would be legal.)

Not doing this caused regressions with generic interfaces with pure
specifics.  The symbol points to the generic function, whilst the esym
points to the specific.  The generic need not (cannot?) be pure.
>
> I'm also interested in these details out of self-interest,  My patch for
> PR 20851 broke resolution of some other permutations of valid objects.
> This problem is now PR33689.
>

It looks like you fixed that one:-)

Thanks

Paul

Re: [Patch, fortran] PR33664 - crash on invalid program

[Tobias Schlüter]

Paul Richard Thomas wrote:
> Tobias,
> 
> I am sorry to be so long in coming back to you - I've been a bit
> pressed with daytime work.
> 
>>> For the purposes of this patch, the lack of resolution and the
>>> impurity indicate that the symbol cannot be a function in a
>>> specification expression.
>> Why is that?  How does the lack of resolution indicate this?  (It could
>> have its PURE attribute set during resolution, couldn't it?  Then the
>> code would be legal.)
> 
> Not doing this caused regressions with generic interfaces with pure
> specifics.  The symbol points to the generic function, whilst the esym
> points to the specific.  The generic need not (cannot?) be pure.

Well, I still don't understand it, but as you had a purpose in adding 
this, I think it (and the patch) is ok.  If I don't miss anything, PR 
33499 looks like an overhaul of interface handling would be a good thing 
in any case.

>> I'm also interested in these details out of self-interest,  My patch for
>> PR 20851 broke resolution of some other permutations of valid objects.
>> This problem is now PR33689.
>>
> 
> It looks like you fixed that one:-)

It turned out to be much simpler than thought :-)

Cheers,
- Tobi