Bug 42742 - [4.5 Regression] Handle very large format strings correctly
Summary: [4.5 Regression] Handle very large format strings correctly
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.5.0
: P4 normal
Target Milestone: 4.5.0
Assignee: Jerry DeLisle
URL:
Keywords: wrong-code
: 42744 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-01-14 10:27 UTC by Manfred Schwarb
Modified: 2010-02-23 16:41 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.5 4.4.3
Known to fail: 4.5.0
Last reconfirmed: 2010-01-14 13:06:51


Attachments
test case 2 (343 bytes, text/plain)
2010-01-16 18:13 UTC, Manfred Schwarb
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Manfred Schwarb 2010-01-14 10:27:40 UTC
In a larger program of mine, I got a SIGSEGV when printing

      WRITE(*,fmtstr)
     &           dat(1),"-",dat(2),"-",dat(3),
     &           datedelim,dat(4),":",dat(5),
     &           (delim,bufarr(pindx),pindx=1,anzarg2)

with gfortran 4.5 (latest snapshot). Using gfortran 4.3.5 the code runs fine.
Unfortunately, I can not reproduce the crash in a standalone program.

In the above print statement, fmtstr is rather large (~550chars).

Here is the gdb output:

Program received signal SIGSEGV, Segmentation fault.
reset_node (fn=0xc) at ../../../gfortran-source/gcc-4.5-20100107/libgfortran/io/format.c:111
111       if (fn->format != FMT_LPAREN)
(gdb) where
#0  reset_node (fn=0xc) at ../../../gfortran-source/gcc-4.5-20100107/libgfortran/io/format.c:111
#1  0x0000000000423b30 in reset_fnode_counters ()
    at ../../../gfortran-source/gcc-4.5-20100107/libgfortran/io/format.c:134
#2  parse_format () at ../../../gfortran-source/gcc-4.5-20100107/libgfortran/io/format.c:1233
#3  0x0000000000418208 in data_transfer_init (dtp=0x7ffffffead80, read_flag=0)
    at ../../../gfortran-source/gcc-4.5-20100107/libgfortran/io/transfer.c:2182
#4  0x000000000040ba86 in writevals (startjul=<value optimized out>, endjul=<value optimized out>,
    reqts=<value optimized out>, basets=<value optimized out>, use_monthtu=<value optimized out>,
    station=<value optimized out>, param=<value optimized out>, aggrtypes=<value optimized out>,
    parfmt2=<value optimized out>, valarr2=<value optimized out>, datefmt=<value optimized out>,
    delim=<value optimized out>, missing=<value optimized out>, anzarg2=<value optimized out>,
    anzval=<value optimized out>, toffset=<value optimized out>, qualarr=<value optimized out>,
    wantqf=<value optimized out>, _station=<value optimized out>, _param=<value optimized out>,
    _aggrtypes=<value optimized out>, _parfmt2=<value optimized out>,
    _delim=<value optimized out>, _missing=<value optimized out>) at extractdata.f:1464
#5  0x0000000000407818 in extractdata () at extractdata.f:58
#6  0x0000000000407906 in main (argc=<value optimized out>, argv=<value optimized out>)
    at extractdata.f:1179
#7  0x000000000042b406 in __libc_start_main (main=<value optimized out>,
    argc=<value optimized out>, ubp_av=<value optimized out>, init=0x42b8b0 <__libc_csu_init>,
    fini=0x42b870 <__libc_csu_fini>, rtld_fini=0, stack_end=0x7fffffffdf98) at libc-start.c:220
#8  0x0000000000400209 in _start () at ../sysdeps/x86_64/elf/start.S:113
(gdb) list
106       fnode *f;
107
108       fn->count = 0;
109       fn->current = NULL;
110
111       if (fn->format != FMT_LPAREN)
112         return;
113
114       for (f = fn->u.child; f; f = f->next)
115         {


Hopefully this gives already some clues. Otherwise please tell me 
how to debug further.
Comment 1 Tobias Burnus 2010-01-14 11:30:50 UTC
Hmm, without test case, it is quite difficult to debug. Though, maybe Jerry gets an idea from the backtrace.

Can't you write out "fmtstr" & create a program with "fmtstr" set such, set anzarg2 and call the same WRITE statement? Even if it does not work, knowing the "fmtstr" might help -- I could imaging the it has something to do with the format-cache patch - in that case also 4.4 would be affected.

I think, at the end we really need a small testcase - either by reducing the big program or by dreaming up a new test case based on the clues one gets from backtracing & looking at the source.

Reducing for run-time problems is unfortunately not as easy as for internal compiler errors (for which http://gcc.gnu.org/wiki/A_guide_to_testcase_reduction helps a lot, though it also works for run-time problems.)
Comment 2 Manfred Schwarb 2010-01-14 12:59:51 UTC
*** Bug 42744 has been marked as a duplicate of this bug. ***
Comment 3 Manfred Schwarb 2010-01-14 13:01:13 UTC
See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42744
for comments and test case.

Sorry.
Comment 4 Jerry DeLisle 2010-01-14 13:06:50 UTC
I will take this one.
Comment 5 Jerry DeLisle 2010-01-14 13:23:16 UTC
This is related to format caching and the length of the format string.

Something like this:

      fmtstr="(i4,4(a1,i2.2),10(a1,a12))"

Is a work around until I fix this, unless of course the test case is contrived.
Comment 6 Manfred Schwarb 2010-01-14 13:44:38 UTC
fmtstr is put together at runtime, each column may (and actually does
sometimes) have different width (minimal width to save space), so
- no, your work around does not work for me
- no, this example is not contrived
- in some invocations of my program I do have ~1000 columns (instead of 74
  as in this example), so fmtstr may get huge. Well, this sounds
  a bit silly, I know. But it worked so far and is well in line with
  the GNU "no limits" credo ;-)
Comment 7 Jerry DeLisle 2010-01-14 19:30:19 UTC
By contrived I meant - made to look like your real code, but not necessarily your real code. I want to make sure "contrived" is not inetrpreted here as negative in any way.

At least conceptually one could compute the format string differently.

Regardless, its still a bug and I still plan to fix it short of getting completely stuck for some reason.  :)
Comment 8 Tobias Burnus 2010-01-15 15:40:26 UTC
Submitted: Patch by Jerry:
  http://gcc.gnu.org/ml/fortran/2010-01/msg00091.html
  "It avoids the excessive use of memory that results from the parsed format
   node tree, probably exceeding available stack or heap."
