Bug 18540 - Jumping into blocks gives error rather than warning
Summary: Jumping into blocks gives error rather than warning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.0.0
: P2 enhancement
Target Milestone: 4.1.0
Assignee: Tobias Schlüter
URL:
Keywords: diagnostic, rejects-valid
: 25705 (view as bug list)
Depends on:
Blocks: 19292
  Show dependency treegraph
 
Reported: 2004-11-18 11:26 UTC by Paul Thomas
Modified: 2006-01-28 20:59 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-01-14 19:03:09


Attachments
allow jumping into blocks in legacy mode (463 bytes, patch)
2006-01-08 21:30 UTC, Steven Bosscher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Thomas 2004-11-18 11:26:42 UTC
program lau_error
!
! most compilers seem to permit this excremental bit of coding
! some give warnings that this is bad stuff
! gfortran barfs on it
!
! In file lau_error.f90:12
!
! 10    continue
!  1
! In file lau_error.f90:9
!
!    if ( i > 1 ) goto 10
!                       2
!Error: Label at (1) is not in the same block as the GOTO statement at (2)
!                                        Paul Thomas 18/11/04
!
  integer              ::    i , N = 2
  do i = 1 , N
    print * , 'i =  ', i
    if ( i > 1 ) goto 10                        !!jump into the if block *sigh*
    if ( i > 0 ) then
       print * , 'first statement in if block'
 10    continue
       print * , 'continue within if block'
    end if
    print * , 'out of if block'
  end do
  stop
end program lau_error
Comment 1 Andrew Pinski 2004-11-18 12:31:32 UTC
Confirmed.
Comment 2 Paul Thomas 2005-01-06 18:17:06 UTC
Subject: Re:  Jumping into blocks gives error rather than warning

Hi Tobias,

I just received a bunch of messages about gfortran bugs for which nothing 
has changed, as far as I can tell.  Is there a reason for this?  Am I 
supposed to affirm that they are still bugs?

Happy New Year to you all

Paul Thomas 


