Bug 26890 - SIZE parameter interacts with same variable in IO list character length specification.
Summary: SIZE parameter interacts with same variable in IO list character length speci...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.1.1
Assignee: Jerry DeLisle
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-27 17:27 UTC by Paul Thomas
Modified: 2006-04-14 03:16 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-04-09 05:30:33


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Thomas 2006-03-27 17:27:39 UTC
The program below produces the correct result for the first read and nothing for the second.  ifort and g95 produce the same result for each.  It is my belief that this construct is not strictly standard compliant but it is widely used - I found this during my efforts to make iso_varying_string work.

The code looks OK, so I think that the logic in the library needs a little bit of tweaking:

  nchars = 80;
  {
    struct __st_parameter_dt dt_parm.8;

    dt_parm.8.common.filename = "advance_io.f90";
    dt_parm.8.common.line = 21;
    dt_parm.8.common.unit = 10;
    dt_parm.8.advance = "no";
    dt_parm.8.advance_len = 2;
    dt_parm.8.format = "(a)";
    dt_parm.8.format_len = 3;
    dt_parm.8.size = &nchars;
    dt_parm.8.common.flags = 13328;
    _gfortran_st_read (&dt_parm.8);
    _gfortran_transfer_character (&dt_parm.8, &buffer, NON_LVALUE_EXPR <nchars>) ;
    _gfortran_st_read_done (&dt_parm.8);
    switch (dt_parm.8.common.flags & 3)
      {
        case 3:;
        goto __label_000999;
      }
  }

Paul

  character(80) :: buffer, line
  integer :: nchars
  line = "The quick brown fox jumps over the lazy dog."
  open (10)
  write (10, '(a)') trim(line)
  rewind (10)
  nchars = 80
  read (10, '(a)', advance = 'no', size = nchars, eor = 998) buffer
998 print *, nchars, buffer
  rewind (10)
  buffer = ""
  nchars = 80
  read (10, '(a)', advance = 'no', size = nchars, eor = 999) buffer (:nchars)
999 print *, nchars, buffer
  close (10)
end
Comment 1 Jerry DeLisle 2006-03-28 01:43:57 UTC
I will look into this if you would like.
Comment 2 Paul Thomas 2006-03-28 04:08:44 UTC
Subject: Re:  SIZE parameter interacts with same variable
 in IO list character length specification.

jvdelisle at gcc dot gnu dot org wrote:

>------- Comment #1 from jvdelisle at gcc dot gnu dot org  2006-03-28 01:43 -------
>I will look into this if you would like.
>  
>
Yes, indeed - you are the expert, which was why I CC'd you....

All the best,

Paul

PS I suppose that the library zeros the variable pointed too by SIZE.  
This then leads to the calculation that the destination has zero length 
and that nothing should be read?  I guess a copy of SIZE is needed to 
work with an that it should be written back on exiting the library?

Comment 3 Jerry DeLisle 2006-03-31 05:11:07 UTC
Subject: Bug 26890

Author: jvdelisle
Date: Fri Mar 31 05:11:03 2006
New Revision: 112570

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112570
Log:
2006-03-30  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26890
	* io/io.h: Add size_used to st_parameter_dt, adjust pad size.
	*io/transfer.c (data_transfer_init): Initialize size_used to zero.
	(read_sf): Use size_used.
	(read_block): Likewise.
	(read_block_direct): Likewise.
	(write_block): Likewise.
	(write_buf): Likewise and eliminate erroneous FAILURE return.
	(finalize_transfer): Assign value of size_used to *dtp->size.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/io.h
    trunk/libgfortran/io/transfer.c

Comment 4 Jerry DeLisle 2006-03-31 05:15:46 UTC
Subject: Bug 26890

Author: jvdelisle
Date: Fri Mar 31 05:15:42 2006
New Revision: 112571

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112571
Log:
2006-03-30  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26890
	* gfortran.dg/read_size_noadvance.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/read_size_noadvance.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 5 Steve Ellcey 2006-04-07 17:02:26 UTC
