Bug 41403 - [4.3/4.4 Regression] miscompilation of goto/label using code
Summary: [4.3/4.4 Regression] miscompilation of goto/label using code
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P2 normal
Target Milestone: 4.5.0
Assignee: Daniel Kraft
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-09-19 05:42 UTC by Jerry DeLisle
Modified: 2009-10-05 13:26 UTC (History)
3 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target:
Build:
Known to work: 3.3.3 4.5.0
Known to fail: 4.1.2 4.3.1 4.4.2
Last reconfirmed: 2009-10-02 16:26:32


Attachments
NIST test case that is failing (3.47 KB, text/plain)
2009-09-19 05:44 UTC, Jerry DeLisle
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jerry DeLisle 2009-09-19 05:42:31 UTC
This test case passes with no optimization. It hangs in a loop with -O1, -O2, or -O3.

Fails also on 4.3, and 4.4. Attachment to follow.
Comment 1 Jerry DeLisle 2009-09-19 05:44:01 UTC
Created attachment 18605 [details]
NIST test case that is failing

I will try to reduce this.
Comment 2 Jerry DeLisle 2009-09-19 05:47:11 UTC
Its ugly code so remember to compile with -w and save the attachment as a .f file so that fixed form is used. This will turn off all of the warnings.
Comment 3 Jerry DeLisle 2009-09-19 06:15:05 UTC
Reduced case:

Expected result:

$ ./a.out 
                    0 ERRORS ENCOUNTERED

Wrong result:

$ ./a.out 
                    1 ERRORS ENCOUNTERED

      PROGRAM FM013
      I01 = 5                                                           
      I02 = 6                                                           
      IVPASS=0                                                          
      IVFAIL=0                                                          
      IVDELE=0                                                          
      ICZERO=0                                                          
      IVTNUM = 126                                                      
C                                                                       
      IF (ICZERO) 31260, 1260, 31260                                    
 1260 CONTINUE                                                          
      ASSIGN 1263 TO I                                                  
      GO TO I, (1262,1263,1264)                                         
 1262 ICON01 = 1262                                                     
      GO TO 1265                                                        
 1263 ICON01 = 1263                                                     
      GO TO 1265                                                        
 1264 ICON01 = 1264                                                     
 1265 CONTINUE                                                          
      GO TO 41260                                                       
31260 IVDELE = IVDELE + 1                                               
      IF (ICZERO) 41260, 1271, 41260                                    
41260 IF ( ICON01 - 1263 )  21260, 11260, 21260                         
11260 IVPASS = IVPASS + 1                                               
      GO TO 1271                                                        
21260 IVFAIL = IVFAIL + 1                                               
      IVCOMP=ICON01                                                     
      IVCORR = 1263                                                     
 1271 CONTINUE                                                          
      IVTNUM = 127                                                      
99999 CONTINUE                                                          
      WRITE (I02,90008)  IVFAIL                                         
      STOP                                                              
90008 FORMAT (" ",15X,I5," ERRORS ENCOUNTERED" )                        
      END                                                               
Comment 4 Joost VandeVondele 2009-09-19 18:27:18 UTC
further reduced, looks more like a middle end issue to me:

      IVFAIL=0
      ASSIGN 1263 TO I
      GO TO I, (1262,1263,1264)
 1262 ICON01 = 1262
      GO TO 1265
 1263 ICON01 = 1263
      GO TO 1265
 1264 ICON01 = 1264
 1265 CONTINUE
41260 IF ( ICON01 - 1263 )  21260, 11260, 21260
11260 IVPASS = IVPASS + 1
      GO TO 1271
21260 IVFAIL = IVFAIL + 1
 1271 CONTINUE
      WRITE (6,*)  IVFAIL
      END
Comment 5 Joost VandeVondele 2009-09-19 18:48:40 UTC
and also fails in C

#include <stdio.h>
main()
{
  int i;
  int i0;
  void * i1;
  int icon01;
  int ivfail;
  int ivpass;
  int D1545;

  ivfail = 0;
  i0 = -1;
  i1 = &&label_001263;
  if (i1 == &&label_001262) goto label_001262;
  if (i1 == &&label_001263) goto label_001263;
  if (i1 == &&label_001264) goto label_001264;
  label_001262:
  icon01 = 1262;
  goto label_001265;
  label_001263:
  icon01 = 1263;
  goto label_001265;
  label_001264:
  icon01 = 1264;
  label_001265:
  label_041260:

    D1545 = icon01 + -1263;
    if (D1545 != 0) goto label_021260; else goto label_011260;
  label_011260:;
  ivpass = ivpass + 1;
  goto label_001271;
  label_021260:
  ivfail = ivfail + 1;
  label_001271:
  printf("%d\n",ivfail);
}
Comment 6 Joost VandeVondele 2009-09-19 18:54:15 UTC
and smaller C testcase:
main()
{
  void * i1;
  int icon01;
  int ivfail;
  int D1545;
  ivfail = 0;
  i1 = &&label_001263;
  if (i1 == &&label_001262) goto label_001262;
  if (i1 == &&label_001263) goto label_001263;
  label_001262:
  icon01 = 1262;
  goto label_001265;
  label_001263:
  icon01 = 1263;
  label_001265:
    D1545 = icon01 + -1263;
    if (D1545 != 0) goto label_021260; else goto label_011260;
  label_011260:;
  goto label_001271;
  label_021260:
  ivfail = ivfail + 1;
  label_001271:
  printf("%d\n",ivfail);
}

