[RFC][gdb/testsuite] Add check-readmore

[Tom de Vries <tdevries@suse.de>]

Hi,

Consider the gdb output:
...
27        return SYSCALL_CANCEL (nanosleep, requested_time, remaining);^M
(gdb) ^M
Thread 2 "run-attach-whil" stopped.^M
...

When trying to match the gdb prompt using gdb_test which uses '$gdb_prompt $',
it may pass or fail.

This sort of thing needs to be fixed (see commit b0e2f96b56b), but there's
currently no way to reliably find this type of FAILs.

We have check-read1, but that one actually make the test pass reliably.

We need something like the opposite of check-read1: something that makes
expect read a bit slower, or more exhaustively.

Add a new test target check-readmore that implements this.

Atm there are still two methods of implementing this in read1.c:
- the first method waits a bit before doing a read
- the second method does a read and then decides whether to
  return or to wait a bit and do another read.

Atm the first method is enabled by default, given that it is more foolproof.
The second method tries to be smart about waiting less than the first method,
but consequently needs to make decisions about error codes, which is more
fragile.

Tested on x86_64-linux, both with method 1 and 2.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Add check-readmore

gdb/ChangeLog:

2021-06-07  Tom de Vries  <tdevries@suse.de>

	PR testsuite/27957
	* Makefile.in (check-readmore): New target.

gdb/testsuite/ChangeLog:

2021-06-07  Tom de Vries  <tdevries@suse.de>

	PR testsuite/27957
	* Makefile.in (EXPECT): Update.
	(check-readmore, expect-readmore, readmore.so): New target.
	(readmore): New phony target.
	(clean mostlyclean): Update.
	* lib/read1.c (read): Add READMORE case.
	* README: Mention check-readmore.

---
 gdb/Makefile.in           |  4 ++--
 gdb/testsuite/Makefile.in | 43 ++++++++++++++++++++++++++++++-------------
 gdb/testsuite/README      |  2 +-
 gdb/testsuite/lib/read1.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index f664d964536..ea33146f9ee 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1672,12 +1672,12 @@ check-perf: force
 	  $(MAKE) $(TARGET_FLAGS_TO_PASS) check-perf; \
 	else true; fi
 
-check-read1: force
+check-read1 check-readmore: force
 	@if [ -f testsuite/Makefile ]; then \
 	  rootme=`pwd`; export rootme; \
 	  rootsrc=`cd $(srcdir); pwd`; export rootsrc; \
 	  cd testsuite; \
-	  $(MAKE) $(TARGET_FLAGS_TO_PASS) check-read1; \
+	  $(MAKE) $(TARGET_FLAGS_TO_PASS) $@; \
 	else true; fi
 
 check-parallel: force
diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 2c0482cef1b..1762d631c70 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -39,6 +39,8 @@ CC=@CC@
 
 EXPECT = `if [ "$${READ1}" != "" ] ; then \
             echo $${rootme}/expect-read1; \
+          elif [ "$${READMORE}" != "" ] ; then \
+            echo $${rootme}/expect-readmore; \
           elif [ -f $${rootme}/../../expect/expect ] ; then \
             echo $${rootme}/../../expect/expect ; \
           else \
@@ -161,6 +163,9 @@ check: all $(abs_builddir)/site.exp
 check-read1: read1.so expect-read1
 	$(MAKE) READ1="1" check
 
+check-readmore: readmore.so expect-readmore
+	$(MAKE) READMORE="1" check
+
 # Check whether we need to print the timestamp for each line of
 # status.
 TIMESTAMP = $(if $(TS),| $(srcdir)/print-ts.py $(if $(TS_FORMAT),$(TS_FORMAT),),)
@@ -344,7 +349,7 @@ clean mostlyclean:
 	-rm -f *.dwo *.dwp
 	-rm -rf outputs temp cache
 	-rm -rf gdb.perf/workers gdb.perf/outputs gdb.perf/temp gdb.perf/cache
-	-rm -f read1.so expect-read1
+	-rm -f read1.so expect-read1 readmore.so expect-readmore
 
 distclean maintainer-clean realclean: clean
 	-rm -f *~ core
@@ -364,18 +369,22 @@ TAGS: force
 		-
 
 # Build the expect wrapper script that preloads the read1.so library.
-expect-read1:
+expect-read1 expect-readmore:
 	$(ECHO_GEN) \
-	rm -f expect-read1-tmp; \
-	touch expect-read1-tmp; \
-	echo "# THIS FILE IS GENERATED -*- buffer-read-only: t -*- \n" >>expect-read1-tmp; \
-	echo "# vi:set ro: */\n\n" >>expect-read1-tmp; \
-	echo "# To regenerate this file, run:\n" >>expect-read1-tmp; \
-	echo "#      make clean; make/\n" >>expect-read1-tmp; \
-	echo "export LD_PRELOAD=`pwd`/read1.so" >>expect-read1-tmp; \
-	echo 'exec expect "$$@"' >>expect-read1-tmp; \
-	chmod +x expect-read1-tmp; \
-	mv expect-read1-tmp expect-read1
+	rm -f $@-tmp; \
+	touch $@-tmp; \
+	echo "# THIS FILE IS GENERATED -*- buffer-read-only: t -*- \n" >>$@-tmp; \
+	echo "# vi:set ro: */\n\n" >>$@-tmp; \
+	echo "# To regenerate this file, run:\n" >>$@-tmp; \
+	echo "#      make clean; make/\n" >>$@-tmp; \
+	if [ $@ = expect-read1 ]; then \
+	  echo "export LD_PRELOAD=`pwd`/read1.so" >>$@-tmp; \
+	else \
+	  echo "export LD_PRELOAD=`pwd`/readmore.so" >>$@-tmp; \
+	fi; \
+	echo 'exec expect "$$@"' >>$@-tmp; \
+	chmod +x $@-tmp; \
+	mv $@-tmp $@
 
 # Build the read1.so preload library.  This overrides the `read'
 # function, making it read one byte at a time.  Running the testsuite
