Bug 26801 - -fbounds-check generates segfault
Summary: -fbounds-check generates segfault
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.1.2
Assignee: Francois-Xavier Coudert
URL:
Keywords: patch, wrong-code
Depends on:
Blocks: Fortran_bounds_checking
  Show dependency treegraph
 
Reported: 2006-03-22 12:32 UTC by Salvatore Filippone
Modified: 2006-07-02 21:22 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.0 4.1.2
Known to fail:
Last reconfirmed: 2006-06-16 19:31:10


Attachments
test case (186 bytes, text/plain)
2006-03-22 12:34 UTC, Salvatore Filippone
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Salvatore Filippone 2006-03-22 12:32:22 UTC
Attached program generates a segfault when compiled with bounds-check enabled. 
There is nothing strange in the code that I can see (though of course I might be proven wrong).

[sfilippo@localhost sfilippo]$ gfortran -v 
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.2-20060318/configure --prefix=/usr/local/gfortran
Thread model: posix
gcc version 4.2.0 20060318 (experimental)
[sfilippo@localhost sfilippo]$ gfortran -o assoc assoc.f90 
[sfilippo@localhost sfilippo]$ ./assoc
 F
 T
 F
 F
 T
 T
[sfilippo@localhost sfilippo]$ gfortran -o assoc assoc.f90 -fbounds-check
[sfilippo@localhost sfilippo]$ ./assoc
 F
 T
Segmentation fault (core dumped)
Comment 1 Salvatore Filippone 2006-03-22 12:34:39 UTC
Created attachment 11094 [details]
test case

Note: the code segfaults even if the first do loop (before allocating the %a components) is commented out.
Comment 2 Andrew Pinski 2006-03-22 21:22:10 UTC
Confirmed, but I don't see could not figure out why the segfault is there.
Comment 3 Francois-Xavier Coudert 2006-05-28 13:23:38 UTC
I tried with the following reduced testcase:

  implicit none

  integer :: i
  logical :: l
  type dt
    integer, pointer :: a => null()
  end type dt
  type(dt), pointer :: obj(:) => null()


  allocate(obj(2))
  i = 1
  l = associated(obj(i)%a)
  print *, l
end


The code generated for the ASSOCIATED statement is the following when bounds-checking is disabled:
  i = 1;
  l = (*(struct dt[0:] *) obj.data)[obj.dim[0].stride * i + obj.offset].a != 0B;

When bounds-checking is enabled, it is changed into:
  i = 1;
  {
    int4 D.924;

    l = (*(struct dt[0:] *) obj.data)[obj.dim[0].stride * D.924 + obj.offset].a != 0B;
  }

Notice that D.924 is used uninitialized. The -fbounds-check options gives a special codepath in three occasions:

0x080ad083 in gfc_conv_ss_startstride (loop=0xbfa1813c)
    at ../../../trunk/gcc/fortran/trans-array.c:2456
2456      if (flag_bounds_check)
(gdb) c
Continuing.
Hardware read watchpoint 1: flag_bounds_check

Value = 1
0x080acc2c in gfc_conv_array_ref (se=0xbfa17ab4, ar=0x87222bc)
    at ../../../trunk/gcc/fortran/trans-array.c:1951
1951          if (flag_bounds_check && ar->as->type != AS_ASSUMED_SIZE)
(gdb) 
Continuing.
Hardware read watchpoint 1: flag_bounds_check

Value = 1
0x080acd5c in gfc_conv_array_ref (se=0xbfa17ab4, ar=0x87222bc)
    at ../../../trunk/gcc/fortran/trans-array.c:1980
1980      if (flag_bounds_check)


I don't understand where this D.924 variable is generated.
Comment 4 Francois-Xavier Coudert 2006-06-07 10:55:24 UTC
The extra variable is generated from the call to gfc_evaluate_now around line 1970 of trans-array.c (the call is "indexse.expr = gfc_evaluate_now (indexse.expr, &se->pre)").

I'm not sure yet if it's the right solution (maybe Andrew, you'd have an idea on that) but evaluating the index to a different tree than the one used later on fixes the segfault.

Index: trans-array.c
===================================================================
--- trans-array.c       (revision 114461)
+++ trans-array.c       (working copy)
@@ -1967,14 +1967,13 @@
          (ar->as->type != AS_ASSUMED_SIZE  || n < ar->dimen - 1))
        {
          /* Check array bounds.  */
-         tree cond;
+         tree cond, i;
          char *msg;
 
-         indexse.expr = gfc_evaluate_now (indexse.expr, &se->pre);
+         i = gfc_evaluate_now (indexse.expr, &se->pre);
 
          tmp = gfc_conv_array_lbound (se->expr, n);
-         cond = fold_build2 (LT_EXPR, boolean_type_node, 
-                             indexse.expr, tmp);
+         cond = fold_build2 (LT_EXPR, boolean_type_node, i, tmp);
          asprintf (&msg, "%s for array '%s', "
                    "lower bound of dimension %d exceeded", gfc_msg_fault,
                    sym->name, n+1);
@@ -1982,8 +1981,7 @@
          gfc_free (msg);
 
          tmp = gfc_conv_array_ubound (se->expr, n);
-         cond = fold_build2 (GT_EXPR, boolean_type_node, 
-                             indexse.expr, tmp);
+         cond = fold_build2 (GT_EXPR, boolean_type_node, i, tmp);
          asprintf (&msg, "%s for array '%s', "
                    "upper bound of dimension %d exceeded", gfc_msg_fault,
                    sym->name, n+1);



I worked with the following reduced testcase:

  integer :: i = 1
  logical :: l
  type dt
    integer, pointer :: a
  end type dt
  type(dt) :: obj(1)

  l = associated(obj(i)%a)
  print *, l
end
Comment 5 Francois-Xavier Coudert 2006-06-07 20:21:52 UTC
The patch I proposed is wrong. We need to call gfc_evaluate_now. The question is to know why, in this precise case, the block of code we're building (se->pre) seems to be dropped later on (both the call that sets the temporary variable and the checks and calls to _gfortran_runtime_error).
Comment 6 Francois-Xavier Coudert 2006-06-16 19:31:10 UTC
I'm currently regtesting the following patch:

Index: trans-intrinsic.c
===================================================================
--- trans-intrinsic.c   (revision 114721)
+++ trans-intrinsic.c   (working copy)
@@ -2840,6 +2840,8 @@
           gfc_conv_expr_lhs (&arg1se, arg1->expr);
           tmp2 = gfc_conv_descriptor_data_get (arg1se.expr);
         }
