Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Andre Vehreschild]

Hi Janne,

> Ugh. Is is possible to un-break the ABI then, or is the plan to merge
> fortran-dev regardless and fix the performance issues later? Seems to
> me that breaking the ABI for GCC 7 and again for GCC 8 is the worst
> choice.

The ABI is broken without bumping the version. Bumping the ABI-version is not
breaking it. That's called progress. I don't get why you oppose progress in
gfortran that vehemently. There is only limited manpower to progress gfortran,
so we all should be happy, that things are addressed. Users can always choose
to stick with an older version of gfortran when they don't want to recompile
everything. When I get the commercial compilers right, then do they change
their ABI-version with every major release. Why is what is good for them, bad
for us? We are making progress; Slowly, but progress nevertheless. So please
motivate folks to do work ...

- Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Janne Blomqvist]

On Mon, Oct 24, 2016 at 7:15 PM, Jerry DeLisle  wrote:
> On 10/24/2016 07:49 AM, Janne Blomqvist wrote:
>>
>> On Mon, Oct 24, 2016 at 2:53 PM, Jerry DeLisle 
>> wrote:
>>>
>>> On 10/18/2016 04:48 PM, Jerry DeLisle wrote:


 Hi Folks,

 The attached patch does some minor cleanup and bumps the libgfortran
 version
 number.  I have wanted to reorder the dtp structure for many years now.
 Not
 strictly needed but it has bugged me forever.

 The bump is needed because of the significant changes from
 implementation
 of DTIO.

>>>
>>> --- snip ---
>>>
>>> Revised patch. Changed a single byte char to int for better memory
>>> alignment
>>> on 32 bit boundaries. Regression tested on x86-64-linux, FreeBSD, and
>>> Power8.
>>>
>>> I plan to commit this to trunk sometime today.
>>
>>
>> I understood from https://gcc.gnu.org/ml/fortran/2016-10/msg00168.html
>> that fortran-dev won't be in shape for merging until GCC 8 stage 1. If
>> so, shouldn't we avoid breaking the ABI until then?
>>
>
> The ABI is already changed with the UD-DTIO changes, so we need to bump
> version to avoid surprises with other earlier versions.

Ugh. Is is possible to un-break the ABI then, or is the plan to merge
fortran-dev regardless and fix the performance issues later? Seems to
me that breaking the ABI for GCC 7 and again for GCC 8 is the worst
choice.


-- 
Janne Blomqvist

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Jerry DeLisle]

On 10/24/2016 07:49 AM, Janne Blomqvist wrote:

On Mon, Oct 24, 2016 at 2:53 PM, Jerry DeLisle  wrote:

On 10/18/2016 04:48 PM, Jerry DeLisle wrote:


Hi Folks,

The attached patch does some minor cleanup and bumps the libgfortran
version
number.  I have wanted to reorder the dtp structure for many years now.
Not
strictly needed but it has bugged me forever.

The bump is needed because of the significant changes from implementation
of DTIO.



--- snip ---

Revised patch. Changed a single byte char to int for better memory alignment
on 32 bit boundaries. Regression tested on x86-64-linux, FreeBSD, and
Power8.

I plan to commit this to trunk sometime today.


I understood from https://gcc.gnu.org/ml/fortran/2016-10/msg00168.html
that fortran-dev won't be in shape for merging until GCC 8 stage 1. If
so, shouldn't we avoid breaking the ABI until then?



The ABI is already changed with the UD-DTIO changes, so we need to bump version 
to avoid surprises with other earlier versions.


Jerry

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Janne Blomqvist]

On Mon, Oct 24, 2016 at 2:53 PM, Jerry DeLisle  wrote:
> On 10/18/2016 04:48 PM, Jerry DeLisle wrote:
>>
>> Hi Folks,
>>
>> The attached patch does some minor cleanup and bumps the libgfortran
>> version
>> number.  I have wanted to reorder the dtp structure for many years now.
>> Not
>> strictly needed but it has bugged me forever.
>>
>> The bump is needed because of the significant changes from implementation
>> of DTIO.
>>
>
> --- snip ---
>
> Revised patch. Changed a single byte char to int for better memory alignment
> on 32 bit boundaries. Regression tested on x86-64-linux, FreeBSD, and
> Power8.
>
> I plan to commit this to trunk sometime today.

I understood from https://gcc.gnu.org/ml/fortran/2016-10/msg00168.html
that fortran-dev won't be in shape for merging until GCC 8 stage 1. If
so, shouldn't we avoid breaking the ABI until then?