Comment 3 Andrew Pinski 2005-01-06 18:19:13 UTC
(In reply to comment #2)
> Subject: Re:  Jumping into blocks gives error rather than warning
> 
> Hi Tobias,
> 
> I just received a bunch of messages about gfortran bugs for which nothing 
> has changed, as far as I can tell.  Is there a reason for this?  Am I 
> supposed to affirm that they are still bugs?

No, Tobias just created a meta bug which records the failures in gfortran which worked in g77.

Thanks,
Andrew
Comment 4 Andrew Pinski 2006-01-07 00:16:22 UTC
*** Bug 25705 has been marked as a duplicate of this bug. ***
Comment 5 Steven Bosscher 2006-01-08 21:30:00 UTC
Created attachment 10595 [details]
allow jumping into blocks in legacy mode

Something like this is probably all that's needed.
Comment 6 Steven Bosscher 2006-01-08 21:32:55 UTC
Comment on attachment 10595 [details]
allow jumping into blocks in legacy mode

Index: resolve.c
===================================================================
--- resolve.c   (revision 109449)
+++ resolve.c   (working copy)
@@ -3579,9 +3579,12 @@ resolve_branch (gfc_st_label * label, gf

   if (found == NULL)
     {
-      /* still nothing, so illegal.  */
-      gfc_error_now ("Label at %L is not in the same block as the "
-                    "GOTO statement at %L", &lp->where, &code->loc);
+      /* Still nothing, so strictly illegal.  However, this kind of
+        what is now considered gross coding style was common in the
+        past.  So allow it for legacy code.  */
+      gfc_notify_std (GFC_STD_LEGACY,
+                     "Label at %L is not in the same block as the "
+                     "GOTO statement at %L", &lp->where, &code->loc);
       return;
     }
Comment 7 Steven Bosscher 2006-01-08 21:33:51 UTC
Comment on attachment 10595 [details]
allow jumping into blocks in legacy mode

See comment #6
Comment 8 Tobias Schlüter 2006-01-08 21:42:22 UTC
No, this is not sufficient, because you'll still need to find the label, unless we have some gross code duplication that I'm not aware of.  What needs to be done is adding a search through the entire program unit if no matching label is found and GFC_STD_LEGACY is allowed.  Since this will take much longer than the original parent block only search, we should really only do this, if need be.  I think we also have a bug about slow code with lots of jumps, so it might probably be worthwhile to overhaul this whole thing, to allow searching for statement labels without having to traverse the gfc_code tree.
Comment 9 Steven Bosscher 2006-01-08 21:45:43 UTC
Actually we already know for sure that the label exists and that it is a valid jump target.  From resolve_branch:

  /* Step one: is this a valid branching target?  */

  if (lp->defined == ST_LABEL_UNKNOWN)
    {
      gfc_error ("Label %d referenced at %L is never defined", lp->value,
                 &lp->where);
      return;
    }

  if (lp->defined != ST_LABEL_TARGET)
    {
      gfc_error ("Statement at %L is not a valid branch target statement "
                 "for the branch statement at %L", &lp->where, &code->loc);
      return;
    }

  /* Step two: make sure this branch is not a branch to itself ;-)  */

  if (code->here == label)
    {
      gfc_warning ("Branch at %L causes an infinite loop", &code->loc);
      return;
    }

  /* Step three: Try to find the label in the parse tree. To do this,
     we traverse the tree block-by-block: first the block that
     contains this GOTO, then the block that it is nested in, etc.  We
     can ignore other blocks because branching into another block is
     not allowed.  */

Step three is what we can skip if we want to allow this kind of invalid jumping around through a program unit.

Comment 10 Steven Bosscher 2006-01-08 21:54:11 UTC
Note that this code in resolve_branch is only slow for deeply nested programs with many gotos.  The code in resolve_branch is linear in the size of the program, but if your program has many GOTO statements, say of the same order of magnitude as the size of the program, then you effectively get quadratic behavior in resolve_namespace.

I had the following idea to speed things up:
1. attach to each block a bitmap of statement labels defined in that block
2. compute a closure over the blocks tree to obtain a bitmap of all statement
   labels reachable from that block
3. in resolve_branch, only look at that bitmap.

You would only have to do 2. once for each program unit.  The bitmap check would be cheap obviously.  You'd never have to walk the statements in resolve_branch, so you would make the quadratic behavior go away.  Unfortunately I've never found the time to try to implement this idea.
Comment 11 Tobias Schlüter 2006-01-08 22:16:03 UTC
Subject: Re:  Jumping into blocks gives error rather than
 warning

steven at gcc dot gnu dot org wrote:
> ------- Comment #9 from steven at gcc dot gnu dot org  2006-01-08 21:45 -------
> Actually we already know for sure that the label exists and that it is a valid
> jump target.  From resolve_branch:

Indeed, I had missed that we already keep track of all existing labels when
parsing via the code in gfc_get_st_label.  I'll give your code a spin and
submit it, when it passes testing.  Ok?
Comment 12 Steven Bosscher 2006-01-08 22:23:33 UTC
Yes please.

And what do you think of the other idea to speed things up?
Comment 13 Tobias Schlüter 2006-01-08 22:53:29 UTC
Subject: Re:  Jumping into blocks gives error rather than
 warning

steven at gcc dot gnu dot org wrote:
> ------- Comment #12 from steven at gcc dot gnu dot org  2006-01-08 22:23 -------
> Yes please.
> 
> And what do you think of the other idea to speed things up?

I know nothing about bitmaps :-)  I've just finished up coding a version which
puts them into a balanced binary tree, this should fix one quadratic bottleneck.
Comment 14 Tobias Schlüter 2006-01-09 16:46:26 UTC
Coming to think of it, I think that while your speedup would work, it would probably be easier and even faster if we kept track of the enclosing blocks while building the blocks and labels, so that the data structure would look something like this:

gfc_st_label *label -> gfc_code *block -> gfc_code *enclosing_block ...

or maybe

gfc_st_label *label -> gfc_code *statement -> gfc_code *block
        -> gfc_code *enclosing_block ...

(where label is a statement label, statement the statement its attached to, block the block containing the statement [this would e.g. be an if], etc.)

Then the time taken by the validation of the blocks would still scale linearly with the number of blocks, but the length of the blocks would no longer play a role, leaving us with linear behavior. 
Comment 15 Tobias Schlüter 2006-01-09 16:50:05 UTC
Forgot to say: the validity check would then look like:

for (b = code->block: b != NULL; b = b->enclosing_block)
  if (b == label->block)
     /* valid GOTO */
Comment 16 Steven Bosscher 2006-01-09 18:35:44 UTC
The idea of comment #14 and #15 looks better than mine, yes.

Which bug is the slowness bug btw?  We should really be discussing solutions for that bug in the audit trail of that bug instead of this one ;-)
Comment 17 Tobias Schlüter 2006-01-09 18:56:04 UTC
(In reply to comment #16)
> The idea of comment #14 and #15 looks better than mine, yes.

I'll continue what I've started, then.

> Which bug is the slowness bug btw?  We should really be discussing solutions
> for that bug in the audit trail of that bug instead of this one ;-)

