From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 175023857C47 for ; Fri, 10 Dec 2021 12:56:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 175023857C47 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id C90F43000ED0; Fri, 10 Dec 2021 13:56:41 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 47C40400027B; Fri, 10 Dec 2021 13:56:41 +0100 (CET) Message-ID: Subject: Re: [PATCH] Add valgrind smoke test From: Mark Wielaard To: DJ Delorie , Alexandra =?ISO-8859-1?Q?H=E1jkov=E1?= Cc: libc-alpha@sourceware.org, ahajkova@redhat.com Date: Fri, 10 Dec 2021 13:56:41 +0100 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Dec 2021 12:56:47 -0000 Hi DJ, On Tue, 2021-12-07 at 15:32 -0500, DJ Delorie wrote: > > +# Test whether valgrind is available in the test > > +# environment. If not, skip the test. > > +${test_wrapper_env} \ > > +${run_program_env} \ > > +$rtld --library-path "$library_path" \ > > + /bin/sh -c 'command -v valgrind' || exit 77 >=20 > Tested on bash, sh, and ksh - although 1003.2-1992 doesn't list > "command" as a built-in. Not a problem; we don't support systems > that old ;-) >=20 > We run this script via $(SHELL), we should use $(SHELL) here too, not > /bin/sh Can we rely on SHELL being defined here or should we pass that to the test script too? > > +${test_wrapper_env} \ > > +${run_program_env} \ > > +$rtld --library-path "$library_path" \ > > +/bin/sh -c "valgrind -q --error-exitcode=3D1 $test_prog" >=20 > Same $(SHELL) here. >=20 > A bit of inconsistency with $x vs ${x} vs "$x" but that doesn't > affect > functionality here. However, it looks like we're running /bin/sh > with > the just-built glibc, and the test program with the system's glibc? > If you look in $(builddir)/testrun.sh you'll see how we run valgrind > to > test the built glibc. >=20 > I.e. (in short): >=20 > ${...} /bin/sh valgrind foo <- runs /bin/sh with new glibc > /bin/sh ${...} valgrind foo <- runs valgrind with new glibc > /bin/sh valgrind ${...} foo <- runs foo with new glibc >=20 > Also, if it's your *intent* to test the system valgrind against the > just-built glibc[*], that needs a comment, and should be tested in a > cross-compile environment. > [*] or both valgrind and the test binary The intent is to test the valgrind available in the test environment against the just-built glibc (that is both valgrind and the test binary valgrind-test, which are really the same process) So we want option 2 from your list. What exactly is ${...} in this case? > (in general, testing in a cross-compiled case is an excellent way to > see > if you got the rtld stuff in the right place ;-) I never did a cross-compiled case. Is there a description of that process? > > diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c > > +int > > +main (void) > > +{ > > + setlocale (LC_ALL, ""); > > + bindtextdomain ("translit", ""); > > + textdomain ("translit"); > > + > > + return 0; > > +} >=20 > Any reason why these particular calls were chosen, rather than (for > example) a malloc/free pair? Perhaps a bit of commentary for future > readers about what valgrind is expected to be testing here... See also my other comment in=20 https://sourceware.org/pipermail/libc-alpha/2021-December/133714.html This tests not just a simple malloc/free pair, but also various string/path comparisons, dlopen, etc. which in the past have seen issues with valgrind. Thanks, Mark