>
>
>>
>> Regression tested on x86-64-linux.
>>
>> OK for trunk?
>>
>> Regards,
>>
>> Jerry
>>
>> 2016-10-18  Jerry DeLisle  
>>
>> PR fortran/77828
>> * io/io.h (st_parameter_dt): Reorder for readability and sanity.
>> * io/transfer.c (data_transfer_init): Remove TODO and enable the
>> runtime error message, rec= specifier not allowed in STREAM
>> access.
>> * libtool-version: Bump major version of libgfortran to 4.
>>
>> 2016-10-18  Jerry DeLisle  
>>
>> PR fortran/77828
>> * ioparm.def: Reorder dt parameters to match libgfortran.
>>
>>
>>
>



-- 
Janne Blomqvist

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Jerry DeLisle]

On 10/18/2016 04:48 PM, Jerry DeLisle wrote:

Hi Folks,

The attached patch does some minor cleanup and bumps the libgfortran version
number.  I have wanted to reorder the dtp structure for many years now. Not
strictly needed but it has bugged me forever.

The bump is needed because of the significant changes from implementation of 
DTIO.



--- snip ---

Revised patch. Changed a single byte char to int for better memory alignment on 
32 bit boundaries. Regression tested on x86-64-linux, FreeBSD, and Power8.


I plan to commit this to trunk sometime today.



Regression tested on x86-64-linux.

OK for trunk?

Regards,

Jerry

2016-10-18  Jerry DeLisle  

PR fortran/77828
* io/io.h (st_parameter_dt): Reorder for readability and sanity.
* io/transfer.c (data_transfer_init): Remove TODO and enable the
runtime error message, rec= specifier not allowed in STREAM
access.
* libtool-version: Bump major version of libgfortran to 4.

2016-10-18  Jerry DeLisle  

PR fortran/77828
* ioparm.def: Reorder dt parameters to match libgfortran.





diff --git a/gcc/fortran/ioparm.def b/gcc/fortran/ioparm.def
index 17b7ac7..bd628ce 100644
--- a/gcc/fortran/ioparm.def
+++ b/gcc/fortran/ioparm.def
@@ -90,11 +90,9 @@ IOPARM (inquire, id,		1 << 7,  pint4)
 IOPARM (inquire, iqstream,	1 << 8,  char1)
 IOPARM (wait,common,	0,	 common)
 IOPARM (wait,id,		1 << 7,  pint4)
-#ifndef IOPARM_dt_list_format
+IOPARM (dt,  common,	0,	 common)
 #define IOPARM_dt_list_format		(1 << 7)
 #define IOPARM_dt_namelist_read_mode	(1 << 8)
-#endif
-IOPARM (dt,  common,	0,	 common)
 IOPARM (dt,  rec,		1 << 9,  intio)
 IOPARM (dt,  size,		1 << 10, pintio)
 IOPARM (dt,  iolength,	1 << 11, pintio)
@@ -103,7 +101,6 @@ IOPARM (dt,  format,	1 << 12, char1)
 IOPARM (dt,  advance,	1 << 13, char2)
 IOPARM (dt,  internal_unit,	1 << 14, char1)
 IOPARM (dt,  namelist_name,	1 << 15, char2)
-IOPARM (dt,  u,		0,	 pad)
 IOPARM (dt,  id,		1 << 16, pint4)
 IOPARM (dt,  pos,		1 << 17, intio)
 IOPARM (dt,  asynchronous, 	1 << 18, char1)
@@ -115,3 +112,4 @@ IOPARM (dt,  round,		1 << 23, char2)
 IOPARM (dt,  sign,		1 << 24, char1)
 #define IOPARM_dt_f2003		  (1 << 25)
 #define IOPARM_dt_dtio		  (1 << 26)
+IOPARM (dt,  u,		0,	 pad)
diff --git a/gcc/testsuite/gfortran.dg/streamio_17.f90 b/gcc/testsuite/gfortran.dg/streamio_17.f90
new file mode 100644
index 000..41fa0b9
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/streamio_17.f90
@@ -0,0 +1,12 @@
+! { dg-do run }
+program stream_test
+implicit none
+integer :: ios
+character(128) :: message
+open(10, status='scratch', access='stream')
+write (10, rec=1, iostat=ios, iomsg=message) "This is a test" !
+if (ios.ne.5001) call abort
+if (message.ne. &
+  &"Record number not allowed for stream access data transfer") &
+  call abort
+end program
diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h
index 8b20389..8c6caef 100644
--- a/libgfortran/io/io.h
+++ b/libgfortran/io/io.h
@@ -424,6 +424,15 @@ typedef struct st_parameter_dt
   CHARACTER2 (advance);
   CHARACTER1 (internal_unit);
   CHARACTER2 (namelist_name);
