[patch,libgfortran] PR88776 Namelist read from stdin: loss of data

[patch,libgfortran] PR88776 Namelist read from stdin: loss of data

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

Hi all,

As stated in the PR, the problem turns out to be an ungraceful return 
after an error.  Most namelist errors go through nml_err_ret, The one I 
am removing did not and in the unique case of UNIT=5 after the error it 
falls through and hits some code which modifies pointers to the namelist 
data structures.

This patch fixes it.

Regression tested on x86-64 and manually tested with a redirection to 
stdin. (cat somefile | ./a.out )

I plan to commit today as simple along with a new testcase.

Regards.

Jerry

2019-01-12  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/88776
	* io/list_read.c (namelist_read): Use nml_err_ret path on
	read error, not based on stdin_unit.

[-- Attachment #2: pr88776.diff --]
[-- Type: text/x-patch, Size: 670 bytes --]

diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index 4a7ccb3ddd5..d9af255a034 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -3614,11 +3614,7 @@ find_nml_name:
   while (!dtp->u.p.input_complete)
     {
       if (!nml_get_obj_data (dtp, &prev_nl, nml_err_msg, sizeof nml_err_msg))
-	{
-	  if (dtp->u.p.current_unit->unit_number != options.stdin_unit)
-	    goto nml_err_ret;
-	  generate_error (&dtp->common, LIBERROR_READ_VALUE, nml_err_msg);
-        }
+	goto nml_err_ret;
 
       /* Reset the previous namelist pointer if we know we are not going
 	 to be doing multiple reads within a single namelist object.  */

Re: [patch,libgfortran] PR88776 Namelist read from stdin: loss of data

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On 1/12/19 2:35 PM, Jerry DeLisle wrote:
> Hi all,
> 
> As stated in the PR, the problem turns out to be an ungraceful return 
> after an error.  Most namelist errors go through nml_err_ret, The one I 
> am removing did not and in the unique case of UNIT=5 after the error it 
> falls through and hits some code which modifies pointers to the namelist 
> data structures.
> 
> This patch fixes it.
> 
> Regression tested on x86-64 and manually tested with a redirection to 
> stdin. (cat somefile | ./a.out )
> 
> I plan to commit today as simple along with a new testcase.
> 
> Regards.
> 
> Jerry
> 
> 2019-01-12  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
>      PR libfortran/88776
>      * io/list_read.c (namelist_read): Use nml_err_ret path on
>      read error, not based on stdin_unit.

Committed.

While doing extra testing on the test case for this PR, I noticed a 
memory leak with valgrind.

The attached patch fixes this. The problem is when opening a file with a 
preconnected unit, the previously initialized format buffer (fbuf) in 
the preconnected unit structure gets abandoned (pointer overwritten by 
the new fbuf), and a new one allocated later in the code path.

I will commit this additional patch and the test case as simple and obvious.

Regression tested on x86_64. Valgrind clean.

Regards,

Jerry

2019-01-13  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

    PR libfortran/88776
    * io/open.c (newunit): Free format buffer if the unit specified is
    for stdin, stdout, or stderr.

2019-01-13  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

    PR libfortran/88776
    * gfortran.dg/namelist_96.f90: New test.



[-- Attachment #2: pr88776a.diff --]
[-- Type: text/x-patch, Size: 694 bytes --]

diff --git a/libgfortran/io/open.c b/libgfortran/io/open.c
index 97fb17c7424..b48afabf7a3 100644
--- a/libgfortran/io/open.c
+++ b/libgfortran/io/open.c
@@ -530,6 +530,14 @@ new_unit (st_parameter_open *opp, gfc_unit *u, unit_flags *flags)
   if (u2 != NULL)
     unlock_unit (u2);
 
+  /* If the unit specified is preconnected with a file specified to be open,
+     then clear the format buffer.  */
+  if ((opp->common.unit == options.stdin_unit ||
+       opp->common.unit == options.stdout_unit ||
+       opp->common.unit == options.stderr_unit)
+      && (opp->common.flags & IOPARM_OPEN_HAS_FILE) != 0)
+    fbuf_destroy (u);
+
   /* Open file.  */
 
   s = open_external (opp, flags);

[-- Attachment #3: namelist_96.f90 --]
[-- Type: text/x-fortran, Size: 1069 bytes --]

! ( dg-do run }
program pr88776
  implicit none
  character(*), parameter :: file = "pr88776.dat"
  type t_chan
     integer          :: ichan = -1
     character(len=8) :: flag  = ''
     integer          :: band  = -1
  end type t_chan
  type(t_chan) :: chan
  namelist /NML/ chan
  open (11,file=file)
  write(11,'(a)') trim("&nml chan = 1   '#1 '    10 /")
  write(11,'(a)') trim("&nml chan = 2   '#2 '    42.36/")
  write(11,'(a)') trim("&nml chan = 3   '#3 '    30 /")
  close(11)
  call read (unit=10) ! No problem
  call read (unit=5)  ! problem, now fixed
  open (11,file=file)
  close (11, status="delete")
contains
  subroutine read (unit)
    integer, intent(in) :: unit
    integer             :: stat
    open (unit, file=file, action="read")
    chan = t_chan(-1,'',-1)
    stat = 0
    read (unit, nml=NML, iostat=stat)
    if (stat /= 0) stop 1
    chan = t_chan(-1,'',-1)
    read (unit, nml=NML, iostat=stat)
    if (stat == 0) stop 2
    if (chan% ichan /= 2) then
       stop 3
    end if
    close (unit)
  end subroutine read
end program pr88776