[committed 2.40 0/5] vfscanf test case and ungetc fix

[committed 2.40 0/5] vfscanf test case and ungetc fix

[Siddhesh Poyarekar]

Backport the ungetc fix for BZ #27821 and also vfscanf test case to
match the fix for BZ #27650.

Maciej W. Rozycki (2):
  support: Add FAIL test failure helper
  stdio-common: Add test for vfscanf with matches longer than INT_MAX
    [BZ #27650]

Siddhesh Poyarekar (3):
  Make tst-ungetc use libsupport
  ungetc: Fix uninitialized read when putting into unused streams [BZ
    #27821]
  ungetc: Fix backup buffer leak on program exit [BZ #27821]

 debug/tst-fortify-syslog.c       |   8 +--
 libio/genops.c                   |   8 ++-
 libio/libioP.h                   |   4 +-
 localedata/tst-ctype.c           |  40 +++--------
 manual/stdio.texi                |   8 +--
 math/test-tgmath2.c              |  13 +---
 stdio-common/Makefile            |  12 ++++
 stdio-common/tst-scanf-bz27650.c | 108 +++++++++++++++++++++++++++++
 stdio-common/tst-ungetc-leak.c   |  32 +++++++++
 stdio-common/tst-ungetc.c        | 114 ++++++++++++++++---------------
 support/check.h                  |   5 ++
 11 files changed, 241 insertions(+), 111 deletions(-)
 create mode 100644 stdio-common/tst-scanf-bz27650.c
 create mode 100644 stdio-common/tst-ungetc-leak.c

-- 
2.45.1

[committed 2.40 1/5] support: Add FAIL test failure helper

[Siddhesh Poyarekar]

From: "Maciej W. Rozycki" <macro@redhat.com>

Add a FAIL test failure helper analogous to FAIL_RET, that does not
cause the current function to return, providing a standardized way to
report a test failure with a message supplied while permitting the
caller to continue executing, for further reporting, cleaning up, etc.

Update existing test cases that provide a conflicting definition of FAIL
by removing the local FAIL definition and then as follows:

- tst-fortify-syslog: provide a meaningful message in addition to the
  file name already added by <support/check.h>; 'support_record_failure'
  is already called by 'support_print_failure_impl' invoked by the new
  FAIL test failure helper.

- tst-ctype: no update to FAIL calls required, with the name of the file
  and the line number within of the failure site additionally included
  by the new FAIL test failure helper, and error counting plus count
  reporting upon test program termination also already provided by
  'support_record_failure' and 'support_report_failure' respectively,
  called by 'support_print_failure_impl' and 'adjust_exit_status' also
  respectively.  However in a number of places 'printf' is called and
  the error count adjusted by hand, so update these places to make use
  of FAIL instead.  And last but not least adjust the final summary just
  to report completion, with any error count following as reported by
  the test driver.

- test-tgmath2: no update to FAIL calls required, with the name of the
  file of the failure site additionally included by the new FAIL test
  failure helper.  Also there is no need to track the return status by
  hand as any call to FAIL will eventually cause the test case to return
  an unsuccesful exit status regardless of the return status from the
  test function, via a call to 'adjust_exit_status' made by the test
  driver.

Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 1b97a9f23bf605ca608162089c94187573fb2a9e)
---
 debug/tst-fortify-syslog.c |  8 +-------
 localedata/tst-ctype.c     | 40 +++++++++-----------------------------
 math/test-tgmath2.c        | 13 +++----------
 support/check.h            |  5 +++++
 4 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/debug/tst-fortify-syslog.c b/debug/tst-fortify-syslog.c
index a7ddbf7c6b..2712acf689 100644
--- a/debug/tst-fortify-syslog.c
+++ b/debug/tst-fortify-syslog.c
@@ -22,7 +22,6 @@
 #include <syslog.h>
 #include <string.h>
 #include <unistd.h>
-#include <stdio.h>
 
 #include <support/check.h>
 #include <support/support.h>
@@ -46,18 +45,13 @@ handler (int sig)
     _exit (127);
 }
 
-#define FAIL()						\
-  do {							\
-      printf ("Failure on line %d\n", __LINE__);	\
-      support_record_failure ();			\
-  } while (0)
 #define CHK_FAIL_START					\
   chk_fail_ok = 1;					\
   if (! setjmp (chk_fail_buf))				\
     {
 #define CHK_FAIL_END					\
       chk_fail_ok = 0;					\
-      FAIL ();						\
+      FAIL ("not supposed to reach here");		\
     }
 
 static void
diff --git a/localedata/tst-ctype.c b/localedata/tst-ctype.c
index 9de979a2d7..a23689719c 100644
--- a/localedata/tst-ctype.c
+++ b/localedata/tst-ctype.c
@@ -21,6 +21,8 @@
 #include <stdio.h>
 #include <string.h>
 
+#include <support/check.h>
+
 
 static const char lower[] = "abcdefghijklmnopqrstuvwxyz";
 static const char upper[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
@@ -53,19 +55,11 @@ static struct classes
 #define nclasses (sizeof (classes) / sizeof (classes[0]))
 
 
-#define FAIL(str, args...) \
-  {									      \
-    printf ("      " str "\n", ##args);					      \
-    ++errors;								      \
-  }
-
-
 static int
 do_test (void)
 {
   const char *cp;
   const char *cp2;
-  int errors = 0;
   char *inpline = NULL;
   size_t inplinelen = 0;
   char *resline = NULL;
@@ -394,11 +388,8 @@ punct = %04x  alnum = %04x\n",
 	    {
 	      if (((__ctype_b[(unsigned int) *inp] & classes[n].mask) != 0)
 		  != (*resp != '0'))
-		{
-		  printf ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
-			  *inp, *inp, *resp == '1' ? "not" : "is");
-		  ++errors;
-		}
+		FAIL ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
+		      *inp, *inp, *resp == '1' ? "not" : "is");
 	      ++inp;
 	      ++resp;
 	    }
@@ -408,11 +399,8 @@ punct = %04x  alnum = %04x\n",
 	  while (*inp != '\0')
 	    {
 	      if (tolower (*inp) != *resp)
-		{
-		  printf ("    tolower('%c' = '\\x%02x') != '%c'\n",
-			  *inp, *inp, *resp);
-		  ++errors;
-		}
+		FAIL ("    tolower('%c' = '\\x%02x') != '%c'\n",
+		      *inp, *inp, *resp);
 	      ++inp;
 	      ++resp;
 	    }
@@ -422,11 +410,8 @@ punct = %04x  alnum = %04x\n",
 	  while (*inp != '\0')
 	    {
 	      if (toupper (*inp) != *resp)
-		{
-		  printf ("    toupper('%c' = '\\x%02x') != '%c'\n",
-			  *inp, *inp, *resp);
-		  ++errors;
-		}
+		FAIL ("    toupper('%c' = '\\x%02x') != '%c'\n",
+		      *inp, *inp, *resp);
 	      ++inp;
 	      ++resp;
 	    }
@@ -436,14 +421,7 @@ punct = %04x  alnum = %04x\n",
     }
 
 
-  if (errors != 0)
-    {
-      printf ("  %d error%s for `%s' locale\n\n\n", errors,
-	      errors == 1 ? "" : "s", setlocale (LC_ALL, NULL));
-      return 1;
-    }
-
-  printf ("  No errors for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
+  printf ("Completed testing for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
   return 0;
 }
 
diff --git a/math/test-tgmath2.c b/math/test-tgmath2.c
index 37afa8a08a..4aeb877b8e 100644
--- a/math/test-tgmath2.c
+++ b/math/test-tgmath2.c
@@ -24,6 +24,8 @@
 #include <string.h>
 #include <tgmath.h>
 
+#include <support/check.h>
+
 //#define DEBUG
 
 typedef complex float cfloat;
@@ -87,13 +89,6 @@ enum
 int count;
 int counts[Tlast][C_last];
 
-#define FAIL(str) \
-  do								\
-    {								\
-      printf ("%s failure on line %d\n", (str), __LINE__);	\
-      result = 1;						\
-    }								\
-  while (0)
 #define TEST_TYPE_ONLY(expr, rettype) \
   do								\
     {								\
@@ -133,8 +128,6 @@ int counts[Tlast][C_last];
 int
 test_cos (const int Vint4, const long long int Vllong4)
 {
-  int result = 0;
-
   TEST (cos (vfloat1), float, cos);
   TEST (cos (vdouble1), double, cos);
   TEST (cos (vldouble1), ldouble, cos);
@@ -152,7 +145,7 @@ test_cos (const int Vint4, const long long int Vllong4)
   TEST (cos (Vcdouble1), cdouble, cos);
   TEST (cos (Vcldouble1), cldouble, cos);
 
-  return result;
+  return 0;
 }
 
 int
diff --git a/support/check.h b/support/check.h
index 711f34b83b..7ea22c7a2c 100644
--- a/support/check.h
+++ b/support/check.h
@@ -24,6 +24,11 @@
 
 __BEGIN_DECLS
 
+/* Record a test failure, print the failure message to standard output
+   and pass the result of 1 through.  */
+#define FAIL(...) \
+  support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__)
+
 /* Record a test failure, print the failure message to standard output
    and return 1.  */
 #define FAIL_RET(...) \
-- 
2.45.1

[committed 2.40 2/5] stdio-common: Add test for vfscanf with matches longer than INT_MAX [BZ #27650]

[Siddhesh Poyarekar]

From: "Maciej W. Rozycki" <macro@redhat.com>

Complement commit b03e4d7bd25b ("stdio: fix vfscanf with matches longer
than INT_MAX (bug 27650)") and add a test case for the issue, inspired
by the reproducer provided with the bug report.

This has been verified to succeed as from the commit referred and fail
beforehand.

As the test requires 2GiB of data to be passed around its performance
has been evaluated using a choice of systems and the execution time
determined to be respectively in the range of 9s for POWER9@2.166GHz,
24s for FU740@1.2GHz, and 40s for 74Kf@950MHz.  As this is on the verge
of and beyond the default timeout it has been increased by the factor of
8.  Regardless, following recent practice the test has been added to the
standard rather than extended set.

Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 89cddc8a7096f3d9225868304d2bc0a1aaf07d63)
---
 stdio-common/Makefile            |   5 ++
 stdio-common/tst-scanf-bz27650.c | 108 +++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 stdio-common/tst-scanf-bz27650.c

diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index a63c05a120..e4f0146d2c 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -240,6 +240,7 @@ tests := \
   tst-scanf-binary-c23 \
   tst-scanf-binary-gnu11 \
   tst-scanf-binary-gnu89 \
+  tst-scanf-bz27650 \
   tst-scanf-intn \
   tst-scanf-round \
   tst-scanf-to_inpunct \
@@ -328,6 +329,7 @@ generated += \
   tst-printf-fp-free.mtrace \
   tst-printf-fp-leak-mem.out \
   tst-printf-fp-leak.mtrace \
+  tst-scanf-bz27650.mtrace \
   tst-vfprintf-width-prec-mem.out \
   tst-vfprintf-width-prec.mtrace \
   # generated
@@ -419,6 +421,9 @@ tst-printf-fp-free-ENV = \
 tst-printf-fp-leak-ENV = \
   MALLOC_TRACE=$(objpfx)tst-printf-fp-leak.mtrace \
   LD_PRELOAD=$(common-objpfx)/malloc/libc_malloc_debug.so
+tst-scanf-bz27650-ENV = \
+  MALLOC_TRACE=$(objpfx)tst-scanf-bz27650.mtrace \
+  LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so
 
 $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
 	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
diff --git a/stdio-common/tst-scanf-bz27650.c b/stdio-common/tst-scanf-bz27650.c
new file mode 100644
index 0000000000..3a742bc865
--- /dev/null
+++ b/stdio-common/tst-scanf-bz27650.c
@@ -0,0 +1,108 @@
+/* Test for BZ #27650, formatted input matching beyond INT_MAX.
+   Copyright (C) 2024 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <error.h>
+#include <errno.h>
+#include <limits.h>
+#include <mcheck.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <sys/types.h>
+
+#include <support/check.h>
+#include <support/test-driver.h>
+
+/* Produce a stream of more than INT_MAX characters via buffer BUF of
+   size SIZE according to bookkeeping in COOKIE and then return EOF.  */
+
+static ssize_t
+io_read (void *cookie, char *buf, size_t size)
+{
+  unsigned int *written = cookie;
+  unsigned int w = *written;
+
+  if (w > INT_MAX)
+    return 0;
+
+  memset (buf, 'a', size);
+  *written = w + size;
+  return size;
+}
+
+/* Consume a stream of more than INT_MAX characters from an artificial
+   input stream of which none is the new line character.  The call to
+   fscanf is supposed to complete upon the EOF condition of input,
+   however in the presence of BZ #27650 it will terminate prematurely
+   with characters still outstanding in input.  Diagnose the condition
+   and return status accordingly.  */
+
+int
+do_test (void)
+{
+  static cookie_io_functions_t io_funcs = { .read = io_read };
+  unsigned int written = 0;
+  FILE *in;
+  int v;
+
+  mtrace ();
+
+  in = fopencookie (&written, "r", io_funcs);
+  if (in == NULL)
+    {
+      FAIL ("fopencookie: %m");
+      goto out;
+    }
+
+  v = fscanf (in, "%*[^\n]");
+  if (ferror (in))
+    {
+      FAIL ("fscanf: input failure, at %u: %m", written);
+      goto out_close;
+    }
+  else if (v == EOF)
+    {
+      FAIL ("fscanf: unexpected end of file, at %u", written);
+      goto out_close;
+    }
+
+  if (!feof (in))
+    {
+      v = fgetc (in);
+      if (ferror (in))
+	FAIL ("fgetc: input failure: %m");
+      else if (v == EOF)
+	FAIL ("fgetc: unexpected end of file after missing end of file");
+      else if (v == '\n')
+	FAIL ("unexpected new line character received");
+      else
+	FAIL ("character received after end of file expected: \\x%02x", v);
+    }
+
+out_close:
+  if (fclose (in) != 0)
+    FAIL ("fclose: %m");
+
+out:
+  return EXIT_SUCCESS;
+}
+
+#define TIMEOUT (DEFAULT_TIMEOUT * 8)
+#include <support/test-driver.c>
-- 
2.45.1

[committed 2.40 3/5] Make tst-ungetc use libsupport

[Siddhesh Poyarekar]

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
(cherry picked from commit 3f7df7e757f4efec38e45d4068e5492efcac4856)
---
 stdio-common/tst-ungetc.c | 112 +++++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 55 deletions(-)

diff --git a/stdio-common/tst-ungetc.c b/stdio-common/tst-ungetc.c
index 1344b2b591..5c808f0734 100644
--- a/stdio-common/tst-ungetc.c
+++ b/stdio-common/tst-ungetc.c
@@ -1,70 +1,72 @@
-/* Test for ungetc bugs.  */
+/* Test for ungetc bugs.
+   Copyright (C) 1996-2024 Free Software Foundation, Inc.
+   Copyright The GNU Toolchain Authors.
+   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
+   <https://www.gnu.org/licenses/>.  */
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
-
-#undef assert
-#define assert(x) \
-  if (!(x)) \
-    { \
-      fputs ("test failed: " #x "\n", stderr); \
-      retval = 1; \
-      goto the_end; \
-    }
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
 
-int
-main (int argc, char *argv[])
+static int
+do_test (void)
 {
-  char name[] = "/tmp/tst-ungetc.XXXXXX";
+  char *name = NULL;
   FILE *fp = NULL;
-  int retval = 0;
   int c;
   char buffer[64];
 
-  int fd = mkstemp (name);
+  int fd = create_temp_file ("tst-ungetc.", &name);
   if (fd == -1)
-    {
-      printf ("mkstemp failed: %m\n");
-      return 1;
-    }
-  close (fd);
-  fp = fopen (name, "w");
-  assert (fp != NULL)
-  fputs ("bla", fp);
-  fclose (fp);
-  fp = NULL;
+    FAIL_EXIT1 ("cannot create temporary file: %m");
+  xclose (fd);
 
-  fp = fopen (name, "r");
-  assert (fp != NULL);
-  assert (ungetc ('z', fp) == 'z');
-  assert (getc (fp) == 'z');
-  assert (getc (fp) == 'b');
-  assert (getc (fp) == 'l');
-  assert (ungetc ('m', fp) == 'm');
-  assert (getc (fp) == 'm');
-  assert ((c = getc (fp)) == 'a');
-  assert (getc (fp) == EOF);
-  assert (ungetc (c, fp) == c);
-  assert (feof (fp) == 0);
-  assert (getc (fp) == c);
-  assert (getc (fp) == EOF);
-  fclose (fp);
-  fp = NULL;
+  fp = xfopen (name, "w");
+  fputs ("bla", fp);
+  xfclose (fp);
 
-  fp = fopen (name, "r");
-  assert (fp != NULL);
-  assert (getc (fp) == 'b');
-  assert (getc (fp) == 'l');
-  assert (ungetc ('b', fp) == 'b');
-  assert (fread (buffer, 1, 64, fp) == 2);
-  assert (buffer[0] == 'b');
-  assert (buffer[1] == 'a');
+  fp = xfopen (name, "r");
+  TEST_VERIFY_EXIT (ungetc ('z', fp) == 'z');
+  TEST_VERIFY_EXIT (getc (fp) == 'z');
+  TEST_VERIFY_EXIT (getc (fp) == 'b');
+  TEST_VERIFY_EXIT (getc (fp) == 'l');
+  TEST_VERIFY_EXIT (ungetc ('m', fp) == 'm');
+  TEST_VERIFY_EXIT (getc (fp) == 'm');
+  TEST_VERIFY_EXIT ((c = getc (fp)) == 'a');
+  TEST_VERIFY_EXIT (getc (fp) == EOF);
+  TEST_VERIFY_EXIT (ungetc (c, fp) == c);
+  TEST_VERIFY_EXIT (feof (fp) == 0);
+  TEST_VERIFY_EXIT (getc (fp) == c);
+  TEST_VERIFY_EXIT (getc (fp) == EOF);
+  xfclose (fp);
 
-the_end:
-  if (fp != NULL)
-    fclose (fp);
-  unlink (name);
+  fp = xfopen (name, "r");
+  TEST_VERIFY_EXIT (getc (fp) == 'b');
+  TEST_VERIFY_EXIT (getc (fp) == 'l');
+  TEST_VERIFY_EXIT (ungetc ('b', fp) == 'b');
+  TEST_VERIFY_EXIT (fread (buffer, 1, 64, fp) == 2);
+  TEST_VERIFY_EXIT (buffer[0] == 'b');
+  TEST_VERIFY_EXIT (buffer[1] == 'a');
+  xfclose (fp);
 
-  return retval;
+  return 0;
 }
+
+#include <support/test-driver.c>
-- 
2.45.1

[committed 2.40 4/5] ungetc: Fix uninitialized read when putting into unused streams [BZ #27821]

[Siddhesh Poyarekar]

When ungetc is called on an unused stream, the backup buffer is
allocated without the main get area being present.  This results in
every subsequent ungetc (as the stream remains in the backup area)
checking uninitialized memory in the backup buffer when trying to put a
character back into the stream.

Avoid comparing the input character with buffer contents when in backup
to avoid this uninitialized read.  The uninitialized read is harmless in
this context since the location is promptly overwritten with the input
character, thus fulfilling ungetc functionality.

Also adjust wording in the manual to drop the paragraph that says glibc
cannot do multiple ungetc back to back since with this change, ungetc
can actually do this.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
(cherry picked from commit cdf0f88f97b0aaceb894cc02b21159d148d7065c)
---
 libio/genops.c            | 2 +-
 manual/stdio.texi         | 8 +++-----
 stdio-common/tst-ungetc.c | 2 ++
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libio/genops.c b/libio/genops.c
index 99f5e80f20..b012fa33d2 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -662,7 +662,7 @@ _IO_sputbackc (FILE *fp, int c)
 {
   int result;
 
-  if (fp->_IO_read_ptr > fp->_IO_read_base
+  if (fp->_IO_read_ptr > fp->_IO_read_base && !_IO_in_backup (fp)
       && (unsigned char)fp->_IO_read_ptr[-1] == (unsigned char)c)
     {
       fp->_IO_read_ptr--;
diff --git a/manual/stdio.texi b/manual/stdio.texi
index 8517653507..92614775fa 100644
--- a/manual/stdio.texi
+++ b/manual/stdio.texi
@@ -1467,11 +1467,9 @@ program; usually @code{ungetc} is used only to unread a character that
 was just read from the same stream.  @Theglibc{} supports this
 even on files opened in binary mode, but other systems might not.
 
-@Theglibc{} only supports one character of pushback---in other
-words, it does not work to call @code{ungetc} twice without doing input
-in between.  Other systems might let you push back multiple characters;
-then reading from the stream retrieves the characters in the reverse
-order that they were pushed.
+@Theglibc{} supports pushing back multiple characters; subsequently
+reading from the stream retrieves the characters in the reverse order
+that they were pushed.
 
 Pushing back characters doesn't alter the file; only the internal
 buffering for the stream is affected.  If a file positioning function
diff --git a/stdio-common/tst-ungetc.c b/stdio-common/tst-ungetc.c
index 5c808f0734..388b202493 100644
--- a/stdio-common/tst-ungetc.c
+++ b/stdio-common/tst-ungetc.c
@@ -48,6 +48,8 @@ do_test (void)
   TEST_VERIFY_EXIT (getc (fp) == 'b');
   TEST_VERIFY_EXIT (getc (fp) == 'l');
   TEST_VERIFY_EXIT (ungetc ('m', fp) == 'm');
+  TEST_VERIFY_EXIT (ungetc ('n', fp) == 'n');
+  TEST_VERIFY_EXIT (getc (fp) == 'n');
   TEST_VERIFY_EXIT (getc (fp) == 'm');
   TEST_VERIFY_EXIT ((c = getc (fp)) == 'a');
   TEST_VERIFY_EXIT (getc (fp) == EOF);
-- 
2.45.1

[committed 2.40 5/5] ungetc: Fix backup buffer leak on program exit [BZ #27821]

[Siddhesh Poyarekar]

If a file descriptor is left unclosed and is cleaned up by _IO_cleanup
on exit, its backup buffer remains unfreed, registering as a leak in
valgrind.  This is not strictly an issue since (1) the program should
ideally be closing the stream once it's not in use and (2) the program
is about to exit anyway, so keeping the backup buffer around a wee bit
longer isn't a real problem.  Free it anyway to keep valgrind happy
when the streams in question are the standard ones, i.e. stdout, stdin
or stderr.

Also, the _IO_have_backup macro checks for _IO_save_base,
which is a roundabout way to check for a backup buffer instead of
directly looking for _IO_backup_base.  The roundabout check breaks when
the main get area has not been used and user pushes a char into the
backup buffer with ungetc.  Fix this to use the _IO_backup_base
directly.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
(cherry picked from commit 3e1d8d1d1dca24ae90df2ea826a8916896fc7e77)
---
 libio/genops.c                 |  6 ++++++
 libio/libioP.h                 |  4 ++--
 stdio-common/Makefile          |  7 +++++++
 stdio-common/tst-ungetc-leak.c | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 stdio-common/tst-ungetc-leak.c

diff --git a/libio/genops.c b/libio/genops.c
index b012fa33d2..35d8b30710 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -816,6 +816,12 @@ _IO_unbuffer_all (void)
 	legacy = 1;
 #endif
 
+      /* Free up the backup area if it was ever allocated.  */
+      if (_IO_have_backup (fp))
+	_IO_free_backup_area (fp);
+      if (fp->_mode > 0 && _IO_have_wbackup (fp))
+	_IO_free_wbackup_area (fp);
+
       if (! (fp->_flags & _IO_UNBUFFERED)
 	  /* Iff stream is un-orientated, it wasn't used. */
 	  && (legacy || fp->_mode != 0))
diff --git a/libio/libioP.h b/libio/libioP.h
index 1af287b19f..616253fcd0 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -577,8 +577,8 @@ extern void _IO_old_init (FILE *fp, int flags) __THROW;
        ((__fp)->_wide_data->_IO_write_base \
 	= (__fp)->_wide_data->_IO_write_ptr = __p, \
 	(__fp)->_wide_data->_IO_write_end = (__ep))
-#define _IO_have_backup(fp) ((fp)->_IO_save_base != NULL)
-#define _IO_have_wbackup(fp) ((fp)->_wide_data->_IO_save_base != NULL)
+#define _IO_have_backup(fp) ((fp)->_IO_backup_base != NULL)
+#define _IO_have_wbackup(fp) ((fp)->_wide_data->_IO_backup_base != NULL)
 #define _IO_in_backup(fp) ((fp)->_flags & _IO_IN_BACKUP)
 #define _IO_have_markers(fp) ((fp)->_markers != NULL)
 #define _IO_blen(fp) ((fp)->_IO_buf_end - (fp)->_IO_buf_base)
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index e4f0146d2c..a91754f52d 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -254,6 +254,7 @@ tests := \
   tst-swscanf \
   tst-tmpnam \
   tst-ungetc \
+  tst-ungetc-leak \
   tst-unlockedio \
   tst-vfprintf-mbs-prec \
   tst-vfprintf-user-type \
@@ -316,6 +317,7 @@ tests-special += \
   $(objpfx)tst-printf-bz25691-mem.out \
   $(objpfx)tst-printf-fp-free-mem.out \
   $(objpfx)tst-printf-fp-leak-mem.out \
+  $(objpfx)tst-ungetc-leak-mem.out \
   $(objpfx)tst-vfprintf-width-prec-mem.out \
   # tests-special
 
@@ -330,6 +332,8 @@ generated += \
   tst-printf-fp-leak-mem.out \
   tst-printf-fp-leak.mtrace \
   tst-scanf-bz27650.mtrace \
+  tst-ungetc-leak-mem.out \
+  tst-ungetc-leak.mtrace \
   tst-vfprintf-width-prec-mem.out \
   tst-vfprintf-width-prec.mtrace \
   # generated
@@ -424,6 +428,9 @@ tst-printf-fp-leak-ENV = \
 tst-scanf-bz27650-ENV = \
   MALLOC_TRACE=$(objpfx)tst-scanf-bz27650.mtrace \
   LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so
+tst-ungetc-leak-ENV = \
+  MALLOC_TRACE=$(objpfx)tst-ungetc-leak.mtrace \
+  LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so
 
 $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
 	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
diff --git a/stdio-common/tst-ungetc-leak.c b/stdio-common/tst-ungetc-leak.c
new file mode 100644
index 0000000000..6c5152b43f
--- /dev/null
+++ b/stdio-common/tst-ungetc-leak.c
@@ -0,0 +1,32 @@
+/* Test for memory leak with ungetc when stream is unused.
+   Copyright The GNU Toolchain Authors.
+   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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <mcheck.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static int
+do_test (void)
+{
+  mtrace ();
+  TEST_COMPARE (ungetc('y', stdin), 'y');
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.45.1

Re: [committed 2.40 1/5] support: Add FAIL test failure helper

[Siddhesh Poyarekar]

On 2024-08-30 11:29, Siddhesh Poyarekar via Libc-stable wrote:
> From: "Maciej W. Rozycki" <macro@redhat.com>

Sorry, I missed a couple of other patches that fixed up test cases with 
overridden FAIL().  I'll backport those as well.

Sid