Comment 7 Joost VandeVondele 2009-09-20 13:18:23 UTC
Not easy to find a machine that old, but it works with 3.3.3 and it fails already with 4.1.2. Pretty old regression thus [tested with the C code].
Comment 8 Richard Biener 2009-09-20 14:05:07 UTC
On the tree level we end up with the correct (but unfortunately unfolded)

main ()
{
  int icon01;

<bb 2>:
  if (&label_001263 == &label_001262)
    goto <bb 5> (label_001265);
  else
    goto <bb 3> (label_001262);

label_001262:
  if (&label_001263 == &label_001263)
    goto <bb 5> (label_001265);
  else
    goto <bb 4> (label_001263);

label_001263:

  # icon01_1 = PHI <1262(4), 1263(3), 1262(2)>
label_001265:

label_001271:
  if (icon01_1 != 1263)
    goto <bb 7>;
  else
    goto <bb 8>;

<bb 7>:
  abort ();

<bb 8>:
  return 0;


Note that comparing addresses of labels is inherently fragile as they may
collapse to a single location (like it happens here):

main:
        pushl   %ebp
        movl    %esp, %ebp
        andl    $-16, %esp
.L4:
.L3:
.L2:
.L5:
        movl    $.L3, %eax
        cmpl    $.L4, %eax
        jne     .L6
        call    abort
.L6:
        movl    $0, %eax
        movl    %ebp, %esp
        popl    %ebp
        ret

forcing addresses of non-equivalent labels to compare non-equal would be
a way out here, but I think the Fortran frontend relies on a
fragile area of label address comparisons - labels are supposed to be
jumped to only.

That is, the Frontend presents us with

  i.0 = -2;
  ivfail = 0;
  i.0 = -1;
  i.1 = &__label_001263;
  if ((logical(kind=4)) __builtin_expect (i.0 != -1, 0))
    {
      _gfortran_runtime_error_at (&"At line 3 of file t.f"[1]{lb: 1 sz: 1}, &"Assigned label is not a target label"[1]{lb: 1 sz: 1});
    }
  if (i.1 == &__label_001262) goto __label_001262;
  if (i.1 == &__label_001263) goto __label_001263;
  if (i.1 == &__label_001264) goto __label_001264;
...

but instead it should have used a computed goto, like

  C.0 = { &__label_001262, &__label_001263, &__label_001264 };
  goto *C.0[i - 1262];

for assigned goto.

Thus, this is a frontend issue with assigned goto (a deleted feature btw).
Comment 9 Joost VandeVondele 2009-09-20 14:18:40 UTC
(In reply to comment #8)
> Thus, this is a frontend issue with assigned goto (a deleted feature btw).

so just for my curiosity, is the C code thus invalid?


Comment 10 Richard Biener 2009-09-20 14:44:22 UTC
No, the C code is valid, but it's results depend on optimization level
(just like if you would compare the addresses of stack locals).
Comment 11 Jerry DeLisle 2009-09-20 17:04:31 UTC
Subject: Re:  [4.3/4.4/4.5 Regression] miscompilation of
 goto/label using code

rguenth at gcc dot gnu dot org wrote:
> but instead it should have used a computed goto, like
> 
>   C.0 = { &__label_001262, &__label_001263, &__label_001264 };
>   goto *C.0[i - 1262];
> 
> for assigned goto.
> 
> Thus, this is a frontend issue with assigned goto (a deleted feature btw).
> 

I was confused by the parenthetical statement.

Is this:

C.0 = { &__label_001262, &__label_001263, &__label_001264 };
	goto *C.0[i - 1262];

OK to use or not?  Computed goto in Fortran is a legacy feature we must support.

Comment 12 Richard Biener 2009-09-20 17:09:45 UTC
Yes that's ok.  What is not ok is to compare addresses of labels and to rely
on different labels having different addresses.
Comment 13 Daniel Kraft 2009-10-02 16:26:32 UTC
I'll work on this.
Comment 14 Daniel Kraft 2009-10-03 14:13:40 UTC
Here's a patch and some comments for this:
http://gcc.gnu.org/ml/fortran/2009-10/msg00017.html
Comment 15 Daniel Kraft 2009-10-05 13:15:53 UTC
Subject: Bug 41403

Author: domob
Date: Mon Oct  5 13:15:35 2009
New Revision: 152448

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=152448
Log:
2009-10-05  Daniel Kraft  <d@domob.eu>

	PR fortran/41403
	* trans-stmt.c (gfc_trans_goto): Ignore statement list on assigned goto
	if it is present.

2009-10-05  Daniel Kraft  <d@domob.eu>

	PR fortran/41403
	* gfortran.dg/goto_6.f: New test.
	* gfortran.dg/goto_7.f: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/goto_6.f
    trunk/gcc/testsuite/gfortran.dg/goto_7.f
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/testsuite/ChangeLog

Comment 16 Daniel Kraft 2009-10-05 13:19:38 UTC
Fixed on trunk.  I won't backport, as this is no real regression.

I still volunteer to rework the assigned/computed goto implementation (and have some ideas for that) in case we deem it worth the effort, but as both are deleted/obsolete features I don't think we should go for it.