[PATCH] [BZ #22830] malloc_stats: restore cancellation for stderr correctly.

[Zack Weinberg <zackw at panix dot com>]

malloc_stats means to disable cancellation for writes to stderr while
it runs, but it restores stderr->_flags2 with |= instead of =, so what
it actually does is disable cancellation on stderr permanently.

I'm going to go ahead and commit this as an obvious bugfix, but I
would welcome feedback on the test case, which is rather complicated
and could probably be made more robust (for instance, setting stderr to
line-buffered breaks the test and I don't understand why).

Perhaps _IO_lock_acquire_clear_flags2 should be extended to handle
_IO_FLAGS2_NOTCANCEL as well as what it already does, but that would
_not_ be an obvious bugfix. :-)

	[BZ #22830]
	* malloc/malloc.c (__malloc_stats): Restore stderr->_flags2
        correctly.
        * malloc/tst-malloc-stats-cancellation.c: New test case.
        * malloc/Makefile: Add new test case.
---
 malloc/Makefile                        |   2 +
 malloc/malloc.c                        |   2 +-
 malloc/tst-malloc-stats-cancellation.c | 216 +++++++++++++++++++++++++++++++++
 3 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 malloc/tst-malloc-stats-cancellation.c

diff --git a/malloc/Makefile b/malloc/Makefile
index 17873e67c4b..7d54bad866f 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -37,6 +37,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-malloc-tcache-leak \
 	 tst-malloc_info \
 	 tst-malloc-too-large \
+	 tst-malloc-stats-cancellation \
 
 tests-static := \
 	 tst-interpose-static-nothread \
@@ -96,6 +97,7 @@ $(objpfx)tst-malloc-backtrace: $(shared-thread-library)
 $(objpfx)tst-malloc-thread-exit: $(shared-thread-library)
 $(objpfx)tst-malloc-thread-fail: $(shared-thread-library)
 $(objpfx)tst-malloc-fork-deadlock: $(shared-thread-library)
+$(objpfx)tst-malloc-stats-cancellation: $(shared-thread-library)
 
 # Export the __malloc_initialize_hook variable to libc.so.
 LDFLAGS-tst-mallocstate = -rdynamic
diff --git a/malloc/malloc.c b/malloc/malloc.c
index f8e7250f70f..3224b3a899f 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5009,7 +5009,7 @@ __malloc_stats (void)
   fprintf (stderr, "max mmap regions = %10u\n", (unsigned int) mp_.max_n_mmaps);
   fprintf (stderr, "max mmap bytes   = %10lu\n",
            (unsigned long) mp_.max_mmapped_mem);
-  ((_IO_FILE *) stderr)->_flags2 |= old_flags2;
+  ((_IO_FILE *) stderr)->_flags2 = old_flags2;
   _IO_funlockfile (stderr);
 }
 
diff --git a/malloc/tst-malloc-stats-cancellation.c b/malloc/tst-malloc-stats-cancellation.c
new file mode 100644
index 00000000000..9a8f475830b
--- /dev/null
+++ b/malloc/tst-malloc-stats-cancellation.c
@@ -0,0 +1,216 @@
+/* Bug 22830: malloc_stats fails to re-enable cancellation on exit.
+   Copyright (C) 2018 Free Software Foundation.
+   Copying and distribution of this file, with or without modification,
+   are permitted in any medium without royalty provided the copyright
+   notice and this notice are preserved. This file is offered as-is,
+   without any warranty.  */
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <pthread.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <malloc.h>
+
+static void *
+test_threadproc (void *gatep)
+{
+  /* When we are released from the barrier, there is a cancellation
+     request pending for this thread.  N.B. pthread_barrier_wait is
+     not itself a cancellation point (oddly enough).  */
+  pthread_barrier_wait ((pthread_barrier_t *)gatep);
+  malloc_stats ();
+  fputs ("this call should trigger cancellation\n", stderr);
+  return 0;
+}
+
+/* We cannot replace stderr with a memstream because writes to memstreams
+   do not trigger cancellation.  Instead, main swaps out fd 2 to point to
+   a pipe, and this thread reads from the pipe and writes to a memstream
+   until EOF, then returns the data accumulated in the memstream.  main
+   can't do that itself because, when the test thread gets cancelled,
+   it doesn't close the pipe.  */
+
+struct buffer_tp_args
+{
+  int ifd;
+  FILE *real_stderr;
+};
+
+static void *
+buffer_threadproc (void *argp)
+{
+  struct buffer_tp_args *args = argp;
+  int ifd = args->ifd;
+  char block[BUFSIZ], *p;
+  ssize_t nread;
+  size_t nwritten;
+
+  char *obuf = 0;
+  size_t obufsz = 0;
+  FILE *ofp = open_memstream (&obuf, &obufsz);
+  if (!ofp)
+    {
+      fprintf (args->real_stderr,
+               "buffer_threadproc: open_memstream: %s\n", strerror (errno));
+      return 0;
+    }
+
+  while ((nread = read (ifd, block, BUFSIZ)) > 0)
+    {
+      p = block;
+      do
+        {
+          nwritten = fwrite_unlocked (p, 1, nread, ofp);
+          if (nwritten == 0)
+            {
+              fprintf (args->real_stderr,
+                       "buffer_threadproc: fwrite_unlocked: %s\n",
+                       strerror (errno));
+              return 0;
+            }
+          nread -= nwritten;
+          p += nwritten;
+        }
+      while (nread > 0);
+    }
+  if (nread == -1)
+    {
+      fprintf (args->real_stderr, "buffer_threadproc: read: %s\n",
+               strerror (errno));
+      return 0;
+    }
+  close (ifd);
+  fclose (ofp);
+  return obuf;
+}
+
+
+int
+main (void)
+{
+  int result = 0, err, real_stderr_fd, bufpipe[2];
+  pthread_t t_thr, b_thr;
+  pthread_barrier_t gate;
+  void *rv;
+  FILE *real_stderr;
+  char *obuf;
+  void *obuf_v;
+  struct buffer_tp_args b_args;
+
+  real_stderr_fd = dup (2);
+  if (real_stderr_fd == -1)
+    {
+      perror ("dup");
+      return 2;
+    }
+  real_stderr = fdopen(real_stderr_fd, "w");
+  if (!real_stderr)
+    {
+      perror ("fdopen");
+      return 2;
+    }
+  if (setvbuf (real_stderr, 0, _IOLBF, 0))
+    {
+      perror ("setvbuf(real_stderr)");
+      return 2;
+    }
+
+  if (pipe (bufpipe))
+    {
+      perror ("pipe");
+      return 2;
+    }
+
+  /* Below this point, nobody other than the test_threadproc should use
+     the normal stderr.  */
+  if (dup2 (bufpipe[1], 2) == -1)
+    {
+      fprintf (real_stderr, "dup2: %s\n", strerror (errno));
+      return 2;
+    }
+  close (bufpipe[1]);
+
+  b_args.ifd = bufpipe[0];
+  b_args.real_stderr = real_stderr;
+  err = pthread_create (&b_thr, 0, buffer_threadproc, &b_args);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_create(buffer_thr): %s\n",
+               strerror (err));
+      return 2;
+    }
+
+  err = pthread_barrier_init (&gate, 0, 2);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_barrier_init: %s\n", strerror (err));
+      return 2;
+    }
+
+  err = pthread_create (&t_thr, 0, test_threadproc, &gate);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_create(test_thr): %s\n", strerror (err));
+      return 2;
+    }
+
+  err = pthread_cancel (t_thr);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_cancel: %s\n", strerror (err));
+      return 2;
+    }
+
+  pthread_barrier_wait (&gate); /* cannot fail */
+
+  err = pthread_join (t_thr, &rv);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_join(test_thr): %s\n", strerror (err));
+      return 2;
+    }
+
+  /* Closing the normal stderr releases the buffer_threadproc from its
+     loop.  */
+  fclose (stderr);
+  err = pthread_join (b_thr, &obuf_v);
+  if (err)
+    {
+      fprintf (real_stderr, "pthread_join(buffer_thr): %s\n", strerror (err));
+      return 2;
+    }
+  obuf = obuf_v;
+  if (obuf == 0)
+    return 2; /* error within buffer_threadproc, already reported */
+
+  if (rv != PTHREAD_CANCELED)
+    {
+      fputs ("FAIL: thread was not cancelled\n", real_stderr);
+      result = 1;
+    }
+  /* obuf should have received all of the text printed by malloc_stats,
+     but not the text printed by the final call to fputs.  */
+  if (!strstr (obuf, "max mmap bytes"))
+    {
+      fputs ("FAIL: malloc_stats output incomplete\n", real_stderr);
+      result = 1;
+    }
+  if (strstr (obuf, "this call should trigger cancellation"))
+    {
+      fputs ("FAIL: fputs produced output\n", real_stderr);
+      result = 1;
+    }
+
+  if (result == 1)
+    {
+      fputs ("--- output from thread below ---\n", real_stderr);
+      fputs (obuf, real_stderr);
+    }
+  return result;
+}
-- 
2.15.1