+  GFC_INTEGER_4 *id;
+  GFC_IO_INT pos;
+  CHARACTER1 (asynchronous);
+  CHARACTER2 (blank);
+  CHARACTER1 (decimal);
+  CHARACTER2 (delim);
+  CHARACTER1 (pad);
+  CHARACTER2 (round);
+  CHARACTER1 (sign);
   /* Private part of the structure.  The compiler just needs
  to reserve enough space.  */
   union
@@ -440,7 +449,8 @@ typedef struct st_parameter_dt
 	  unit_blank blank_status;
 	  unit_sign sign_status;
 	  int scale_factor;
-	  int max_pos; /* Maximum righthand column written to.  */
+	  /* Maximum righthand column written to.  */
+	  int max_pos;
 	  /* Number of skips + spaces to be done for T and X-editing.  */
 	  int skips;
 	  /* Number of spaces to be done for T and X-editing.  */
@@ -494,8 +504,7 @@ typedef struct st_parameter_dt
 	 are an unsigned char, EOF, or EOF - 1 used to mark the
 	 field as not valid.  */
 	  int last_char; /* No longer used, moved to gfc_unit.  */
-	  char nml_delim;
-
+	  int nml_delim;
 	  int repeat_count;
 	  int saved_length;
 	  int saved_used;
@@ -523,15 +532,6 @@ typedef struct st_parameter_dt
 	 must be smaller or equal to this array.  */
   char pad[16 * sizeof (char *) + 32 * sizeof (int)];
 } u;
-  GFC_INTEGER_4 *id;
-  GFC_IO_INT pos;
-  CHARACTER1 (asynchronous);
-  CHARACTER2 (blank);
-  CHARACTER1 (decimal);
-  CHARACTER2 (delim);
-  CHARACTER1 (pad);
-  CHARACTER2 (round);
-  CHARACTER1 (sign);
 }
 st_parameter_dt;
 
diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c
index 0879ca7..e3f75b6 100644
--- a/libgfortran/io/transfer.c
+++ b/libgfortran/io/transfer.c
@@ -3026,20 +3026,13 @@ data_transfer_init (st_parameter_dt *dtp, int read_flag)
 	  return;
 	}
 