+      gfc_add_block_to_block (&se->pre, &arg1se.pre);
+      gfc_add_block_to_block (&se->post, &arg1se.post);
       tmp = build2 (NE_EXPR, boolean_type_node, tmp2,
                    fold_convert (TREE_TYPE (tmp2), null_pointer_node));
       se->expr = tmp;
Comment 7 tobias.burnus 2006-06-17 10:57:16 UTC
The test case of comment #4 is invalid as the Fortran standard says that a pointer is undefined unless it is associated (allocated, assigned) or deassociated (nullifyed). In this case it is undefined.
(What gfortran should do in this case is a matter of taste, crashing is completely acceptable.)

However, the test case of comment #3 is valid Fortran 2003, nonetheless gfortran crashes here, which should be fixed (and seems to be fixed by patch in comment 6).
Comment 8 Francois-Xavier Coudert 2006-06-18 17:36:55 UTC
Subject: Bug 26801

Author: fxcoudert
Date: Sun Jun 18 17:36:47 2006
New Revision: 114757

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114757
Log:
	PR fortran/26801

	* trans-intrinsic.c (gfc_conv_associated): Use pre and post blocks
	of the scalarization expression.

	* gfortran.dg/associated_4.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/associated_4.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog

Comment 9 Francois-Xavier Coudert 2006-07-02 21:17:21 UTC
Subject: Bug 26801

Author: fxcoudert
Date: Sun Jul  2 21:17:05 2006
New Revision: 115134

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115134
Log:
	PR fortran/28094
	* trans-intrinsic.c (gfc_conv_intrinsic_mod): Support cases where
	there is no integer kind equal to the resulting real kind.
	* intrinsic.c (add_functions): MODULO is not allowed as an actual
	argument.
	* Makefile.am: Add _mod_r10.F90 and _mod_r16.F90.
	* Makefile.in: Regenerate.
	* generated/_mod_r10.F90: New file.
	* generated/_mod_r16.F90: New file.

	PR fortran/27965
	* trans-array.c (gfc_conv_ss_startstride): Correct the runtime
	conditions for bounds-checking. Check for nonzero stride.
	Don't check the last dimension of assumed-size arrays. Fix the
	dimension displayed in the error message.

	PR fortran/26801
	* trans-intrinsic.c (gfc_conv_associated): Use pre and post blocks
	of the scalarization expression.
	* gfortran.dg/associated_4.f90: New test.

	PR fortran/28081
	* resolve.c (resolve_substring): Don't issue out-of-bounds
	error messages when the range has zero size.
	* gfortran.dg/substr_3.f: New test.
	* gfortran.dg/equiv_2.f90: Update expected error message.

	PR fortran/23862
	* lang-specs.h (f95-cpp-input): Pass -ffree-form to f951 unless
	-ffixed-form is explicitly specified.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/associated_4.f90
      - copied unchanged from r114757, trunk/gcc/testsuite/gfortran.dg/associated_4.f90
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/substr_3.f
      - copied unchanged from r114972, trunk/gcc/testsuite/gfortran.dg/substr_3.f
    branches/gcc-4_1-branch/libgfortran/generated/_mod_r10.F90
      - copied unchanged from r114961, trunk/libgfortran/generated/_mod_r10.F90
    branches/gcc-4_1-branch/libgfortran/generated/_mod_r16.F90
      - copied unchanged from r114961, trunk/libgfortran/generated/_mod_r16.F90
Modified:
    branches/gcc-4_1-branch/gcc/fortran/ChangeLog
    branches/gcc-4_1-branch/gcc/fortran/intrinsic.c
    branches/gcc-4_1-branch/gcc/fortran/lang-specs.h
    branches/gcc-4_1-branch/gcc/fortran/resolve.c
    branches/gcc-4_1-branch/gcc/fortran/trans-array.c
    branches/gcc-4_1-branch/gcc/fortran/trans-intrinsic.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/equiv_2.f90
    branches/gcc-4_1-branch/libgfortran/ChangeLog
    branches/gcc-4_1-branch/libgfortran/Makefile.am
    branches/gcc-4_1-branch/libgfortran/Makefile.in

Comment 10 Francois-Xavier Coudert 2006-07-02 21:22:54 UTC
Fixed.