From fa9ce2c143ce7ee6bc4f22a0577fe5c0858beddd Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Sat, 9 Oct 2021 18:53:12 +0200 Subject: [PATCH] [gdb/testsuite] Add check-readmore 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. There are 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, and so on. The second method is potentially faster, has less risc of timeout and could potentially detect more problems. The first method has a simpler implementation. The second method is enabled by default. The default waiting period is 10 miliseconds. The first method can be enabled using: ... $ export READMORE_METHOD=1 ... and the waiting period can be specified in miliseconds using: ... $ export READMORE_SLEEP=9 ... Also a log file can be specified using: ... $ export READMORE_LOG=$(pwd -P)/LOG ... Tested on x86_64-linux. Testing with check-readmore showed these regressions: ... FAIL: gdb.base/bp-cmds-continue-ctrl-c.exp: run: stop with control-c (continue) FAIL: gdb.base/bp-cmds-continue-ctrl-c.exp: attach: stop with control-c (continue) ... I have not been able to find a problem in the test-case, and I think it's the nature of both the test-case and readmore that makes it run longer. Make these pass by increasing the alarm timeout from 60 to 120 seconds. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27957 --- gdb/Makefile.in | 4 +- gdb/testsuite/Makefile.in | 43 ++++-- gdb/testsuite/README | 2 +- .../gdb.base/bp-cmds-continue-ctrl-c.c | 2 +- gdb/testsuite/lib/read1.c | 136 +++++++++++++++++- 5 files changed, 166 insertions(+), 21 deletions(-) diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 5a3bb952279..4201f65e68d 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1681,12 +1681,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 20cb3dae2ed..6b4c7588cfa 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 @@ -367,18 +372,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 @@ -386,9 +395,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 862f423a988..7552774c78b 100644 --- a/gdb/testsuite/README +++ b/gdb/testsuite/README @@ -361,7 +361,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/gdb.base/bp-cmds-continue-ctrl-c.c b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.c index 968c879dd09..fb76cfb3973 100644 --- a/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.c +++ b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.c @@ -26,7 +26,7 @@ foo (void) int main () { - alarm (60); + alarm (120); while (1) foo (); diff --git a/gdb/testsuite/lib/read1.c b/gdb/testsuite/lib/read1.c index 7dabdd4ca0c..4e8aac6566f 100644 --- a/gdb/testsuite/lib/read1.c +++ b/gdb/testsuite/lib/read1.c @@ -21,14 +21,62 @@ #include #include #include +#include +#include +#include -/* Wrap 'read', forcing it to return only one byte at a time, if - reading from the terminal. */ +/* Default READMORE method. */ +#define READMORE_METHOD_DEFAULT 2 + +/* Default READMORE sleep time in miliseconds. */ +#define READMORE_SLEEP_DEFAULT 10 + +/* Helper function. Intialize *METHOD according to environment variable + READMORE_METHOD, and *SLEEP according to environment variable + READMORE_SLEEP. */ + +static void +init_readmore (int *method, unsigned int *sleep, FILE **log) +{ + char *env = getenv ("READMORE_METHOD"); + if (env == NULL) + *method = READMORE_METHOD_DEFAULT; + else if (strcmp (env, "1") == 0) + *method = 1; + else if (strcmp (env, "2") == 0) + *method = 2; + else + /* Default. */ + *method = READMORE_METHOD_DEFAULT; + + env = getenv ("READMORE_SLEEP"); + if (env == NULL) + *sleep = READMORE_SLEEP_DEFAULT; + else + *sleep = atoi (env); + + env = getenv ("READMORE_LOG"); + if (env == NULL) + *log = NULL; + else + *log = fopen (env, "w"); +} + +/* Wrap 'read', and modify it's behaviour using READ1 or READMORE style. */ ssize_t read (int fd, void *buf, size_t count) { static ssize_t (*read2) (int fd, void *buf, size_t count) = NULL; + static FILE *log; + int readmore; +#ifdef READMORE + readmore = 1; +#else + readmore = 0; +#endif + static int readmore_method; + static unsigned int readmore_sleep; if (read2 == NULL) { /* Use setenv (v, "", 1) rather than unsetenv (v) to work around @@ -37,8 +85,88 @@ read (int fd, void *buf, size_t count) for existence from another interp". */ setenv ("LD_PRELOAD", "", 1); read2 = dlsym (RTLD_NEXT, "read"); + if (readmore) + init_readmore (&readmore_method, &readmore_sleep, &log); } - if (count > 1 && isatty (fd) >= 1) - count = 1; + + /* Only modify 'read' behaviour when reading from the terminal. */ + if (isatty (fd) == 0) + goto fallback; + + if (!readmore) + { + /* READ1. Force read to return only one byte at a time. */ + return read2 (fd, buf, 1); + } + + if (readmore_method == 1) + { + /* READMORE, method 1. Wait a little before doing a read. */ + usleep (readmore_sleep * 1000); + return read2 (fd, buf, count); + } + + if (readmore_method == 2) + { + /* READMORE, method 2. After doing a read, either return or wait + a little and do another read, and so on. */ + ssize_t res, total; + int iteration; + int max_iterations = -1; + + total = 0; + for (iteration = 1; ; iteration++) + { + res = read2 (fd, (char *)buf + total, count - total); + if (log != NULL) + fprintf (log, + "READ (%d): fd: %d, COUNT: %zd, RES: %zd, ERRNO: %s\n", + iteration, fd, count - total, res, + res == -1 ? strerror (errno) : "none"); + if (res == -1) + { + if (iteration == 1) + { + /* Error on first read, report. */ + total = -1; + break; + } + + if (total > 0 + && (errno == EAGAIN || errno == EWOULDBLOCK || errno == EIO)) + { + /* Ignore error, but don't try anymore reading. */ + errno = 0; + break; + } + + /* Other error, report back. */ + total = -1; + break; + } + + total += res; + if (total == count) + /* Buf full, no need to do any more reading. */ + break; + + /* Handle end-of-file. */ + if (res == 0) + break; + + if (iteration == max_iterations) + break; + + usleep (readmore_sleep * 1000); + } + + if (log) + fprintf (log, "READ returning: RES: %zd, ERRNO: %s\n", + total, total == -1 ? strerror (errno) : "none"); + return total; + } + + fallback: + /* Fallback, regular read. */ return read2 (fd, buf, count); } -- 2.39.2