PR18937.  I'll have it point to the discussion here.



Comment 18 Tobias Schlüter 2006-01-18 20:54:52 UTC
Subject: Bug 18540

Author: tobi
Date: Wed Jan 18 20:54:49 2006
New Revision: 109914

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=109914
Log:
PR fortran/18540
PR fortran/18937
* gfortran.h (BBT_HEADER): Move definition up.
(gfc_st_label): Add BBT_HEADER, remove 'prev' and 'next'.
* io.c (format_asterisk): Adapt initializer.
* resolve.c (resolve_branch): Allow FORTRAN 66 cross-block GOTOs
as extension.
* symbol.c (compare_st_labels): New function.
(gfc_free_st_label, free_st_labels, gfc_get_st_label): Convert to
using balanced binary tree.
* decl.c (match_char_length, gfc_match_old_kind_spec): Do away
with 'cnt'.
(warn_unused_label): Adapt to binary tree.
* match.c (gfc_match_small_literal_int): Only set cnt if non-NULL.
* primary.c (match_kind_param): Do away with cnt.

Also converted the ChangeLog to use latin1 characters.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/decl.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/io.c
    trunk/gcc/fortran/match.c
    trunk/gcc/fortran/primary.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/symbol.c

Comment 19 Tobias Schlüter 2006-01-18 21:08:43 UTC
Fixed on the mainline.  I will backport the cross-jumping part to 4.1.
Comment 20 Tobias Schlüter 2006-01-25 22:34:20 UTC
Subject: Bug 18540

Author: tobi
Date: Wed Jan 25 22:34:17 2006
New Revision: 110228

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110228
Log:
PR fortran/18540
* gfortran.dg/goto_1.f: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/goto_1.f
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 21 Tobias Schlüter 2006-01-25 23:38:40 UTC
Subject: Bug 18540

Author: tobi
Date: Wed Jan 25 23:38:34 2006
New Revision: 110232

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110232
Log:
backport from r109914
fortran/
PR fortran/18540
* resolve.c (resolve_branch): Allow FORTRAN 66 cross-block GOTOs
as extension.

testsuite/
PR fortran/18540
* gfortran.dg/goto_1.f: New.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/goto_1.f
      - copied unchanged from r110228, trunk/gcc/testsuite/gfortran.dg/goto_1.f
Modified:
    branches/gcc-4_1-branch/gcc/fortran/ChangeLog
    branches/gcc-4_1-branch/gcc/fortran/resolve.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 22 Tobias Schlüter 2006-01-26 10:14:57 UTC
Fixed on trunk and 4.1.