-  /* TODO: This is required to maintain compatibility between
- 4.3 and 4.4 runtime. Remove when 

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Steve Kargl]

I haven't had time to test the patch.  I don't see
off hand anything wrong the patch.  But, as you've
seen in bugzilla recent IO work has caused some
problem for a few archs.  You'll probably need to
keep an eye open for new issues.

-- 
steve

On Thu, Oct 20, 2016 at 12:32:38PM -0700, Jerry DeLisle wrote:
> Ping (shall I proceed)
> 
> On 10/18/2016 04:48 PM, Jerry DeLisle wrote:
> > Hi Folks,
> >
> > The attached patch does some minor cleanup and bumps the libgfortran version
> > number.  I have wanted to reorder the dtp structure for many years now. Not
> > strictly needed but it has bugged me forever.
> >
> > The bump is needed because of the significant changes from implementation 
> > of DTIO.
> >
> > I also took care of the stream I/O TODO and added a new test case for the 
> > error
> > message.
> >
> > On my system, I have LD_LIBRARY_PATH set to point to the new library version
> > first and then the version 3 are found elsewhere. With the patch we now see 
> > the
> > following behavior. (Test case in the PR)
> >
> > Compile with version 6. Finds libgfortran3 on execution:
> > $gfc6 pr77828.f90
> > $ ./a.out
> >  Greetings from i 42 of 43
> >
> >  Greetings from i 42 of 43
> >
> > Compile with version 7. Finds libgfortran4 on execution:
> > $ gfc pr77828.f90
> > [jerry@amda8 pr77828]$ ./a.out
> >  Greetings from i 42 of 43
> >
> >  Greetings from i 42 of 43
> >
> > Change LD_LIBRARY_PATH to not find libgfortran4:
> > $ export LD_LIBRARY_PATH=/home/jerry/dev/usr6/lib64
> > $ gfc pr77828.f90
> > $ ./a.out
> > ./a.out: error while loading shared libraries: libgfortran.so.4: cannot open
> > shared object file: No such file or directory
> >
> > Word of caution. When this patch is applied rebuild from a clean/empty build
> > directory. You must delete any libgfortran3 remnants that may have been 
> > built
> > and installed previously with gcc version 7 trunk, Otherwise the 
> > linker/loader
> > may find those rather than a libgfortran3 built for gcc 6 or previous. (When
> > compiling with a previous version of gcc)
> >
> > New test case provided for the streamio error check.
> >
> > Regression tested on x86-64-linux.
> >
> > OK for trunk?
> >
> > Regards,
> >
> > Jerry
> >
> > 2016-10-18  Jerry DeLisle  
> >
> > PR fortran/77828
> > * io/io.h (st_parameter_dt): Reorder for readability and sanity.
> > * io/transfer.c (data_transfer_init): Remove TODO and enable the
> > runtime error message, rec= specifier not allowed in STREAM
> > access.
> > * libtool-version: Bump major version of libgfortran to 4.
> >
> > 2016-10-18  Jerry DeLisle  
> >
> > PR fortran/77828
> > * ioparm.def: Reorder dt parameters to match libgfortran.
> >
> >
> >

-- 
Steve

Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Jerry DeLisle]

Ping (shall I proceed)

On 10/18/2016 04:48 PM, Jerry DeLisle wrote:

Hi Folks,

The attached patch does some minor cleanup and bumps the libgfortran version
number.  I have wanted to reorder the dtp structure for many years now. Not
strictly needed but it has bugged me forever.

The bump is needed because of the significant changes from implementation of 
DTIO.

I also took care of the stream I/O TODO and added a new test case for the error
message.

On my system, I have LD_LIBRARY_PATH set to point to the new library version
first and then the version 3 are found elsewhere. With the patch we now see the
following behavior. (Test case in the PR)

Compile with version 6. Finds libgfortran3 on execution:
$gfc6 pr77828.f90
$ ./a.out
 Greetings from i 42 of 43

 Greetings from i 42 of 43

Compile with version 7. Finds libgfortran4 on execution:
$ gfc pr77828.f90
[jerry@amda8 pr77828]$ ./a.out
 Greetings from i 42 of 43

 Greetings from i 42 of 43

Change LD_LIBRARY_PATH to not find libgfortran4:
$ export LD_LIBRARY_PATH=/home/jerry/dev/usr6/lib64
$ gfc pr77828.f90
$ ./a.out
./a.out: error while loading shared libraries: libgfortran.so.4: cannot open
shared object file: No such file or directory

Word of caution. When this patch is applied rebuild from a clean/empty build
directory. You must delete any libgfortran3 remnants that may have been built
and installed previously with gcc version 7 trunk, Otherwise the linker/loader
may find those rather than a libgfortran3 built for gcc 6 or previous. (When
compiling with a previous version of gcc)

New test case provided for the streamio error check.

Regression tested on x86-64-linux.

OK for trunk?

Regards,

Jerry

2016-10-18  Jerry DeLisle  

PR fortran/77828
* io/io.h (st_parameter_dt): Reorder for readability and sanity.
* io/transfer.c (data_transfer_init): Remove TODO and enable the
runtime error message, rec= specifier not allowed in STREAM
access.
* libtool-version: Bump major version of libgfortran to 4.

2016-10-18  Jerry DeLisle  

PR fortran/77828
* ioparm.def: Reorder dt parameters to match libgfortran.

[patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes

[Jerry DeLisle]

Hi Folks,

The attached patch does some minor cleanup and bumps the libgfortran version 
number.  I have wanted to reorder the dtp structure for many years now. Not 
strictly needed but it has bugged me forever.


The bump is needed because of the significant changes from implementation of 
DTIO.

I also took care of the stream I/O TODO and added a new test case for the error 
message.


On my system, I have LD_LIBRARY_PATH set to point to the new library version 
first and then the version 3 are found elsewhere. With the patch we now see the 
following behavior. (Test case in the PR)


Compile with version 6. Finds libgfortran3 on execution:
$gfc6 pr77828.f90
$ ./a.out
 Greetings from i 42 of 43

 Greetings from i 42 of 43

Compile with version 7. Finds libgfortran4 on execution:
$ gfc pr77828.f90
[jerry@amda8 pr77828]$ ./a.out
 Greetings from i 42 of 43

 Greetings from i 42 of 43

Change LD_LIBRARY_PATH to not find libgfortran4:
$ export LD_LIBRARY_PATH=/home/jerry/dev/usr6/lib64
$ gfc pr77828.f90
$ ./a.out
./a.out: error while loading shared libraries: libgfortran.so.4: cannot open 
shared object file: No such file or directory


Word of caution. When this patch is applied rebuild from a clean/empty build 
directory. You must delete any libgfortran3 remnants that may have been built 
and installed previously with gcc version 7 trunk, Otherwise the linker/loader 
may find those rather than a libgfortran3 built for gcc 6 or previous. (When 
compiling with a previous version of gcc)


New test case provided for the streamio error check.

Regression tested on x86-64-linux.

OK for trunk?

Regards,

Jerry

2016-10-18  Jerry DeLisle  

PR fortran/77828
* io/io.h (st_parameter_dt): Reorder for readability and sanity.
* io/transfer.c (data_transfer_init): Remove TODO and enable the
runtime error message, rec= specifier not allowed in STREAM
access.
* libtool-version: Bump major version of libgfortran to 4.

2016-10-18  Jerry DeLisle  

PR fortran/77828
* ioparm.def: Reorder dt parameters to match libgfortran.



diff --git a/gcc/fortran/ioparm.def b/gcc/fortran/ioparm.def
index 17b7ac78..bd628ce2 100644
--- a/gcc/fortran/ioparm.def
+++ b/gcc/fortran/ioparm.def
@@ -90,11 +90,9 @@ IOPARM (inquire, id,		1 << 7,  pint4)
 IOPARM (inquire, iqstream,	1 << 8,  char1)
 IOPARM (wait,common,	0,	 common)
 IOPARM (wait,id,		1 << 7,  pint4)
-#ifndef IOPARM_dt_list_format
+IOPARM (dt,  common,	0,	 common)
 #define IOPARM_dt_list_format		(1 << 7)
 #define IOPARM_dt_namelist_read_mode	(1 << 8)
-#endif
-IOPARM (dt,  common,	0,	 common)
 IOPARM (dt,  rec,		1 << 9,  intio)
 IOPARM (dt,  size,		1 << 10, pintio)
 IOPARM (dt,  iolength,	1 << 11, pintio)
@@ -103,7 +101,6 @@ IOPARM (dt,  format,	1 << 12, char1)
 IOPARM (dt,  advance,	1 << 13, char2)
 IOPARM (dt,  internal_unit,	1 << 14, char1)
 IOPARM (dt,  namelist_name,	1 << 15, char2)
-IOPARM (dt,  u,		0,	 pad)
 IOPARM (dt,  id,		1 << 16, pint4)
 IOPARM (dt,  pos,		1 << 17, intio)
 IOPARM (dt,  asynchronous, 	1 << 18, char1)
@@ -115,3 +112,4 @@ IOPARM (dt,  round,		1 << 23, char2)
 IOPARM (dt,  sign,		1 << 24, char1)
 #define IOPARM_dt_f2003		  (1 << 25)
 #define IOPARM_dt_dtio		  (1 << 26)
+IOPARM (dt,  u,		0,	 pad)
diff --git a/gcc/testsuite/gfortran.dg/streamio_17.f90 b/gcc/testsuite/gfortran.dg/streamio_17.f90
new file mode 100644
index ..41fa0b98
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/streamio_17.f90
@@ -0,0 +1,12 @@
+! { dg-do run }
+program stream_test
+implicit none
+integer :: ios
+character(128) :: message
+open(10, status='scratch', access='stream')
+write (10, rec=1, iostat=ios, iomsg=message) "This is a test" !
+if (ios.ne.5001) call abort
+if (message.ne. &
+  &"Record number not allowed for stream access data transfer") &
+  call abort
+end program
diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h
index edc520a9..cae2193b 100644
--- a/libgfortran/io/io.h
+++ b/libgfortran/io/io.h
@@ -424,6 +424,15 @@ typedef struct st_parameter_dt
   CHARACTER2 (advance);
   CHARACTER1 (internal_unit);
   CHARACTER2 (namelist_name);
+  GFC_INTEGER_4 *id;
+  GFC_IO_INT pos;
+  CHARACTER1 (asynchronous);
+  CHARACTER2 (blank);
+  CHARACTER1 (decimal);
+  CHARACTER2 (delim);
+  CHARACTER1 (pad);
+  CHARACTER2 (round);
+  CHARACTER1 (sign);
   /* Private part of the structure.  The compiler just needs
  to reserve enough space.  */
   union
@@ -440,7 +449,8 @@ typedef struct st_parameter_dt
 	  unit_blank blank_status;
 	  unit_sign sign_status;
 	  int scale_factor;
-	  int max_pos; /* Maximum righthand column written to.  */
+	  /* Maximum righthand column written to.  */
+	  int max_pos;
 	  /* Number of skips + spaces to be done for T and X-editing.  */
 	  int skips;
 	  /* Number of spaces to be done for T and X-editing.  */
@@ -522,15 +532,6 @@ typedef struct