[PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Takashi Yano]

Previously, fprintf() on a wide-oriented stream crashes or outputs
garbage. This is because a narrow char string which can be odd bytes
in length is cast into a wide char string which should be even
bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
As a result, if the length is odd bytes, the reading buffer runs over
the buffer length, which causes a crash. If the length is even bytes,
garbage is printed.

With this patch, any output to the stream which is set to different
orientation fails with error just like glibc. Note that it behaves
differently from other libc implementations such as BSD, musl and
Solaris.

Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 newlib/libc/stdio/fgetwc.c    |  6 ++++--
 newlib/libc/stdio/fgetwc_u.c  |  3 ++-
 newlib/libc/stdio/fgetws.c    |  3 ++-
 newlib/libc/stdio/fputs.c     |  9 ++++++---
 newlib/libc/stdio/fputwc.c    |  6 ++++--
 newlib/libc/stdio/fputwc_u.c  |  3 ++-
 newlib/libc/stdio/fputws.c    |  6 ++++--
 newlib/libc/stdio/fread.c     |  7 ++++++-
 newlib/libc/stdio/fwrite.c    |  9 +++++++--
 newlib/libc/stdio/local.h     | 31 +++++++++++++++++--------------
 newlib/libc/stdio/putc.c      |  4 ++++
 newlib/libc/stdio/puts.c      |  9 ++++++---
 newlib/libc/stdio/refill.c    |  3 ++-
 newlib/libc/stdio/ungetc.c    |  6 +++++-
 newlib/libc/stdio/ungetwc.c   |  5 +++--
 newlib/libc/stdio/vfprintf.c  |  5 ++++-
 newlib/libc/stdio/vfscanf.c   |  6 +++++-
 newlib/libc/stdio/vfwprintf.c |  5 ++++-
 newlib/libc/stdio/vfwscanf.c  |  6 +++++-
 19 files changed, 92 insertions(+), 40 deletions(-)

diff --git a/newlib/libc/stdio/fgetwc.c b/newlib/libc/stdio/fgetwc.c
index 522697e9b..38e2db897 100644
--- a/newlib/libc/stdio/fgetwc.c
+++ b/newlib/libc/stdio/fgetwc.c
@@ -177,8 +177,10 @@ _fgetwc_r (struct _reent *ptr,
   wint_t r;
 
   _newlib_flockfile_start (fp);
-  ORIENT(fp, 1);
-  r = __fgetwc (ptr, fp);
+  if (ORIENT(fp, 1) != 1)
+    r = WEOF;
+  else
+    r = __fgetwc (ptr, fp);
   _newlib_flockfile_end (fp);
   return r;
 }
diff --git a/newlib/libc/stdio/fgetwc_u.c b/newlib/libc/stdio/fgetwc_u.c
index cbb97360b..007f9c844 100644
--- a/newlib/libc/stdio/fgetwc_u.c
+++ b/newlib/libc/stdio/fgetwc_u.c
@@ -33,7 +33,8 @@ wint_t
 _fgetwc_unlocked_r (struct _reent *ptr,
 	register FILE *fp)
 {
-  ORIENT(fp, 1);
+  if (ORIENT(fp, 1) != 1)
+    return WEOF;
   return __fgetwc (ptr, fp);
 }
 