Comment 9 Jerry DeLisle 2010-01-15 15:58:58 UTC
Sending        ChangeLog
Sending        io/format.c
Transmitting file data ..
Committed revision 155939.

This will be back ported to 4.4.4 in a few days.
Comment 10 Manfred Schwarb 2010-01-16 18:13:53 UTC
Created attachment 19625 [details]
test case 2
Comment 11 Manfred Schwarb 2010-01-16 18:35:40 UTC
With test case 2, I get

> ./writebug2 > writebug2.txt
 Interation            1 :           25
 Interation            2 :           32
 Interation            3 :           39
 Interation            4 :           46
 Interation            5 :           53
 Interation            6 :           60
 Interation            7 :           67
 Interation            8 :           74
 Interation            9 :           81
 Interation           10 :           88
 Interation           11 :           95
 Interation           12 :          102
 Interation           13 :          109
 Interation           14 :          116
 Interation           15 :          123
 Interation           16 :          130
 Interation           17 :          137
 Interation           18 :          144
 Interation           19 :          151
 Interation           20 :          158
 Interation           21 :          165
 Interation           22 :          172
 Interation           23 :          179
 Interation           24 :          186
 Interation           25 :          193
 Interation           26 :          200
 Interation           27 :          207
 Interation           28 :          214
 Interation           29 :          221
 Interation           30 :          228
Segmentation fault

It stops at length 228 after the 5th iteration of the inner loop.
I tried 32bit and 64bit, no difference.
Also, varying values of "ulimit -s" and "-fmax-stack-var-size"
made no difference.

Maybe your FORMAT_CACHE_STRING_LIMIT is not the same thing as the
user space format length, so I can not judge your patch.

It seems however, that the bug is not stack size dependent. And
my box has more than enough memory. So I'm not completely convinced
that the issue is a stack or heap size limitation.

I further noticed, that the reached last iteration of the inner loop
depends on the loop stop value. If I do "DO i=1,100", then the inner loop
stops after iteration 85 (but for the same j=30).
Comment 12 Manfred Schwarb 2010-01-16 21:43:35 UTC
To clarify, this was still with the unpatched gfortran 4.5.0,
snapshot of 20100107.

BTW, the silly, stray line "anzarg2=j" in writebug2.f 
does not alter the result.
Comment 13 Manfred Schwarb 2010-01-16 23:48:00 UTC
I now built gfortran 4.5.0 (20100107) + Jerry's patch.