I believe this patch is causing a bunch of IO failures on ia64-hp-hpux11.23.  Specifically, the setting of "pad" looks bad to me.  pad, in this case, is not a padding on the end of the structure but a parallel array of chars that is in union u and should be the same size as the structure p and using the same space as the structure p.  I think the fix is to increase the size of pad, not decrease it.  I will do some testing and see if I am right.
Comment 6 Jerry DeLisle 2006-04-07 18:42:27 UTC
We posted a question about that on the list.  Not knowing the purpose of pad, I took a guess.  Let me know what you learn.  Appreciate your testing this and reporting.
Comment 7 Jerry DeLisle 2006-04-07 18:50:04 UTC
Another thought.  If comment #5 about size of p is true, why not set pad to sizeof(p)?  Why was it so oddly constructed in the first place?
Comment 8 Steve Ellcey 2006-04-07 18:55:00 UTC
I am not sure why it was created the way it was or why it doesn't just use sizeof(p).  I haven't looked back into SVN/email to see if there are any comments on why it was done the way it was done.  Do you know where structures of type st_parameter_dt are allocated?  I looked quickly but didn't see it.
Comment 9 Jerry DeLisle 2006-04-07 19:17:28 UTC
st_parameter_dt are declared by the front end which then passes to the library functions.
Comment 10 Jerry DeLisle 2006-04-07 22:23:04 UTC
I put some prints at the top of get_unit in unit.c to take a look.  After putting the padding back where it was originally in io.h:

The DTP
Size of p: 132
Size of pad: 200
Size of char *: 4
Size if int: 4

This is OK on i686.  pad needs to be >= p

The original padding in io.h should be:

      char pad[16 * sizeof (char *) + 34 * sizeof (int)];

Steve Ellcey, could you check this on a 64 bit machine, HPUX.  I used the following with a simple program that performed a WRITE.

gfc_unit *
get_unit (st_parameter_dt *dtp, int do_create)
{
  st_printf("The DTP\n");
  st_printf("Size of p: %d\n", sizeof(dtp->u.p));
  st_printf("Size of pad: %d\n", sizeof(dtp->u.pad));
  st_printf("Size of char *: %d\n", sizeof(char *));
  st_printf("Size if int: %d\n", sizeof(int));
...
Comment 11 Steve Ellcey 2006-04-07 22:36:01 UTC
I think putting pad back to where it was is a good first step and I will see if there is room on a 64 bit machine, I think we need some kind of test to make sure that pad is always equal to or greater than the size of structure p and a comment that changing the size of pad will affect compatibility.  I don't know if we can do that check at GCC compile time though. 

I am now running into a related but slighty different problem on IA64 HP-UX. In trans-io.c (gfc_build_st_parameter) we build the 'u' field as an array of char.  Now in actual fact 'u' is the union of an array of char and the structure 'p'.  On IA64 HP-UX (or any strict memory alignment machine) an array of char will just be aligned on a 1-byte boundry, but the union would be aligned on a 4 or 8 byte boundry (whatever the largest field alignment requirement is).  So I am accessing ->u.p.item_count and the alignment is wrong because the front-end allocated the space based on char alignment not on int/struct alignment.
Comment 12 Jerry DeLisle 2006-04-07 23:05:16 UTC
Subject: Bug 26890

Author: jvdelisle
Date: Fri Apr  7 23:05:12 2006
New Revision: 112769

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112769
Log:
2006-04-07  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26890
	* io/io.h: Revert change to pad size made on 2006-03-30.
	Add comment explaining dependency with fortran/trans-io.c.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/io.h

Comment 13 Steve Ellcey 2006-04-10 18:22:51 UTC
Putting the size of pad back seems OK on IA64 in both ILP32 and LP64 modes.  In ILP32 mode I get:

The DTP
Size of p: 136
Size of pad: 200
Size of char *: 4
Size if int: 4

In LP64 mode, both on HP-UX and Linux, I get:

The DTP
Size of p: 168
Size of pad: 264
Size of char *: 8
Size if int: 4

So we have space without changing the size of pad and by putting pad back to the original size I do get programs to work but the alignment problem described in comment #11 is still there waiting to hit at any time.
Comment 14 Jerry DeLisle 2006-04-14 02:31:33 UTC
Subject: Bug 26890

Author: jvdelisle
Date: Fri Apr 14 02:31:28 2006
New Revision: 112943

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112943
Log:
2006-04-14  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26890
	* io/io.h: Add size_used to st_parameter_dt.
	*io/transfer.c (data_transfer_init): Initialize size_used to zero.
	(read_sf): Use size_used.
	(read_block): Likewise.
	(read_block_direct): Likewise.
	(write_block): Likewise.
	(write_buf): Likewise and eliminate erroneous FAILURE return.
	(finalize_transfer): Assign value of size_used to *dtp->size.

Modified:
    branches/gcc-4_1-branch/libgfortran/ChangeLog
    branches/gcc-4_1-branch/libgfortran/io/io.h
    branches/gcc-4_1-branch/libgfortran/io/transfer.c

Comment 15 Jerry DeLisle 2006-04-14 03:14:38 UTC
Testcase was committed to 4.1 branch as well, but I typoed the pr number so it did not register commit here.

Fixed on 4.1 and trunk.
Comment 16 Jerry DeLisle 2006-04-14 03:16:09 UTC
Closing