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 9EF4E3858C39 for ; Tue, 7 Dec 2021 11:56:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9EF4E3858C39 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 07417301BC1C; Tue, 7 Dec 2021 12:56:23 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B5F48413CC9A; Tue, 7 Dec 2021 12:56:22 +0100 (CET) Message-ID: <42d41bb656ca0183bc2340afc3edf6ff6ad79b75.camel@klomp.org> Subject: Re: [PATCH] Add valgrind smoke test From: Mark Wielaard To: Alexandra =?ISO-8859-1?Q?H=E1jkov=E1?= , libc-alpha@sourceware.org Cc: Alexandra =?ISO-8859-1?Q?H=E1jkov=E1?= Date: Tue, 07 Dec 2021 12:56:22 +0100 In-Reply-To: <20211206144043.858697-1-ahajkova@redhat.com> References: <20211206144043.858697-1-ahajkova@redhat.com> 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.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, 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: Tue, 07 Dec 2021 11:56:27 -0000 Hi, On Mon, 2021-12-06 at 15:40 +0100, Alexandra H=C3=A1jkov=C3=A1 wrote: > From: Alexandra H=C3=A1jkov=C3=A1 >=20 > Check if valgrind is present during the configure time and > run smoke tests with valgrind to verify dynamic loader. >=20 > Co-authored-by: Mark Wielaard Let me add some comments on the different parts to help a real review. > --- > elf/Makefile | 7 +++++++ > elf/tst-valgrind-smoke.sh | 38 ++++++++++++++++++++++++++++++++++++++ > elf/valgrind-test.c | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 76 insertions(+) > create mode 100644 elf/tst-valgrind-smoke.sh > create mode 100644 elf/valgrind-test.c >=20 > diff --git a/elf/Makefile b/elf/Makefile > index ef36008673..14aab3624a 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -232,6 +232,7 @@ tests +=3D restest1 preloadtest loadfail multiload or= igtest resolvfail \ > tst-dl-is_dso tst-ro-dynamic \ > tst-audit18 \ > tst-rtld-run-static \ > + valgrind-test > # reldep9 > tests-internal +=3D loadtest unload unload2 circleload1 \ > neededtest neededtest2 neededtest3 neededtest4 \ Adding valgrind-test to tests simply makes sure it gets build for use with tst-valgrind-smoke.sh. > @@ -253,6 +254,12 @@ tests-special +=3D $(objpfx)tst-audit14-cmp.out $(ob= jpfx)tst-audit15-cmp.out \ > endif > endif > endif > + > +tests-special +=3D $(objpfx)tst-valgrind-smoke.out > +$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so > + $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' '$(run-program-env)' '= $(rpath-link)' $(objpfx)valgrind-test > $@; \ > + $(evaluate-test) > + > tests +=3D $(tests-execstack-$(have-z-execstack)) > ifeq ($(run-built-tests),yes) > tests-special +=3D $(objpfx)tst-leaks1-mem.out \ We want a tst-valgrind-smoke.out file, which will show the test run. To get tst-valgrind-smoke.out we need to have tst-valgrind-smoke.sh (which already exists, but we'll also use it as $< in the make rule) and ld.so build first. Question, do we also want to depend on $(objpfx)valgrind-tst to make sure that it exists when this rule is executed, or is having it in tests enough to express that dependency? The rule simply passes the just build linker, test and run-program environments and the rpath of the just build libraries, plus the actual program to run under valgrind to the tst-valgrind-smoke.sh script. The exit code of which will then be processed by evaluate-test. > diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh > new file mode 100644 > index 0000000000..a78d7ff10d > --- /dev/null > +++ b/elf/tst-valgrind-smoke.sh > @@ -0,0 +1,38 @@ > +#!/bin/sh > +# Valgrind smoke test. > +# Copyright (C) 2021 Free Software Foundation, Inc. > +# This file is part of the GNU C Library. > + > +# The GNU C Library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > + > +# The GNU C Library 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 > +# Lesser General Public License for more details. > + > +# You should have received a copy of the GNU Lesser General Public > +# License along with the GNU C Library; if not, see > +# . > + > +set -e > + > +rtld=3D$1 > +test_wrapper_env=3D$2 > +run_program_env=3D$3 > +library_path=3D$4 > +test_prog=3D$5 > + > +# 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 First we make sure that valgrind is actually available in the test environment, if not, like in a cross-build, it will immediately exit with 77 which produces a SKIP. > +${test_wrapper_env} \ > +${run_program_env} \ > +$rtld --library-path "$library_path" \ > +/bin/sh -c "valgrind -q --error-exitcode=3D1 $test_prog" Then we execute valgrind on the test program (valgrind-test) inside the test environment using the just build ld.so and libraries. If valgrind detects any error it exits with exit code 1 (producing a FAIL), otherwise it will use the exit code (0) of the test_prog. > diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c > new file mode 100644 > index 0000000000..606c874b68 > --- /dev/null > +++ b/elf/valgrind-test.c > @@ -0,0 +1,31 @@ > +/* This is the simple test intended to be called by > + tst-valgrind-smoke to perform vagrind smoke test. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library 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 > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > + > +int > +main (void) > +{ > + setlocale (LC_ALL, ""); > + bindtextdomain ("translit", ""); > + textdomain ("translit"); > + > + return 0; > +} The program to be run under valgrind in the test environment is fairly simply, but still exercises some constructs (like string and path lookups and dlopen) that have seen issues under valgrind in the past. I hope this valgrind smoke test will be accepted into the glibc testsuite because it would really help us catch issues with the valgrind/glibc interactions early. Cheers, Mark