Patch works for me, no SIGSEGV any more. Thanks.
Comment 14 Jerry DeLisle 2010-01-22 04:26:21 UTC
Fixed for now, changing summary to reflect the current situation.  I want to leave open until I have time to investigate further.
Comment 15 Tobias Burnus 2010-01-22 08:19:59 UTC
(In reply to comment #14)
> Fixed for now, changing summary to reflect the current situation.  I want to
> leave open until I have time to investigate further.

? The PR is now closed (as you wrote in the first sentence), but in the second you write that you want to leave it open? Do you want to open another PR? Or reopen this PR?
Comment 16 Tobias Burnus 2010-01-23 16:57:51 UTC
Reopen the bug. It seems as if there a deeper issue in the caching, which should be investigated. As the committed patch (limiting fmt-cache size) fixes the issue for all known test cases, I have removed the dependency on PR 32834.
Comment 17 Jerry DeLisle 2010-02-07 00:08:03 UTC
I have found some time and managed to isolate the problem. It is in the allocation of new format nodes after the fnode_array is exhausted. During format caching, reset_fnode relies on a NULL to break out of a loop.  The pointer used comes out of get_fnode not initialized to NULL and the segfault ensues.  I am working on the proper patch.
Comment 18 Jerry DeLisle 2010-02-07 07:46:06 UTC
Subject: Bug 42742

Author: jvdelisle
Date: Sun Feb  7 07:45:55 2010
New Revision: 156568

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156568
Log:
2010-02-06  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/42742
	* io/format.c (reset_fnode_counters): Use the correct pointer to the
	head of the fnode list. (parse_format): Remove previous hack that set
	limit on size of format string for caching.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/format.c

Comment 19 Jerry DeLisle 2010-02-07 07:50:28 UTC
Subject: Bug 42742

Author: jvdelisle
Date: Sun Feb  7 07:50:17 2010
New Revision: 156569

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156569
Log:
2010-02-06  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/42742
	* gfortran.dg/fmt_cache_2.f: New test.

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

Comment 20 Jerry DeLisle 2010-02-07 08:00:10 UTC
Fixed on trunk.  Will back port to 4.4 in a few days.
Comment 21 Jerry DeLisle 2010-02-07 13:37:21 UTC
Closing, not needed on 4.4
Comment 22 hjl@gcc.gnu.org 2010-02-23 17:04:31 UTC
Subject: Bug 42742

Author: hjl
Date: Tue Feb 23 17:02:26 2010
New Revision: 157010

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157010
Log:
Backport testcases from mainline.

2010-02-23  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2010-02-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42749
	* gcc.c-torture/compile/pr42749.c: New testcase.

	2010-02-21  Dodji Seketeli  <dodji@redhat.com>

	PR c++/42824
	* g++.dg/template/memclass4.C: New test.

	2010-02-20  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43111
	* gfortran.dg/internal_pack_8.f90: New test.

	2010-02-18  Jason Merrill  <jason@redhat.com>

	PR c++/43109
	* g++.dg/parse/namespace12.C: New.

	2010-02-18  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/43066
	* gcc.c-torture/compile/pr43066.c: New test.

	2010-02-17  Jason Merrill  <jason@redhat.com>

	PR c++/43069
	* g++.dg/parse/namespace11.C: New.

	PR c++/43093
	* g++.dg/ext/attrib37.C: New.

	PR c++/43079
	* g++.dg/template/ptrmem20.C: New.

	2010-02-16  Jason Merrill  <jason@redhat.com>

	PR c++/43031
	* g++.dg/ext/attrib36.C: New.

	2010-02-15  Richard Guenther  <rguenther@suse.de>

	PR middle-end/43068
	* g++.dg/torture/pr43068.C: New testcase.

	2010-02-11  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/42998
	* gcc.c-torture/compile/pr42998.c: New testcase.

	2010-02-10  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43017
	* gcc.dg/torture/pr43017.c: New testcase.

	2010-02-10  Richard Guenther  <rguenther@suse.de>

	PR c/43007
	* gcc.c-torture/execute/20100209-1.c: New testcase.
	* gcc.dg/fold-div-3.c: Likewise.

	2010-02-09  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/42999
	* gfortran.dg/array_constructor_35.f90: New test.

	2010-02-09  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43008
	* gcc.c-torture/execute/pr43008.c: New testcase.

	2010-02-09  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43000
	* gcc.dg/torture/pr43000.c: New testcase.
	* gcc.dg/torture/pr43002.c: Likewise.

	2010-02-06  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/42742
	* gfortran.dg/fmt_cache_2.f: New test.

	2010-02-03  Jason Merrill  <jason@redhat.com>

	PR c++/42870
	* g++.dg/ext/dllexport3.C: New.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/attrib36.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/ext/attrib36.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/attrib37.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/ext/attrib37.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/dllexport3.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/ext/dllexport3.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/parse/namespace11.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/parse/namespace11.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/parse/namespace12.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/parse/namespace12.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/template/memclass4.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/template/memclass4.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/template/ptrmem20.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/template/ptrmem20.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr43068.C
      - copied unchanged from r157009, trunk/gcc/testsuite/g++.dg/torture/pr43068.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42749.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.c-torture/compile/pr42749.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr42998.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.c-torture/compile/pr42998.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr43066.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.c-torture/compile/pr43066.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/20100209-1.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.c-torture/execute/20100209-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr43008.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.c-torture/execute/pr43008.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/fold-div-3.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.dg/fold-div-3.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/torture/pr43000.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.dg/torture/pr43000.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/torture/pr43002.c
      - copied unchanged from r157009, trunk/gcc/testsuite/gcc.dg/torture/pr43002.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/array_constructor_35.f90
      - copied unchanged from r157009, trunk/gcc/testsuite/gfortran.dg/array_constructor_35.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/fmt_cache_2.f
      - copied unchanged from r157009, trunk/gcc/testsuite/gfortran.dg/fmt_cache_2.f
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/internal_pack_8.f90
      - copied unchanged from r157009, trunk/gcc/testsuite/gfortran.dg/internal_pack_8.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog