Re: [PATCH] Fix crash in _IO_wfile_sync (bug 20568)

[Florian Weimer <fweimer at redhat dot com>]

* Andreas Schwab:

> 	[BZ #20568]
> 	* libio/wfileops.c (_IO_wfile_sync): Correct last argument to
> 	__codecvt_do_length.
> 	* libio/Makefile (tests): Add tst-wfile-sync.
> 	($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales).
> 	* libio/tst-wfile-sync.c: New file.
> 	* libio/tst-wfile-sync.input: New file.

Would you please reference commit
18d26750dd8fd328a78cf639fd0ec2494680a2a4 in the commit message?

> diff --git a/libio/Makefile b/libio/Makefile
> index 95b91084dd..a5236c7042 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -65,7 +65,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
>  	tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
>  	tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
> -	tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153
> +	tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
> +	tst-wfile-sync

Okay.

> @@ -212,6 +213,7 @@ $(objpfx)tst-ungetwc1.out: $(gen-locales)
>  $(objpfx)tst-ungetwc2.out: $(gen-locales)
>  $(objpfx)tst-widetext.out: $(gen-locales)
>  $(objpfx)tst_wprintf2.out: $(gen-locales)
> +$(objpfx)tst-wfile-sync.out: $(gen-locales)
>  endif

Okay.

>  $(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen
> diff --git a/libio/tst-wfile-sync.c b/libio/tst-wfile-sync.c
> new file mode 100644
> index 0000000000..e1f621ede7
> --- /dev/null
> +++ b/libio/tst-wfile-sync.c
> @@ -0,0 +1,35 @@
> +/* Test that _IO_wfile_sync does not crash (bug 20568).
> +   Copyright (C) 2016-2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <locale.h>
> +#include <stdio.h>
> +#include <wchar.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  TEST_VERIFY_EXIT (setlocale (LC_ALL, "de_DE.UTF-8") != NULL);
> +  TEST_VERIFY_EXIT (fgetwc (stdin) != WEOF);
> +  /* This calls _IO_wfile_sync.  */
> +  TEST_VERIFY_EXIT (setvbuf (stdin, NULL, _IONBF, 0) == 0);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

You could perhaps add

  TEST_COMPARE (xlseek (0, 0, SEEK_CUR), 1);

, to show that the underlying descriptor was updated correctly.

> diff --git a/libio/tst-wfile-sync.input b/libio/tst-wfile-sync.input
> new file mode 100644
> index 0000000000..12d0958f7a
> --- /dev/null
> +++ b/libio/tst-wfile-sync.input
> @@ -0,0 +1 @@
> +This is a test of _IO_wfile_sync.

Woah.  Magic!  I didn't know we had this Makefile rule:

$(objpfx)%.out: %.input $(objpfx)%

> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index 0367643703..42aaf5e855 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -508,11 +508,11 @@ _IO_wfile_sync (FILE *fp)
>  	     generate the wide characters up to the current reading
>  	     position.  */
>  	  int nread;
> -
> +	  size_t wnread = fp->_wide_data->_IO_read_ptr - fp->_wide_data->_IO_read_base;
>  	  fp->_wide_data->_IO_state = fp->_wide_data->_IO_last_state;
>  	  nread = (*cv->__codecvt_do_length) (cv, &fp->_wide_data->_IO_state,
>  					      fp->_IO_read_base,
> -					      fp->_IO_read_end, delta);
> +					      fp->_IO_read_end, wnread);
>  	  fp->_IO_read_ptr = fp->_IO_read_base + nread;
>  	  delta = -(fp->_IO_read_end - fp->_IO_read_base - nread);
>  	}

Okay.

The fixed-width case is not affected because there, the seek amount can
be determined directly from the distance between _IO_read_ptr and
_IO_read_end.

Thanks,
Florian