From patchwork Wed Aug 20 18:25:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2455 Received: (qmail 26342 invoked by alias); 20 Aug 2014 18:26:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26332 invoked by uid 89); 20 Aug 2014 18:26:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, URIBL_BLACK autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 20 Aug 2014 18:25:57 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7KIPsE0015557 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 20 Aug 2014 14:25:55 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7KIPq8h023480; Wed, 20 Aug 2014 14:25:53 -0400 Message-ID: <53F4E830.8010309@redhat.com> Date: Wed, 20 Aug 2014 19:25:52 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Integrate PR 12649's race detector directly in the testsuite machinery References: <1406314028-12796-1-git-send-email-palves@redhat.com> <20140815204804.GA3029@host2.jankratochvil.net> In-Reply-To: <20140815204804.GA3029@host2.jankratochvil.net> On 08/15/2014 09:48 PM, Jan Kratochvil wrote: > On Fri, 25 Jul 2014 20:47:08 +0200, Pedro Alves wrote: >> --- /dev/null >> +++ b/gdb/testsuite/read1.c > > I would find it more suitable as: gdb/testsuite/lib/read1.c Done. >> +ssize_t >> +read (int fd, void *buf, size_t count) >> +{ >> + static ssize_t (*read2) (int fd, void *buf, size_t count) = NULL; >> + if (read2 == NULL) >> + { >> + unsetenv ("LD_PRELOAD"); >> + read2 = dlsym (RTLD_NEXT, "read"); >> + } >> + if (count > 0 && isatty (fd) >= 1) > > Here could be 'count > 1'. Indeed. Done. > Otherwise the Makefiles work for me. I've pushed this now, as below. Thanks, Pedro Alves -------- Integrate PR 12649's race detector directly in the testsuite machinery This integrates Jan Kratochvil's nice race reproducer from PR testsuite/12649 into the testsuite infrustructure directly. With this, one only has to do either 'make check-read1' or 'make check READ1="1"' to preload the read1.so library into expect. Currently only enabled for glibc/GNU systems, and if build==host==target. gdb/testsuite/ChangeLog: * Makefile.in (EXTRA_RULES, CC): New variables, get from configure. (EXPECT): Handle READ1 being set. (all): Depend on EXTRA_RULES. (check-read1, expect-read1, read1.so, read1): New rules. * README (Testsuite Parameters): Document the READ1 make variable. (Race detection): New section. * configure: Regenerate. * configure.ac: If build==host==target, and running under a GNU/glibc system, add read1 to the extra Makefile rules. (EXTRA_RULES): AC_SUBST it. * lib/read1.c: New file. gdb/ChangeLog: * Makefile.in (check-read1): New rule. --- gdb/ChangeLog | 4 ++++ gdb/testsuite/ChangeLog | 16 ++++++++++++++++ gdb/Makefile.in | 8 ++++++++ gdb/testsuite/Makefile.in | 44 ++++++++++++++++++++++++++++++++++++++++---- gdb/testsuite/README | 41 +++++++++++++++++++++++++++++++++++++++++ gdb/testsuite/configure | 10 ++++++++++ gdb/testsuite/configure.ac | 9 +++++++++ gdb/testsuite/lib/read1.c | 40 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/lib/read1.c diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8c6b204..8bae7d3 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2014-08-20 Pedro Alves + + * Makefile.in (check-read1): New rule. + 2014-08-20 Joel Brobecker * value.c (value_from_contents_and_address): Strip resolved_type's diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index ef024b9..a9205d1 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,19 @@ +2014-08-20 Pedro Alves + Jan Kratochvil + + * Makefile.in (EXTRA_RULES, CC): New variables, get from + configure. + (EXPECT): Handle READ1 being set. + (all): Depend on EXTRA_RULES. + (check-read1, expect-read1, read1.so, read1): New rules. + * README (Testsuite Parameters): Document the READ1 make variable. + (Race detection): New section. + * configure: Regenerate. + * configure.ac: If build==host==target, and running under a + GNU/glibc system, add read1 to the extra Makefile rules. + (EXTRA_RULES): AC_SUBST it. + * lib/read1.c: New file. + 2014-08-20 Joel Brobecker * gdb.dwarf2/data-loc.exp: Add additional tests exercising diff --git a/gdb/Makefile.in b/gdb/Makefile.in index b33defe..f251627 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1098,6 +1098,14 @@ check-perf: force $(MAKE) $(TARGET_FLAGS_TO_PASS) check-perf; \ else true; fi +check-read1: 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; \ + else true; fi + # The idea is to parallelize testing of multilibs, for example: # make -j3 check//sh-hms-sim/{-m1,-m2,-m3,-m3e,-m4}/{,-nofpu} # will run 3 concurrent sessions of check, eventually testing all 10 diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in index 0020a0f..1c923cd 100644 --- a/gdb/testsuite/Makefile.in +++ b/gdb/testsuite/Makefile.in @@ -39,9 +39,17 @@ ALL_SUBDIRS = gdb.ada gdb.arch gdb.asm gdb.base gdb.btrace gdb.cell gdb.cp \ gdb.stabs gdb.reverse gdb.threads gdb.trace gdb.xml \ $(SUBDIRS) -EXPECT = `if [ -f $${rootme}/../../expect/expect ] ; then \ - echo $${rootme}/../../expect/expect ; \ - else echo expect ; fi` +EXTRA_RULES = @EXTRA_RULES@ + +CC=@CC@ + +EXPECT = `if [ "$${READ1}" != "" ] ; then \ + echo $${rootme}/expect-read1; \ + elif [ -f $${rootme}/../../expect/expect ] ; then \ + echo $${rootme}/../../expect/expect ; \ + else \ + echo expect ; \ + fi` RUNTEST = $(RUNTEST_FOR_TARGET) @@ -84,7 +92,7 @@ TARGET_FLAGS_TO_PASS = \ "RUNTEST=$(RUNTEST)" \ "RUNTESTFLAGS=$(RUNTESTFLAGS)" -all: +all: $(EXTRA_RULES) @echo "Nothing to be done for all..." .NOEXPORT: @@ -146,6 +154,9 @@ installcheck: check: all $(abs_builddir)/site.exp $(MAKE) $(CHECK_TARGET) +check-read1: + $(MAKE) READ1="1" check + # All the hair to invoke dejagnu. A given invocation can just append # $(RUNTESTFLAGS) DO_RUNTEST = \ @@ -230,6 +241,7 @@ clean mostlyclean: -rm -f core.* *.tf *.cl *.py tracecommandsscript copy1.txt zzz-gdbscript -rm -f *.dwo *.dwp -rm -rf outputs temp cache + -rm -f read1.so expect-read1 if [ x"${ALL_SUBDIRS}" != x ] ; then \ for dir in ${ALL_SUBDIRS}; \ do \ @@ -263,3 +275,27 @@ config.status: configure TAGS: force find $(srcdir) -name '*.exp' -print | \ etags --regex='/proc[ \t]+\([^ \t]+\)/\1/' - + +# Build the expect wrapper script that preloads the read1.so library. +expect-read1: + @echo Making expect-read1 + @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 + +# Build the read1.so preload library. This overrides the `read' +# function, making it read one byte at a time. Running the testsuite +# with this catches racy tests. +read1.so: lib/read1.c + $(CC) -o $@ ${srcdir}/lib/read1.c -Wall -g -shared -fPIC $(CFLAGS) + +# Build the read1 machinery. +.PHONY: read1 +read1: read1.so expect-read1 diff --git a/gdb/testsuite/README b/gdb/testsuite/README index 466993d..9a5059a 100644 --- a/gdb/testsuite/README +++ b/gdb/testsuite/README @@ -176,6 +176,47 @@ Example: If not using GNU make then the value is passed directly to runtest. If not specified, all tests are run. +READ1 + +This make (not runtest) variable is used to specify whether the +testsuite preloads the read1.so library into expect. Any non-empty +value means true. See "Race detection" below. + +Race detection +************** + +The testsuite includes a mechanism that helps detect test races. + +For example, say the program running under expect outputs "abcd", and +a test does something like this: + + expect { + "a.*c" { + } + "b" { + } + "a" { + } + } + +Which case happens to match depends on what expect manages to read +into its internal buffer in one go. If it manages to read three bytes +or more, then the first case matches. If it manages to read two +bytes, then the second case matches. If it manages to read only one +byte, then the third case matches. + +To help detect these cases, the race detection mechanism preloads a +library into expect that forces the `read' system call to always +return at most 1 byte. + +To enable this, either pass a non-empty value in the READ1 make +variable, or use the check-read1 make target instead of check. + +Examples: + + make -j10 check-read1 TESTS="*/paginate-*.exp" + make -j10 check READ1="1" + Testsuite Configuration *********************** diff --git a/gdb/testsuite/configure b/gdb/testsuite/configure index 4bf6121..15944ed 100755 --- a/gdb/testsuite/configure +++ b/gdb/testsuite/configure @@ -591,6 +591,7 @@ ac_includes_default="\ ac_subst_vars='LTLIBOBJS LIBOBJS +EXTRA_RULES EGREP GREP CPP @@ -3448,6 +3449,15 @@ done +if test "${build}" = "${host}" -a "${host}" = "${target}"; then + case "${host}" in + *gnu*) + EXTRA_RULES=read1 + ;; + esac +fi + + ac_config_files="$ac_config_files Makefile gdb.ada/Makefile gdb.arch/Makefile gdb.asm/Makefile gdb.base/Makefile gdb.btrace/Makefile gdb.cell/Makefile gdb.cp/Makefile gdb.disasm/Makefile gdb.dwarf2/Makefile gdb.dlang/Makefile gdb.fortran/Makefile gdb.gdb/Makefile gdb.go/Makefile gdb.server/Makefile gdb.java/Makefile gdb.hp/Makefile gdb.hp/gdb.objdbg/Makefile gdb.hp/gdb.base-hp/Makefile gdb.hp/gdb.aCC/Makefile gdb.hp/gdb.compat/Makefile gdb.hp/gdb.defects/Makefile gdb.guile/Makefile gdb.linespec/Makefile gdb.mi/Makefile gdb.modula2/Makefile gdb.multi/Makefile gdb.objc/Makefile gdb.opencl/Makefile gdb.opt/Makefile gdb.pascal/Makefile gdb.perf/Makefile gdb.python/Makefile gdb.reverse/Makefile gdb.stabs/Makefile gdb.threads/Makefile gdb.trace/Makefile gdb.xml/Makefile" cat >confcache <<\_ACEOF diff --git a/gdb/testsuite/configure.ac b/gdb/testsuite/configure.ac index fb084d4..9dccc9f 100644 --- a/gdb/testsuite/configure.ac +++ b/gdb/testsuite/configure.ac @@ -87,6 +87,15 @@ AC_CHECK_HEADERS(pthread.h) AC_EXEEXT +if test "${build}" = "${host}" -a "${host}" = "${target}"; then + case "${host}" in + *gnu*) + EXTRA_RULES=read1 + ;; + esac +fi +AC_SUBST(EXTRA_RULES) + AC_OUTPUT([Makefile \ gdb.ada/Makefile \ gdb.arch/Makefile gdb.asm/Makefile gdb.base/Makefile gdb.btrace/Makefile \ diff --git a/gdb/testsuite/lib/read1.c b/gdb/testsuite/lib/read1.c new file mode 100644 index 0000000..5873ba2 --- /dev/null +++ b/gdb/testsuite/lib/read1.c @@ -0,0 +1,40 @@ +/* This is part of GDB, the GNU debugger. + + Copyright 2011-2014 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#define _GNU_SOURCE 1 +#include +#include +#include +#include + +/* Wrap 'read', forcing it to return only one byte at a time, if + reading from the terminal. */ + +ssize_t +read (int fd, void *buf, size_t count) +{ + static ssize_t (*read2) (int fd, void *buf, size_t count) = NULL; + if (read2 == NULL) + { + unsetenv ("LD_PRELOAD"); + read2 = dlsym (RTLD_NEXT, "read"); + } + if (count > 1 && isatty (fd) >= 1) + count = 1; + return read2 (fd, buf, count); +}