diff --git a/newlib/libc/stdio/fgetws.c b/newlib/libc/stdio/fgetws.c
index c942806c6..3b7fa9516 100644
--- a/newlib/libc/stdio/fgetws.c
+++ b/newlib/libc/stdio/fgetws.c
@@ -110,7 +110,8 @@ _fgetws_r (struct _reent *ptr,
   unsigned char *nl;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
 
   if (n <= 0)
     {
diff --git a/newlib/libc/stdio/fputs.c b/newlib/libc/stdio/fputs.c
index 4e9cb7547..a4f18df2a 100644
--- a/newlib/libc/stdio/fputs.c
+++ b/newlib/libc/stdio/fputs.c
@@ -103,8 +103,10 @@ _fputs_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
-  result = __sfvwrite_r (ptr, fp, &uio);
+  if (ORIENT (fp, -1) != -1)
+    result = EOF;
+  else
+    result = __sfvwrite_r (ptr, fp, &uio);
   _newlib_flockfile_end (fp);
   return result;
 #else
@@ -113,7 +115,8 @@ _fputs_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto error;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto error;
diff --git a/newlib/libc/stdio/fputwc.c b/newlib/libc/stdio/fputwc.c
index 9a4e80d52..ef2be1048 100644
--- a/newlib/libc/stdio/fputwc.c
+++ b/newlib/libc/stdio/fputwc.c
@@ -169,8 +169,10 @@ _fputwc_r (struct _reent *ptr,
   wint_t r;
 
   _newlib_flockfile_start (fp);
-  ORIENT(fp, 1);
-  r = __fputwc(ptr, wc, fp);
+  if (ORIENT(fp, 1) != 1)
+    r = WEOF;
+  else
+    r = __fputwc(ptr, wc, fp);
   _newlib_flockfile_end (fp);
   return r;
 }
diff --git a/newlib/libc/stdio/fputwc_u.c b/newlib/libc/stdio/fputwc_u.c
index d4e51532a..a5cc9a5d5 100644
--- a/newlib/libc/stdio/fputwc_u.c
+++ b/newlib/libc/stdio/fputwc_u.c
@@ -34,7 +34,8 @@ _fputwc_unlocked_r (struct _reent *ptr,
 	wchar_t wc,
 	FILE *fp)
 {
-  ORIENT(fp, 1);
+  if (ORIENT(fp, 1) != 1)
+    return WEOF;
   return __fputwc(ptr, wc, fp);
 }
 
diff --git a/newlib/libc/stdio/fputws.c b/newlib/libc/stdio/fputws.c
index 92f2cbf6a..a9ac9d6d7 100644
--- a/newlib/libc/stdio/fputws.c
+++ b/newlib/libc/stdio/fputws.c
@@ -105,7 +105,8 @@ _fputws_r (struct _reent *ptr,
   struct __siov iov;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
   if (cantwrite (ptr, fp) != 0)
     goto error;
   uio.uio_iov = &iov;
@@ -129,7 +130,8 @@ error:
   return (-1);
 #else
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
   if (cantwrite (ptr, fp) != 0)
     goto error;
 
diff --git a/newlib/libc/stdio/fread.c b/newlib/libc/stdio/fread.c
index df8321461..8664dc3e5 100644
--- a/newlib/libc/stdio/fread.c
+++ b/newlib/libc/stdio/fread.c
@@ -158,7 +158,11 @@ _fread_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      count = 0;
+      goto ret;
+    }
   if (fp->_r < 0)
     fp->_r = 0;
   total = resid;
@@ -252,6 +256,7 @@ _fread_r (struct _reent * ptr,
       return crlf_r(ptr, fp, buf, total, 0) / size;
     }
 #endif
+ret:
   _newlib_flockfile_end (fp);
   return count;
 }
diff --git a/newlib/libc/stdio/fwrite.c b/newlib/libc/stdio/fwrite.c
index d499f6f59..2d9749693 100644
--- a/newlib/libc/stdio/fwrite.c
+++ b/newlib/libc/stdio/fwrite.c
@@ -133,7 +133,11 @@ _fwrite_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      _newlib_flockfile_exit (fp);
+      return 0;
+    }
   if (__sfvwrite_r (ptr, fp, &uio) == 0)
     {
       _newlib_flockfile_exit (fp);
@@ -148,7 +152,8 @@ _fwrite_r (struct _reent * ptr,
   CHECK_INIT (ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto ret;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto ret;
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index b34c7c9d8..3b86cf19a 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -231,21 +231,24 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
  * Set the orientation for a stream. If o > 0, the stream has wide-
  * orientation. If o < 0, the stream has byte-orientation.
  */
-#define ORIENT(fp,ori)					\
-  do								\
-    {								\
-      if (!((fp)->_flags & __SORD))	\
-	{							\
-	  (fp)->_flags |= __SORD;				\
-	  if (ori > 0)						\
-	    (fp)->_flags2 |= __SWID;				\
-	  else							\
-	    (fp)->_flags2 &= ~__SWID;				\
-	}							\
-    }								\
-  while (0)
+#define ORIENT(fp,ori)			\
+  (					\
+    (					\
+      ((fp)->_flags & __SORD) ?		\
+	0				\
+      :					\
+	(				\
+	  ((fp)->_flags |= __SORD),	\
+	  (ori > 0) ?			\
+	    ((fp)->_flags2 |= __SWID)	\
+	  :				\
+	    ((fp)->_flags2 &= ~__SWID)	\
+	)				\
+    ),					\
+    ((fp)->_flags2 & __SWID) ? 1 : -1	\
+  )
 #else
-#define ORIENT(fp,ori)
+#define ORIENT(fp,ori) (-1)
 #endif
 
 /* WARNING: _dcvt is defined in the stdlib directory, not here!  */
diff --git a/newlib/libc/stdio/putc.c b/newlib/libc/stdio/putc.c
index 6a410e216..925341939 100644
--- a/newlib/libc/stdio/putc.c
+++ b/newlib/libc/stdio/putc.c
@@ -84,6 +84,8 @@ _putc_r (struct _reent *ptr,
 {
   int result;
   CHECK_INIT (ptr, fp);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
   _newlib_flockfile_start (fp);
   result = __sputc_r (ptr, c, fp);
   _newlib_flockfile_end (fp);
@@ -100,6 +102,8 @@ putc (int c,
   struct _reent *reent = _REENT;
 
   CHECK_INIT (reent, fp);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
   _newlib_flockfile_start (fp);
   result = __sputc_r (reent, c, fp);
   _newlib_flockfile_end (fp);
diff --git a/newlib/libc/stdio/puts.c b/newlib/libc/stdio/puts.c
index d4d7b7ffb..20d889b81 100644
--- a/newlib/libc/stdio/puts.c
+++ b/newlib/libc/stdio/puts.c
@@ -87,8 +87,10 @@ _puts_r (struct _reent *ptr,
   fp = _stdout_r (ptr);
   CHECK_INIT (ptr, fp);
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
-  result = (__sfvwrite_r (ptr, fp, &uio) ? EOF : '\n');
+  if (ORIENT (fp, -1) != -1)
+    result = EOF;
+  else
+    result = (__sfvwrite_r (ptr, fp, &uio) ? EOF : '\n');
   _newlib_flockfile_end (fp);
   return result;
 #else
@@ -100,7 +102,8 @@ _puts_r (struct _reent *ptr,
   fp = _stdout_r (ptr);
   CHECK_INIT (ptr, fp);
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto err;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto err;
diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index 7bd38806c..c1ef7e120 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -43,7 +43,8 @@ __srefill_r (struct _reent * ptr,
 
   CHECK_INIT (ptr, fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
 
   fp->_r = 0;			/* largely a convenience for callers */
 
diff --git a/newlib/libc/stdio/ungetc.c b/newlib/libc/stdio/ungetc.c
index 533c28eac..79914af08 100644
--- a/newlib/libc/stdio/ungetc.c
+++ b/newlib/libc/stdio/ungetc.c
@@ -125,7 +125,11 @@ _ungetc_r (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      _newlib_flockfile_exit (fp);
+      return EOF;
+    }
 
   /* After ungetc, we won't be at eof anymore */
   fp->_flags &= ~__SEOF;
diff --git a/newlib/libc/stdio/ungetwc.c b/newlib/libc/stdio/ungetwc.c
index 16d37f2d1..18636d773 100644
--- a/newlib/libc/stdio/ungetwc.c
+++ b/newlib/libc/stdio/ungetwc.c
@@ -82,8 +82,9 @@ _ungetwc_r (struct _reent *ptr,
   size_t len;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
-  if (wc == WEOF)
+  if (ORIENT (fp, 1) != 1)
+    wc = WEOF;
+  else if (wc == WEOF)
     wc = WEOF;
   else if ((len = _wcrtomb_r(ptr, buf, wc, &fp->_mbstate)) == (size_t)-1)
     {
diff --git a/newlib/libc/stdio/vfprintf.c b/newlib/libc/stdio/vfprintf.c
index 6a198e2c6..42272abe4 100644
--- a/newlib/libc/stdio/vfprintf.c
+++ b/newlib/libc/stdio/vfprintf.c
@@ -845,7 +845,10 @@ _VFPRINTF_R (struct _reent *data,
 	CHECK_INIT (data, fp);
 	_newlib_flockfile_start (fp);
 
-	ORIENT(fp, -1);
+	if (ORIENT(fp, -1) != -1) {
+		_newlib_flockfile_exit (fp);
+		return (EOF);
+	}
 
 	/* sorry, fprintf(read_only_file, "") returns EOF, not 0 */
 	if (cantwrite (data, fp)) {
diff --git a/newlib/libc/stdio/vfscanf.c b/newlib/libc/stdio/vfscanf.c
index cfeea9876..71e2c3e22 100644
--- a/newlib/libc/stdio/vfscanf.c
+++ b/newlib/libc/stdio/vfscanf.c
@@ -589,7 +589,11 @@ __SVFSCANF_R (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      nassigned = EOF;
+      goto all_done;
+    }
 
   nassigned = 0;
   nread = 0;
diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c
index 7807a1229..bbabbdaee 100644
--- a/newlib/libc/stdio/vfwprintf.c
+++ b/newlib/libc/stdio/vfwprintf.c
@@ -588,7 +588,10 @@ _VFWPRINTF_R (struct _reent *data,
 	CHECK_INIT (data, fp);
 	_newlib_flockfile_start (fp);
 
-	ORIENT(fp, 1);
+	if (ORIENT(fp, 1) != 1) {
+		_newlib_flockfile_exit (fp);
+		return (EOF);
+	}
 
 	/* sorry, fwprintf(read_only_file, "") returns EOF, not 0 */
 	if (cantwrite (data, fp)) {
diff --git a/newlib/libc/stdio/vfwscanf.c b/newlib/libc/stdio/vfwscanf.c
index df966f929..d2f91dde2 100644
--- a/newlib/libc/stdio/vfwscanf.c
+++ b/newlib/libc/stdio/vfwscanf.c
@@ -516,7 +516,11 @@ __SVFWSCANF_R (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    {
+      nassigned = EOF;
+      goto all_done;
+    }
 
   nassigned = 0;
   nread = 0;
-- 
2.39.0

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Corinna Vinschen]

On Nov  9 06:48, Takashi Yano wrote:
> Previously, fprintf() on a wide-oriented stream crashes or outputs
> garbage. This is because a narrow char string which can be odd bytes
> in length is cast into a wide char string which should be even
> bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
> As a result, if the length is odd bytes, the reading buffer runs over
> the buffer length, which causes a crash. If the length is even bytes,
> garbage is printed.
> 
> With this patch, any output to the stream which is set to different
> orientation fails with error just like glibc. Note that it behaves
> differently from other libc implementations such as BSD, musl and
> Solaris.
> 
> Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> ---
>  newlib/libc/stdio/fgetwc.c    |  6 ++++--
>  newlib/libc/stdio/fgetwc_u.c  |  3 ++-
>  newlib/libc/stdio/fgetws.c    |  3 ++-
>  newlib/libc/stdio/fputs.c     |  9 ++++++---
>  newlib/libc/stdio/fputwc.c    |  6 ++++--
>  newlib/libc/stdio/fputwc_u.c  |  3 ++-
>  newlib/libc/stdio/fputws.c    |  6 ++++--
>  newlib/libc/stdio/fread.c     |  7 ++++++-
>  newlib/libc/stdio/fwrite.c    |  9 +++++++--
>  newlib/libc/stdio/local.h     | 31 +++++++++++++++++--------------
>  newlib/libc/stdio/putc.c      |  4 ++++
>  newlib/libc/stdio/puts.c      |  9 ++++++---
>  newlib/libc/stdio/refill.c    |  3 ++-
>  newlib/libc/stdio/ungetc.c    |  6 +++++-
>  newlib/libc/stdio/ungetwc.c   |  5 +++--
>  newlib/libc/stdio/vfprintf.c  |  5 ++++-
>  newlib/libc/stdio/vfscanf.c   |  6 +++++-
>  newlib/libc/stdio/vfwprintf.c |  5 ++++-
>  newlib/libc/stdio/vfwscanf.c  |  6 +++++-
>  19 files changed, 92 insertions(+), 40 deletions(-)

Looks good, please push.


Thanks,
Corinna

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Takashi Yano]

On Thu, 9 Nov 2023 11:08:03 +0100
Corinna Vinschen wrote:
> On Nov  9 06:48, Takashi Yano wrote:
> > Previously, fprintf() on a wide-oriented stream crashes or outputs
> > garbage. This is because a narrow char string which can be odd bytes
> > in length is cast into a wide char string which should be even
> > bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
> > As a result, if the length is odd bytes, the reading buffer runs over
> > the buffer length, which causes a crash. If the length is even bytes,
> > garbage is printed.
> > 
> > With this patch, any output to the stream which is set to different
> > orientation fails with error just like glibc. Note that it behaves
> > differently from other libc implementations such as BSD, musl and
> > Solaris.
> > 
> > Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
> > Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> > ---
> >  newlib/libc/stdio/fgetwc.c    |  6 ++++--
> >  newlib/libc/stdio/fgetwc_u.c  |  3 ++-
> >  newlib/libc/stdio/fgetws.c    |  3 ++-
> >  newlib/libc/stdio/fputs.c     |  9 ++++++---
> >  newlib/libc/stdio/fputwc.c    |  6 ++++--
> >  newlib/libc/stdio/fputwc_u.c  |  3 ++-
> >  newlib/libc/stdio/fputws.c    |  6 ++++--
> >  newlib/libc/stdio/fread.c     |  7 ++++++-
> >  newlib/libc/stdio/fwrite.c    |  9 +++++++--
> >  newlib/libc/stdio/local.h     | 31 +++++++++++++++++--------------
> >  newlib/libc/stdio/putc.c      |  4 ++++
> >  newlib/libc/stdio/puts.c      |  9 ++++++---
> >  newlib/libc/stdio/refill.c    |  3 ++-
> >  newlib/libc/stdio/ungetc.c    |  6 +++++-
> >  newlib/libc/stdio/ungetwc.c   |  5 +++--
> >  newlib/libc/stdio/vfprintf.c  |  5 ++++-
> >  newlib/libc/stdio/vfscanf.c   |  6 +++++-
> >  newlib/libc/stdio/vfwprintf.c |  5 ++++-
> >  newlib/libc/stdio/vfwscanf.c  |  6 +++++-
> >  19 files changed, 92 insertions(+), 40 deletions(-)
> 
> Looks good, please push.

Thanks. Should this also be applied to cygwin-3_4-branch?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Corinna Vinschen]

On Nov  9 19:26, Takashi Yano wrote:
> On Thu, 9 Nov 2023 11:08:03 +0100
> Corinna Vinschen wrote:
> > On Nov  9 06:48, Takashi Yano wrote:
> > > Previously, fprintf() on a wide-oriented stream crashes or outputs
> > > garbage. This is because a narrow char string which can be odd bytes
> > > in length is cast into a wide char string which should be even
> > > bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
> > > As a result, if the length is odd bytes, the reading buffer runs over
> > > the buffer length, which causes a crash. If the length is even bytes,
> > > garbage is printed.
> > > 
> > > With this patch, any output to the stream which is set to different
> > > orientation fails with error just like glibc. Note that it behaves
> > > differently from other libc implementations such as BSD, musl and
> > > Solaris.
> > > 
> > > Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
> > > Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> > > ---
> > >  newlib/libc/stdio/fgetwc.c    |  6 ++++--
> > >  newlib/libc/stdio/fgetwc_u.c  |  3 ++-
> > >  newlib/libc/stdio/fgetws.c    |  3 ++-
> > >  newlib/libc/stdio/fputs.c     |  9 ++++++---
> > >  newlib/libc/stdio/fputwc.c    |  6 ++++--
> > >  newlib/libc/stdio/fputwc_u.c  |  3 ++-
> > >  newlib/libc/stdio/fputws.c    |  6 ++++--
> > >  newlib/libc/stdio/fread.c     |  7 ++++++-
> > >  newlib/libc/stdio/fwrite.c    |  9 +++++++--
> > >  newlib/libc/stdio/local.h     | 31 +++++++++++++++++--------------
> > >  newlib/libc/stdio/putc.c      |  4 ++++
> > >  newlib/libc/stdio/puts.c      |  9 ++++++---
> > >  newlib/libc/stdio/refill.c    |  3 ++-
> > >  newlib/libc/stdio/ungetc.c    |  6 +++++-
> > >  newlib/libc/stdio/ungetwc.c   |  5 +++--
> > >  newlib/libc/stdio/vfprintf.c  |  5 ++++-
> > >  newlib/libc/stdio/vfscanf.c   |  6 +++++-
> > >  newlib/libc/stdio/vfwprintf.c |  5 ++++-
> > >  newlib/libc/stdio/vfwscanf.c  |  6 +++++-
> > >  19 files changed, 92 insertions(+), 40 deletions(-)
> > 
> > Looks good, please push.
> 
> Thanks. Should this also be applied to cygwin-3_4-branch?

Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
situation.  And it's also a behavioral change.  So, from my POV we
shouldn't backport it.

But if you have another POV, we can discuss it.  It occured to me that
you didn't mention where the testcase is coming from.  Was that a
real-world problem?  If so, where and in which circumstances?


Corinna

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Takashi Yano]

On Thu, 9 Nov 2023 13:49:41 +0100
Corinna Vinschen wrote:
> On Nov  9 19:26, Takashi Yano wrote:
> > On Thu, 9 Nov 2023 11:08:03 +0100
> > Corinna Vinschen wrote:
> > > Looks good, please push.
> > 
> > Thanks. Should this also be applied to cygwin-3_4-branch?
> 
> Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
> situation.  And it's also a behavioral change.  So, from my POV we
> shouldn't backport it.

I see. I'll push it only for master branch.

> But if you have another POV, we can discuss it.  It occured to me that
> you didn't mention where the testcase is coming from.  Was that a
> real-world problem?  If so, where and in which circumstances?

No. It was discovered by an accidental mistake while writing
a test case for another problem.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Christophe Lyon]

Hi!

On Thu, 9 Nov 2023 at 16:36, Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> On Thu, 9 Nov 2023 13:49:41 +0100
> Corinna Vinschen wrote:
> > On Nov  9 19:26, Takashi Yano wrote:
> > > On Thu, 9 Nov 2023 11:08:03 +0100
> > > Corinna Vinschen wrote:
> > > > Looks good, please push.
> > >
> > > Thanks. Should this also be applied to cygwin-3_4-branch?
> >
> > Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
> > situation.  And it's also a behavioral change.  So, from my POV we
> > shouldn't backport it.
>
> I see. I'll push it only for master branch.
>
> > But if you have another POV, we can discuss it.  It occured to me that
> > you didn't mention where the testcase is coming from.  Was that a
> > real-world problem?  If so, where and in which circumstances?
>
> No. It was discovered by an accidental mistake while writing
> a test case for another problem.
>

After this patch, our automated CI has detected regressions on arm-none-eabi
with GCC configured with:
--disable-multilib --with-mode=thumb --with-cpu=cortex-m33 --with-float=hard
(all these options are probably not mandatory, we have other
configurations pending in the build queue)

The regressions appear in the libstdc++ tests:
Running libstdc++:libstdc++-dg/conformance.exp ...
FAIL: 27_io/objects/wchar_t/12048-1.cc -std=gnu++17 execution test
FAIL: 27_io/objects/wchar_t/12048-2.cc -std=gnu++17 execution test
FAIL: 27_io/objects/wchar_t/12048-3.cc -std=gnu++17 execution test
FAIL: 27_io/objects/wchar_t/12048-4.cc -std=gnu++17 execution test
XPASS: 27_io/objects/wchar_t/12048-5.cc -std=gnu++17 execution test
FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc -std=gnu++17 execution test
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc -std=gnu++17 execution test
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc -std=gnu++17 execution test
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc -std=gnu++17 execution test
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc -std=gnu++17 execution test

the logs show:
/libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-1.cc:37: void
test01(): Assertion 'std::wcin.good()' failed.
FAIL: 27_io/objects/wchar_t/12048-1.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-2.cc:34: void
test01(): Assertion 'std::wcin.good()' failed.
FAIL: 27_io/objects/wchar_t/12048-2.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-3.cc:34: void
test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
FAIL: 27_io/objects/wchar_t/12048-3.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-4.cc:31: void
test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
FAIL: 27_io/objects/wchar_t/12048-4.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/1.cc:43: void
test01(): Assertion 'traits_type::to_char_type(wsbuf.sgetc()) ==
w_lit[0]' failed.
FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-1.cc:31:
void test02(): Assertion 'c1 != WEOF' failed.
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-2.cc:31:
void test03(): Assertion 'c1 != WEOF' failed.
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-3.cc:31:
void test04(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc  -std=gnu++17 execution test
/libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-4.cc:31:
void test05(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc  -std=gnu++17 execution test

I haven't dug further yet, does that ring a bell?

Thanks,

Christophe

> --
> Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Takashi Yano]

On Fri, 10 Nov 2023 11:18:57 +0100
Christophe Lyon wrote:
> Hi!
> 
> On Thu, 9 Nov 2023 at 16:36, Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> >
> > On Thu, 9 Nov 2023 13:49:41 +0100
> > Corinna Vinschen wrote:
> > > On Nov  9 19:26, Takashi Yano wrote:
> > > > On Thu, 9 Nov 2023 11:08:03 +0100
> > > > Corinna Vinschen wrote:
> > > > > Looks good, please push.
> > > >
> > > > Thanks. Should this also be applied to cygwin-3_4-branch?
> > >
> > > Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
> > > situation.  And it's also a behavioral change.  So, from my POV we
> > > shouldn't backport it.
> >
> > I see. I'll push it only for master branch.
> >
> > > But if you have another POV, we can discuss it.  It occured to me that
> > > you didn't mention where the testcase is coming from.  Was that a
> > > real-world problem?  If so, where and in which circumstances?
> >
> > No. It was discovered by an accidental mistake while writing
> > a test case for another problem.
> >
> 
> After this patch, our automated CI has detected regressions on arm-none-eabi
> with GCC configured with:
> --disable-multilib --with-mode=thumb --with-cpu=cortex-m33 --with-float=hard
> (all these options are probably not mandatory, we have other
> configurations pending in the build queue)
> 
> The regressions appear in the libstdc++ tests:
> Running libstdc++:libstdc++-dg/conformance.exp ...
> FAIL: 27_io/objects/wchar_t/12048-1.cc -std=gnu++17 execution test
> FAIL: 27_io/objects/wchar_t/12048-2.cc -std=gnu++17 execution test
> FAIL: 27_io/objects/wchar_t/12048-3.cc -std=gnu++17 execution test
> FAIL: 27_io/objects/wchar_t/12048-4.cc -std=gnu++17 execution test
> XPASS: 27_io/objects/wchar_t/12048-5.cc -std=gnu++17 execution test
> FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc -std=gnu++17 execution test
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc -std=gnu++17 execution test
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc -std=gnu++17 execution test
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc -std=gnu++17 execution test
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc -std=gnu++17 execution test
> 
> the logs show:
> /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-1.cc:37: void
> test01(): Assertion 'std::wcin.good()' failed.
> FAIL: 27_io/objects/wchar_t/12048-1.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-2.cc:34: void
> test01(): Assertion 'std::wcin.good()' failed.
> FAIL: 27_io/objects/wchar_t/12048-2.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-3.cc:34: void
> test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
> FAIL: 27_io/objects/wchar_t/12048-3.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-4.cc:31: void
> test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
> FAIL: 27_io/objects/wchar_t/12048-4.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/1.cc:43: void
> test01(): Assertion 'traits_type::to_char_type(wsbuf.sgetc()) ==
> w_lit[0]' failed.
> FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-1.cc:31:
> void test02(): Assertion 'c1 != WEOF' failed.
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-2.cc:31:
> void test03(): Assertion 'c1 != WEOF' failed.
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-3.cc:31:
> void test04(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc  -std=gnu++17 execution test
> /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-4.cc:31:
> void test05(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
> FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc  -std=gnu++17 execution test
> 
> I haven't dug further yet, does that ring a bell?

Thanks for the report. Are these all of the additional failure?
I checked the previous patch and found the cause. I'll submit
a patch to fix this.

I confirmed the above errors are resolved with the new patch.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Christophe Lyon]

On Fri, 10 Nov 2023 at 12:34, Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> On Fri, 10 Nov 2023 11:18:57 +0100
> Christophe Lyon wrote:
> > Hi!
> >
> > On Thu, 9 Nov 2023 at 16:36, Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > >
> > > On Thu, 9 Nov 2023 13:49:41 +0100
> > > Corinna Vinschen wrote:
> > > > On Nov  9 19:26, Takashi Yano wrote:
> > > > > On Thu, 9 Nov 2023 11:08:03 +0100
> > > > > Corinna Vinschen wrote:
> > > > > > Looks good, please push.
> > > > >
> > > > > Thanks. Should this also be applied to cygwin-3_4-branch?
> > > >
> > > > Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
> > > > situation.  And it's also a behavioral change.  So, from my POV we
> > > > shouldn't backport it.
> > >
> > > I see. I'll push it only for master branch.
> > >
> > > > But if you have another POV, we can discuss it.  It occured to me that
> > > > you didn't mention where the testcase is coming from.  Was that a
> > > > real-world problem?  If so, where and in which circumstances?
> > >
> > > No. It was discovered by an accidental mistake while writing
> > > a test case for another problem.
> > >
> >
> > After this patch, our automated CI has detected regressions on arm-none-eabi
> > with GCC configured with:
> > --disable-multilib --with-mode=thumb --with-cpu=cortex-m33 --with-float=hard
> > (all these options are probably not mandatory, we have other
> > configurations pending in the build queue)
> >
> > The regressions appear in the libstdc++ tests:
> > Running libstdc++:libstdc++-dg/conformance.exp ...
> > FAIL: 27_io/objects/wchar_t/12048-1.cc -std=gnu++17 execution test
> > FAIL: 27_io/objects/wchar_t/12048-2.cc -std=gnu++17 execution test
> > FAIL: 27_io/objects/wchar_t/12048-3.cc -std=gnu++17 execution test
> > FAIL: 27_io/objects/wchar_t/12048-4.cc -std=gnu++17 execution test
> > XPASS: 27_io/objects/wchar_t/12048-5.cc -std=gnu++17 execution test
> > FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc -std=gnu++17 execution test
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc -std=gnu++17 execution test
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc -std=gnu++17 execution test
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc -std=gnu++17 execution test
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc -std=gnu++17 execution test
> >
> > the logs show:
> > /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-1.cc:37: void
> > test01(): Assertion 'std::wcin.good()' failed.
> > FAIL: 27_io/objects/wchar_t/12048-1.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-2.cc:34: void
> > test01(): Assertion 'std::wcin.good()' failed.
> > FAIL: 27_io/objects/wchar_t/12048-2.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-3.cc:34: void
> > test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
> > FAIL: 27_io/objects/wchar_t/12048-3.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/27_io/objects/wchar_t/12048-4.cc:31: void
> > test01(): Assertion 'std::wcin.rdbuf()->sgetn(buf, 2) == 2' failed.
> > FAIL: 27_io/objects/wchar_t/12048-4.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/1.cc:43: void
> > test01(): Assertion 'traits_type::to_char_type(wsbuf.sgetc()) ==
> > w_lit[0]' failed.
> > FAIL: ext/stdio_sync_filebuf/wchar_t/1.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-1.cc:31:
> > void test02(): Assertion 'c1 != WEOF' failed.
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-1.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-2.cc:31:
> > void test03(): Assertion 'c1 != WEOF' failed.
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-2.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-3.cc:31:
> > void test04(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-3.cc  -std=gnu++17 execution test
> > /libstdc++-v3/testsuite/ext/stdio_sync_filebuf/wchar_t/12948-4.cc:31:
> > void test05(): Assertion 'sbuf.sgetn(buf, 2) == 2' failed.
> > FAIL: ext/stdio_sync_filebuf/wchar_t/12948-4.cc  -std=gnu++17 execution test
> >
> > I haven't dug further yet, does that ring a bell?
>
> Thanks for the report. Are these all of the additional failure?
Yes.

> I checked the previous patch and found the cause. I'll submit
> a patch to fix this.
>
> I confirmed the above errors are resolved with the new patch.

Great! Nice that you could reproduce and fix the problem so quickly!

Christophe

>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>