[patch, fortran] PR66461 ICE on missing end program in fixed source

[patch, fortran] PR66461 ICE on missing end program in fixed source

[Jerry DeLisle]

Hi all,

The following patch regression tested on x86-64.  The ICE is from an attempt to
free a bad expression after a MATCH_ERROR is returned. I have not been able to
identify an exact cause, there being numerous matchers involved attempting to
match the logical expression.

Regardless, it is an error on invalid so I suggest we commit this patch and
close the PR.  I dont think its a regression as marked in bugzilla.  I see the
the internal error as far back as 4.5.  If someone has an earlier build and can
see where this does not occur, please let me know. (In case I missed something.

The results of the patch gives the following:

$ gfc s.f
s.f:4:9:

          if ( x(1) < 0 .or.
         1
Error: Can not process after the IF statement shown at (1)
f951: Error: Unexpected end of file in ‘s.f’


OK for trunk?

Regards,

Jerry

2016-05-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/66461
	* match.c (gfc_match_if): Catch unxpected MATCH_ERROR and issue an error
	message.


diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index f3a4a43..85e6f92 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -1560,7 +1560,16 @@ gfc_match_if (gfc_statement *if_type)
   if (m == MATCH_ERROR)
     return MATCH_ERROR;

-  gfc_match (" if ( %e ) ", &expr);    /* Guaranteed to match.  */
+  m = gfc_match (" if ( %e ) ", &expr);  /* Not always guaranteed to match.  */
+
+  if (m == MATCH_ERROR)
+    {
+      /* Under some invalid conditions like unexpected end of file, one
+        can get an error in the match. We bail out here and hope for
+        the best (the best being an error reported somewhere else).  */
+      gfc_error ("Can not process after the IF statement shown at %C");
+      return MATCH_ERROR;
+    }

   m = gfc_match_pointer_assignment ();
   if (m == MATCH_YES)

Re: [patch, fortran] PR66461 ICE on missing end program in fixed source

[Jerry DeLisle]

Ping  No comments from anyone, shall I just commit?

On 05/18/2016 11:25 AM, Jerry DeLisle wrote:
> Hi all,
> 
> The following patch regression tested on x86-64.  The ICE is from an attempt to
> free a bad expression after a MATCH_ERROR is returned. I have not been able to
> identify an exact cause, there being numerous matchers involved attempting to
> match the logical expression.
> 
> Regardless, it is an error on invalid so I suggest we commit this patch and
> close the PR.  I dont think its a regression as marked in bugzilla.  I see the
> the internal error as far back as 4.5.  If someone has an earlier build and can
> see where this does not occur, please let me know. (In case I missed something.
> 
> The results of the patch gives the following:
> 
> $ gfc s.f
> s.f:4:9:
> 
>           if ( x(1) < 0 .or.
>          1
> Error: Can not process after the IF statement shown at (1)
> f951: Error: Unexpected end of file in ‘s.f’
> 
> 
> OK for trunk?
> 
> Regards,
> 
> Jerry
> 
> 2016-05-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
> 	PR fortran/66461
> 	* match.c (gfc_match_if): Catch unxpected MATCH_ERROR and issue an error
> 	message.
> 
> 
> diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
> index f3a4a43..85e6f92 100644
> --- a/gcc/fortran/match.c
> +++ b/gcc/fortran/match.c
> @@ -1560,7 +1560,16 @@ gfc_match_if (gfc_statement *if_type)
>    if (m == MATCH_ERROR)
>      return MATCH_ERROR;
> 
> -  gfc_match (" if ( %e ) ", &expr);    /* Guaranteed to match.  */
> +  m = gfc_match (" if ( %e ) ", &expr);  /* Not always guaranteed to match.  */
> +
> +  if (m == MATCH_ERROR)
> +    {
> +      /* Under some invalid conditions like unexpected end of file, one
> +        can get an error in the match. We bail out here and hope for
> +        the best (the best being an error reported somewhere else).  */
> +      gfc_error ("Can not process after the IF statement shown at %C");
> +      return MATCH_ERROR;
> +    }
> 
>    m = gfc_match_pointer_assignment ();
>    if (m == MATCH_YES)
> 
> 
> 

Re: [patch, fortran] PR66461 ICE on missing end program in fixed source

[Andre Vehreschild]

Hi Jerry,

I have tested your patch and gave it a review and the only thing I like
to have is a testcase. Can you provide one from the PR? With a testcase
I say the patch is ok for trunk and thanks for the patch.

Please note, I don't have review rights in the area the patch
addresses, although I am familiar with the matcher having worked in it.
This "review" is just a helper for an official reviewer to "second" my
opinion, hoping to get your patch faster into trunk.

Regards and thanks for the patch,
	Andre

On Wed, 18 May 2016 11:25:57 -0700
Jerry DeLisle <jvdelisle@charter.net> wrote:

> Hi all,
> 
> The following patch regression tested on x86-64.  The ICE is from an attempt to
> free a bad expression after a MATCH_ERROR is returned. I have not been able to
> identify an exact cause, there being numerous matchers involved attempting to
> match the logical expression.
> 
> Regardless, it is an error on invalid so I suggest we commit this patch and
> close the PR.  I dont think its a regression as marked in bugzilla.  I see the
> the internal error as far back as 4.5.  If someone has an earlier build and can
> see where this does not occur, please let me know. (In case I missed something.
> 
> The results of the patch gives the following:
> 
> $ gfc s.f
> s.f:4:9:
> 
>           if ( x(1) < 0 .or.
>          1
> Error: Can not process after the IF statement shown at (1)
> f951: Error: Unexpected end of file in ‘s.f’
> 
> 
> OK for trunk?
> 
> Regards,
> 
> Jerry
> 
> 2016-05-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
> 	PR fortran/66461
> 	* match.c (gfc_match_if): Catch unxpected MATCH_ERROR and issue an error
> 	message.
> 
> 
> diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
> index f3a4a43..85e6f92 100644
> --- a/gcc/fortran/match.c
> +++ b/gcc/fortran/match.c
> @@ -1560,7 +1560,16 @@ gfc_match_if (gfc_statement *if_type)
>    if (m == MATCH_ERROR)
>      return MATCH_ERROR;
> 
> -  gfc_match (" if ( %e ) ", &expr);    /* Guaranteed to match.  */
> +  m = gfc_match (" if ( %e ) ", &expr);  /* Not always guaranteed to match.  */
> +
> +  if (m == MATCH_ERROR)
> +    {
> +      /* Under some invalid conditions like unexpected end of file, one
> +        can get an error in the match. We bail out here and hope for
> +        the best (the best being an error reported somewhere else).  */
> +      gfc_error ("Can not process after the IF statement shown at %C");
> +      return MATCH_ERROR;
> +    }
> 
>    m = gfc_match_pointer_assignment ();
>    if (m == MATCH_YES)
> 
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [patch, fortran] PR66461 ICE on missing end program in fixed source

[Jerry DeLisle]

On 05/22/2016 04:53 AM, Andre Vehreschild wrote:
> Hi Jerry,
> 
> I have tested your patch and gave it a review and the only thing I like
> to have is a testcase. Can you provide one from the PR? With a testcase
> I say the patch is ok for trunk and thanks for the patch.
> 
> Please note, I don't have review rights in the area the patch
> addresses, although I am familiar with the matcher having worked in it.
> This "review" is just a helper for an official reviewer to "second" my
> opinion, hoping to get your patch faster into trunk.
> 
> Regards and thanks for the patch,
> 	Andre

Thanks Andre, I am going to hold off just a bit after some further info from Mikael.

We are obviously intercepting the problem in different ways, but the root
problem still exists.

Regards,

Jerry

Re: [patch, fortran] PR66461 ICE on missing end program in fixed source

[Jerry DeLisle]

On 05/22/2016 06:00 PM, Jerry DeLisle wrote:
> On 05/22/2016 04:53 AM, Andre Vehreschild wrote:
>> Hi Jerry,
>>
>> I have tested your patch and gave it a review and the only thing I like
>> to have is a testcase. Can you provide one from the PR? With a testcase
>> I say the patch is ok for trunk and thanks for the patch.
>>
>> Please note, I don't have review rights in the area the patch
>> addresses, although I am familiar with the matcher having worked in it.
>> This "review" is just a helper for an official reviewer to "second" my
>> opinion, hoping to get your patch faster into trunk.
>>
>> Regards and thanks for the patch,
>> 	Andre
> 
> Thanks Andre, I am going to hold off just a bit after some further info from Mikael.
> 
> We are obviously intercepting the problem in different ways, but the root
> problem still exists.
> 
> Regards,
> 
> Jerry
> 

Committed rev 236627 under simple rule. Also credit Mikael Morin for breaking
through the ICE so to speak.  Many thanks,

Jerry

https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=236627