From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35480 invoked by alias); 15 Nov 2019 14:40:01 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 35460 invoked by uid 89); 15 Nov 2019 14:40:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.1 spammy=db, sic X-Spam-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (212.238.236.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Nov 2019 14:39:57 +0000 Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 250FD300073F; Fri, 15 Nov 2019 15:39:51 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id D1D36405CC5B; Fri, 15 Nov 2019 15:39:51 +0100 (CET) Message-ID: <5356c862142d3221d91dc3f8767d6659639c61d9.camel@klomp.org> Subject: Re: patch 2/2 debuginfod server etc. From: Mark Wielaard To: "Frank Ch. Eigler" , elfutils-devel@sourceware.org Cc: amerey@redhat.com Date: Fri, 15 Nov 2019 14:40:00 -0000 In-Reply-To: <20191028190726.GE14349@redhat.com> References: <20191028190438.GC14349@redhat.com> <20191028190602.GD14349@redhat.com> <20191028190726.GE14349@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-5.el7) Mime-Version: 1.0 X-Spam-Flag: NO X-IsSubscribed: yes X-SW-Source: 2019-q4/txt/msg00140.txt.bz2 Hi, On Mon, 2019-10-28 at 15:07 -0400, Frank Ch. Eigler wrote: > Subject: [PATCH 2/2] debuginfod 2/2: server side >=20 > Add the server to the debuginfod/ subdirectory. This is a highly > multithreaded c++11 program (still buildable on rhel7's gcc 4.8, > which is only partly c++11 compliant). Includes an initial suite > of tests, man pages, and a sample systemd service. We already talked a bit on irc about the tests. I had a problem with the testcase because at the end of the test (when trapping exit 0) the working directory isn't empty. Seems sqlite leaves some working files around that are cleaned up too late. And I don't like the pretty big testfiles, which aren't self-contained.=20 You have to fetch them from a remote koji server. It would be much better to have a (small) self-contained .spec file that can be used to generate the rpms. Frank is already working on that. > diff --git a/tests/ChangeLog b/tests/ChangeLog > index 97b8dedb03f5..ef5f2f0f1211 100644 > --- a/tests/ChangeLog > +++ b/tests/ChangeLog > @@ -1,3 +1,10 @@ > +2019-10-28 Aaron Merey > + Frank Ch. Eigler > + > + * run-debuginfod-find.sh, debuginfod_build_id_find.c: New test. > + * testfile-debuginfod-*.rpm.bz2: New data files for test. > + * Makefile.am: Run it. > + > 2019-09-02 Mark Wielaard >=20=20 > * run-readelf-s.sh: Add --dyn-syms case. > diff --git a/tests/Makefile.am b/tests/Makefile.am > index ad0855dec441..a1794493fda5 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -1,6 +1,6 @@ > ## Process this file with automake to create Makefile.in > ## > -## Copyright (C) 1996-2018 Red Hat, Inc. > +## Copyright (C) 1996-2019 Red Hat, Inc. > ## This file is part of elfutils. > ## > ## This file is free software; you can redistribute it and/or modify > @@ -190,6 +190,11 @@ check_PROGRAMS +=3D $(asm_TESTS) > TESTS +=3D $(asm_TESTS) run-disasm-bpf.sh > endif >=20=20 > +if DEBUGINFOD > +check_PROGRAMS +=3D debuginfod_build_id_find > +TESTS +=3D run-debuginfod-find.sh > +endif OK > EXTRA_DIST =3D run-arextract.sh run-arsymtest.sh run-ar.sh \ > run-show-die-info.sh run-get-files.sh run-get-lines.sh \ > run-next-files.sh run-next-lines.sh testfile-only-debug-line= .bz2 \ > @@ -440,7 +445,10 @@ EXTRA_DIST =3D run-arextract.sh run-arsymtest.sh run= -ar.sh \ > run-dwelf_elf_e_machine_string.sh \ > run-elfclassify.sh run-elfclassify-self.sh \ > run-disasm-riscv64.sh \ > - testfile-riscv64-dis1.o.bz2 testfile-riscv64-dis1.expect.bz2 > + testfile-riscv64-dis1.o.bz2 testfile-riscv64-dis1.expect.bz2= \ > + testfile-debuginfod-0.rpm.bz2 testfile-debuginfod-1.rpm.bz2 \ > + testfile-debuginfod-2.rpm.bz2 run-debuginfod-find.sh > + OK. Odd indentation though. > if USE_VALGRIND > valgrind_cmd=3D'valgrind -q --leak-check=3Dfull --error-exitcode=3D1' > @@ -474,7 +482,7 @@ TESTS_ENVIRONMENT =3D LC_ALL=3DC; LANG=3DC; VALGRIND_= CMD=3D$(valgrind_cmd); \ > export LC_ALL; export LANG; export VALGRIND_CMD; \ > NM=3D$(NM); export NM; > LOG_COMPILER =3D $(abs_srcdir)/test-wrapper.sh \ > - $(abs_top_builddir)/libdw:$(abs_top_builddir)/backends:$(a= bs_top_builddir)/libelf:$(abs_top_builddir)/libasm > + $(abs_top_builddir)/libdw:$(abs_top_builddir)/backends:$(a= bs_top_builddir)/libelf:$(abs_top_builddir)/libasm:$(abs_top_builddir)/debu= ginfod OK. > installcheck-local: > $(MAKE) $(AM_MAKEFLAGS) \ > @@ -610,6 +618,7 @@ unit_info_LDADD =3D $(libdw) > next_cfi_LDADD =3D $(libelf) $(libdw) > elfcopy_LDADD =3D $(libelf) > addsections_LDADD =3D $(libelf) > +debuginfod_build_id_find_LDADD =3D $(libdw) You also use libelf functions, they'll be pulled in through libdw, but I think it is better to be explicit debuginfod_build_id_find_LDADD =3D $(libelf) $(libdw) > xlate_notes_LDADD =3D $(libelf) > elfrdwrnop_LDADD =3D $(libelf) > dwelf_elf_e_machine_string_LDADD =3D $(libelf) $(libdw) > diff --git a/tests/debuginfod_build_id_find.c b/tests/debuginfod_build_id= _find.c > new file mode 100644 > index 000000000000..8e302c8e2116 > --- /dev/null > +++ b/tests/debuginfod_build_id_find.c > @@ -0,0 +1,60 @@ > +/* Test program for fetching debuginfo with debuginfo-server. > + Copyright (C) 2019 Red Hat, Inc. > + This file is part of elfutils. > + > + This file 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. > + > + elfutils 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 .= */ > + > + > +#ifdef HAVE_CONFIG_H > +# include > +#endif > +#include > +#include ELFUTILS_HEADER(dwfl) > +#include > +#include > +#include > +#include > +#include > + > +static const char *debuginfo_path =3D ""; > +static const Dwfl_Callbacks cb =3D > + { > + NULL, > + dwfl_standard_find_debuginfo, > + NULL, > + (char **)&debuginfo_path, > + }; Is this done so that the standard_find_debuginfo doesn't do any path/file system based lookups? > +int > +main (int argc __attribute__ ((unused)), char **argv) > +{ > + int expect_pass =3D strcmp(argv[3], "0"); > + Dwarf_Addr bias =3D 0; > + Dwfl *dwfl =3D dwfl_begin(&cb); > + dwfl_report_begin(dwfl); > + > + /* Open an executable. */ > + Dwfl_Module *mod =3D dwfl_report_offline(dwfl, argv[2], argv[2], -1); > + > + /* The corresponding debuginfo will not be found in debuginfo_path > + (since it's empty), causing the server to be queried. */ Aha, it is... :) OK. > + Dwarf *res =3D dwfl_module_getdwarf(mod, &bias); > + if (expect_pass) > + assert(res); > + else > + assert(!res); > + > + return 0; > +} > diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh > new file mode 100755 > index 000000000000..2facef2cbaad > --- /dev/null > +++ b/tests/run-debuginfod-find.sh > @@ -0,0 +1,220 @@ > +# Copyright (C) 2019 Red Hat, Inc. > +# This file is part of elfutils. > +# > +# This file 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. > +# > +# elfutils 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 . > + > +set -x Is this really necessary? It makes the log files very verbose. Or is that the intention? > +. $srcdir/test-subr.sh # includes set -e > + > +# These are released Fedora 30 i686 main,-debuginfo,-debugsource rpms fr= om koji > +# https://koji.fedoraproject.org/koji/buildinfo?buildID=3D1355903 > +testfiles testfile-debuginfod-0.rpm testfile-debuginfod-1.rpm testfile-d= ebuginfod-2.rpm As mentioned earlier I would really, really, really like it if we could create them "by hand" through a self-contained .spec file. You can still check in the, hopefully much smaller, binary rpms. But I feel somewhat strongly about providing the sources and instructions on how to recreate them, instead of relying on some remote server. > +DB=3D${PWD}/.debuginfod_tmp.sqlite > +export DEBUGINFOD_CACHE_PATH=3D${PWD}/.client_cache > + > +# clean up trash if we were aborted early > +trap 'set +e; kill $PID1 $PID2; rm -rf F R ${PWD}/.client_cache*; exit_c= leanup; exit 0' 0 1 2 3 5 9 15 So on my machine this causes exit_cleanup to fail because the working directory isn't fully empty yet. Specifically there seem to be .debuginfod_tmp.sqlite-shm .debuginfod_tmp.sqlite-wal files left. You don't have to remove F and R since you already mark them as tempfiles. > +# find an unused port number > +while true; do > + PORT1=3D`expr '(' $RANDOM % 1000 ')' + 9000` > + ss -atn | fgrep ":$PORT1" || break > +done=20=20=20=20 Which package does ss come from? Make sure it is listed as a BuildRequires. Also I am slightly worried about it not finding anything and looping forever. Probably unlikely, but ss might misfunction? What happens if ss isn't installed? If we assume bash (which I believe we already do in some places) you could use the bash builtin special /dev/tcp redirection to test whether a localhost port is open with echo >/dev/tcp/localhost/$PORT > +# We want to run debuginfod in the background. We also want to start > +# it with the same check/installcheck-sensitive LD_LIBRARY_PATH stuff > +# that the testrun alias sets. But: we if we just use > +# testrun .../debuginfod > +# it runs in a subshell, with different pid, so not helpful. > +# > +# So we gather the LD_LIBRARY_PATH with this cunning trick: > +ldpath=3D`testrun sh -c 'echo $LD_LIBRARY_PATH'` > + > +mkdir F R > +tempfiles F R O, they are directories, then yeah, tempfiles isn't enough. Maybe don't mark them as tempfiles then. > +env LD_LIBRARY_PATH=3D$ldpath DEBUGINFOD_URLS=3D ${abs_builddir}/../debu= ginfod/debuginfod -vvvv -d $DB \ > +-p $PORT1 -t0 -g0 R F & > +PID1=3D$! > +sleep 3 Is there no better way to test the server has started? > +export DEBUGINFOD_URLS=3Dhttp://localhost:$PORT1/=C2=A0 # or without tr= ailing / > + > +# We use -t0 and -g0 here to turn off time-based scanning & grooming. > +# For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process. > + > +######################################################################## > + > +# Compile a simple program, strip its debuginfo and save the build-id. > +# Also move the debuginfo into another directory so that elfutils > +# cannot find it without debuginfod. > +echo "int main() { return 0; }" > ${PWD}/prog.c > +tempfiles prog.c > +gcc -g -o prog ${PWD}/prog.c > + ${abs_top_builddir}/src/strip -g -f prog.debug ${PWD}/prog > +BUILDID=3D`env LD_LIBRARY_PATH=3D$ldpath ${abs_builddir}/../src/readelf \ > + -a prog | grep 'Build ID' | cut -d ' ' -f 7` > + > +mv prog F > +mv prog.debug F > +kill -USR1 $PID1 > +sleep 3 # give enough time for scanning pass=20 Hmm that hardcoded sleep 3 again :{ > +######################################################################## > + > +# Test whether elfutils, via the debuginfod client library dlopen hooks, > +# is able to fetch debuginfo from the local debuginfod. > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1 > + > +######################################################################## > + > +# Test whether debuginfod-find is able to fetch those files. > +rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debug= info $BUILDID` > +cmp $filename F/prog.debug > + > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find execu= table $BUILDID` > +cmp $filename F/prog > + > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find sourc= e $BUILDID ${PWD}/prog.c` > +cmp $filename ${PWD}/prog.c OK. Cute. > +######################################################################## > + > +# Add artifacts to the search paths and test whether debuginfod finds th= em while already running. > + > +# Build another, non-stripped binary > +echo "int main() { return 0; }" > ${PWD}/prog2.c > +tempfiles prog2.c > +gcc -g -o prog2 ${PWD}/prog2.c > +BUILDID2=3D`env LD_LIBRARY_PATH=3D$ldpath ${abs_builddir}/../src/readelf= \ > + -a prog2 | grep 'Build ID' | cut -d ' ' -f 7` > + > +mv prog2 F > +kill -USR1 $PID1 > +sleep 3 > + > +# Rerun same tests for the prog2 binary > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debug= info $BUILDID2` > +cmp $filename F/prog2 > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find execu= table $BUILDID2` > +cmp $filename F/prog2 > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find sourc= e $BUILDID2 ${PWD}/prog2.c` > +cmp $filename ${PWD}/prog2.c > + > +mv testfile-debuginfod-0.rpm R > +mv testfile-debuginfod-1.rpm R > +mv testfile-debuginfod-2.rpm R > +kill -USR1 $PID1 > +sleep 10 Why 10? > +kill -USR1 $PID1 # two hits of SIGUSR1 may be needed to resolve .debug-= >dwz->srefs > +sleep 10 And another :{ > +RPM_BUILDID=3D5cae7f84186d4ff6c462c32324a764f7a38c3b80 # = ./usr/bin/eu-readelf > +RPM_SOURCE_PATH=3D/usr/src/debug/elfutils-0.177-1.fc30.i386/src/readelf.c > +RPM_EXECUTABLE_SHA1SUM=3D9e4c79dd91a4646d95dfbf091b133e1a21ab2d4c > +RPM_DEBUGINFO_SHA1SUM=3D6b638fa2abc5ff0d4d6c438d904092d20cc71827 > +RPM_SOURCE_SHA1SUM=3Da5bde2a096f6d8f8221456c9380d3532235d7980 > + > +# Run similar tests against contents of the test RPMs ... except we can'= t (don't want to) > +# compare the returned binary to the one in the RPM(s), so we cheat a bi= t, just use a > +# sha1sum comparison > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find execu= table $RPM_BUILDID` > +hash=3D`cat $filename | sha1sum | awk '{print $1}'` > +test $hash =3D $RPM_EXECUTABLE_SHA1SUM > + > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debug= info $RPM_BUILDID` > +hash=3D`cat $filename | sha1sum | awk '{print $1}'` > +test $hash =3D $RPM_DEBUGINFO_SHA1SUM > + > +filename=3D`testrun ${abs_top_builddir}/debuginfod/debuginfod-find sourc= e $RPM_BUILDID $RPM_SOURCE_PATH` > +hash=3D`cat $filename | sha1sum | awk '{print $1}'` > +test $hash =3D $RPM_SOURCE_SHA1SUM OK, because otherwise you would need rpm2cpio and friends to unpack. > +######################################################################## > + > +# Drop some of the artifacts, run a groom cycle; confirm that > +# debuginfod has forgotten them, but remembers others > + > +rm R/testfile-* > +kill -USR2 $PID1 # groom cycle > +sleep 3 > +rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests > + > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_B= UILDID && false || true > + > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILD= ID2 OK > +######################################################################## > + > +# Federation mode > + > +# find another unused port > +while true; do > + PORT2=3D`expr '(' $RANDOM % 1000 ')' + 9000` > + ss -atn | fgrep ":$PORT2" || break > +done Same comment as above. > +export DEBUGINFOD_CACHE_PATH=3D${PWD}/.client_cache2 > +mkdir -p $DEBUGINFOD_CACHE_PATH > +# NB: inherits the DEBUGINFOD_URLS to the first server > +env LD_LIBRARY_PATH=3D$ldpath ${abs_builddir}/../debuginfod/debuginfod -= d ${DB}_2 -p $PORT2 & > +PID2=3D$! > +sleep 3 > + > +# have clients contact the new server > +export DEBUGINFOD_URLS=3Dhttp://localhost:$PORT2 > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1 > + > +# test parallel queries in client > +export DEBUGINFOD_CACHE_PATH=3D${PWD}/.client_cache3 > +mkdir -p $DEBUGINFOD_CACHE_PATH > +export DEBUGINFOD_URLS=3D"BAD http://localhost:$PORT1 localhost:$PORT1 h= ttp://localhost:$PORT22 DNE" > + > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog2 1 OK. > +######################################################################## > + > +# Run the tests again without the servers running. The target file should > +# be found in the cache. > + > +kill -INT $PID1 $PID2 > +sleep 5 > +tempfiles .debuginfod_* > + > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog2 1 OK. Cute. > +######################################################################## > + > +# Trigger a cache clean and run the tests again. The clients should be u= nable to > +# find the target. > +echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s > +echo 0 > $DEBUGINFOD_CACHE_PATH/max_unused_age_s > + > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1 > + > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDI= D2 && false || true OK. But that means zero means never cache/always clean? I would have expected 0 to mean "forever". > +######################################################################## > + > +# Ensure debuginfod-find can be safely called with no arguments. > +# Use a relative path to prevent automatic line breaks in the output > +# due to excessive characters. > +testrun_compare ../../debuginfod/debuginfod-find < +Usage: ../../debuginfod/debuginfod-find debuginfo BUILDID > + or: ../../debuginfod/debuginfod-find executable BUILDID > + or: ../../debuginfod/debuginfod-find source BUILDID /FILENAME > +EOF > + > +exit 0 OK.