@@ -383,9 +392,17 @@ expect-read1:
 read1.so: lib/read1.c
 	$(ECHO_CC) $(CC) -o $@ ${srcdir}/lib/read1.c -Wall -g -shared -fPIC $(CFLAGS)
 
+# Build the readmore.so preload library.  This overrides the `read'
+# function, making it try harder to read more at a time.  Running the
+# testsuite with this catches racy tests.
+readmore.so: lib/read1.c
+	$(ECHO_CC) $(CC) -o $@ ${srcdir}/lib/read1.c -Wall -g -shared -fPIC \
+	  $(CFLAGS) -DREADMORE
+
 # Build the read1 machinery.
-.PHONY: read1
+.PHONY: read1 readmore
 read1: read1.so expect-read1
+readmore: readmore.so expect-readmore
 
 # Disable implicit make rules.
 include $(srcdir)/../disable-implicit-rules.mk
diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 0036753eff0..46d39818460 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -357,7 +357,7 @@ fail, it can also have the effect of making otherwise failing tests pass.
 This happens f.i. if the test is trying to match a gdb prompt using an end of
 input marker "${gdb_prompt} $" and there is output after the gdb prompt.  This
 may either pass or fail in normal operation, but using check-read1 will ensure
-that it passes.
+that it passes.  Use check-readmore to detect this type of failure.
 
 Testsuite Configuration
 ***********************
diff --git a/gdb/testsuite/lib/read1.c b/gdb/testsuite/lib/read1.c
index 7dabdd4ca0c..1f3cd4393a1 100644
--- a/gdb/testsuite/lib/read1.c
+++ b/gdb/testsuite/lib/read1.c
@@ -21,6 +21,9 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <stdio.h>
 
 /* Wrap 'read', forcing it to return only one byte at a time, if
    reading from the terminal.  */
@@ -38,7 +41,50 @@ read (int fd, void *buf, size_t count)
       setenv ("LD_PRELOAD", "", 1);
       read2 = dlsym (RTLD_NEXT, "read");
     }
+
+#ifdef READMORE
+  /* READMORE.  */
+
+#if 1
+  /* READMORE, method 1.  */
+
+  if (isatty (fd) != 0)
+    usleep (10 * 1000);
+  return read2 (fd, buf, count);
+
+#else
+  /* READMORE, method 2.  */
+  ssize_t res, res2;
+
+  res = read2 (fd, buf, count);
+  if (isatty (fd) == 0)
+    return res;
+
+  if (res == count || res == -1)
+    return res;
+
+  usleep (10 * 1000);
+  res2 = read2 (fd, (char *)buf + res, count - res);
+  if (res2 == -1)
+    {
+      if (errno == EAGAIN || errno == EIO)
+	res2 = 0;
+      else
+	{
+	  if (0)
+	    printf ("errno: %s\n", strerror (errno));
+	  return res2;
+	}
+    }
+
+  return res + res2;
+#endif
+
+#else
+  /* READ1 */
+
   if (count > 1 && isatty (fd) >= 1)
     count = 1;
   return read2 (fd, buf, count);
+#endif
 }