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

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

[Tom de Vries]

On 6/7/21 11:46 PM, Tom de Vries wrote:
> 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?
> 

Ping.

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
>  }
>