Re: [Bug debuginfod/28577] Make run-...-fd-prefetch-caches.sh test something
Hi Noah,
Hope you don't mind me adding elfutils-devel back to the CC.
On Fri, 2022-05-27 at 15:32 -0400, Noah Sanci wrote:
> On Fri, May 27, 2022 at 12:29 PM Mark Wielaard
> wrote:
>
> > On Wed, 2022-05-25 at 14:44 -0400, Noah Sanci via Elfutils-devel
> > wrote:
> > > PR28577: Make run-debuginfod-fd-prefetch-caches.sh test
> > > something
> > >
> > > Update to the run-debuginfod-fd-prefetch to make the test more
> > > sound.
> >
> > Thanks. And according to Evgeny this makes the test pass for him:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=29180#c2
>
> Good to hear.
> > But I find it a little hard to review, it would have been helpful
> > to
> > get a bit more comments/documentation what exactly is being tested
> > now.
>
> Fair, I should have made what was being tested/why clearer.
Thanks. I also understand now that I read the test logic the wrong way
around.
> > > diff --git a/tests/run-debuginfod-fd-prefetch-caches.sh
> > > b/tests/run-
> > > debuginfod-fd-prefetch-caches.sh
> > > index 66a7a083..50ffc4ac 100755
> > > --- a/tests/run-debuginfod-fd-prefetch-caches.sh
> > > +++ b/tests/run-debuginfod-fd-prefetch-caches.sh
> > > @@ -21,22 +21,22 @@
> > > # for test case debugging, uncomment:
> > > set -x
> > > unset VALGRIND_CMD
> > > +mkdir R Z
> >
> > OK, some boilerplate.
> > > -mkdir R
> > > -cp -rvp ${abs_srcdir}/debuginfod-rpms R
> > > -if [ "$zstd" = "false" ]; then # nuke the zstd fedora 31 ones
> > > -rm -vrf R/debuginfod-rpms/fedora31
> > > -fi
> >
> > This gets moved to below.
>
> The two above changes should look better now
>
> > > DB=${PWD}/.debuginfod_tmp.sqlite
> > > tempfiles $DB
> > > @@ -47,88 +47,68 @@ export
> > > DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
> > > ##
> > > rm -rf $DEBUGINFOD_CACHE_PATH
> > > rm -rf $DB
> > > +
> > > # Set mb values to ensure the caches certainly have enough space
> > > # To store the test files
> > > env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS=
> > > ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -p $PORT1 -d
> > > $DB \
> > > ---fdcache-fds=$FDCACHE_FDS --fdcache-prefetch-
> > > fds=$PREFETCH_FDS --fdcache-mintmp 0 -v -g 0 -t 0 -R R \
> > > ---fdcache-mbs=50 --fdcache-prefetch-mbs=50 \
> > > ---fdcache-prefetch=$PREFETCH > vlog$PORT1 2>&1 &
> > > +--fdcache-fds=$FDCACHE_FDS --fdcache-prefetch-
> > > fds=$PREFETCH_FDS -v -g 0 -t 0 -R R \
> > > +-Z .tar.bz2=bzcat Z --fdcache-mbs=50 --fdcache-prefetch-
> > > mbs=100 \
> > > +--fdcache-prefetch=$PREFETCH -F > vlog$PORT1 2>&1 &
> >
> > Now run with --fdcache-fds=1 --fdcache-mbs=50 and
> > -fdcache-prefetch-fds=1 fdcache-prefetch=1 -fdcache-prefetch-
> > mbs=100
> >
> > It is these last 3 we want to test.
>
> This particular instance of debuginfod was intended to test
> fdcache-fds and prefetch-fds.
> The test is just testing prefetch-fds. fdcache-fds was tested in a
> different diff, but because the
> fdcache is just the basic lru the test was removed. Mb values are set
> high enough so that no
> files will be excluded for being too large. Maybe there should be a
> test prior to this for just
> fdcache-prefetch as it determines how much is prefetched.
OK. I think no extra test is needed.
> > > PID1=$!
> > > tempfiles vlog$PORT1
> > > errfiles vlog$PORT1
> > > +
> > > # Server must become ready
> > > wait_ready $PORT1 'ready' 1
> > >
> > >
> > > -cp -rvp ${abs_srcdir}/debuginfod-rpms R
> > > -if [ "$zstd" = "false" ]; then # nuke the zstd fedora 31 ones
> > > -rm -vrf R/debuginfod-rpms/fedora31
> > > -fi
> > > kill -USR1 $PID1
> > > wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
> > > +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> > > +wait_ready $PORT1 'thread_busy{role="scan"}' 0
> >
> > OK, boilerplate to wait for initial scan.
>
> Yes, Frank told me that just checking traverse is insufficient to
> conclude a
> scan has completed.
> > > -archive_hits=5
> > > -SHA=f4a1a8062be998ae93b8f1cd744a398c6de6dbb1
> > > -for i in $archive_hits
> > > -do
> > > - archive_test c36708a78618d597dee15d0dc989f093ca5f9120
> > > /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
> > > -done
> > > +export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1/
> > > +# load prefetch cache
> > > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo
> > > cee13b2ea505a7f37bd20d271c6bc7e5f8d2dfcb
> >
> > OK, which particular file is this?
>
> pk.pkg.tar/usr/src/debug/hello.c, now elaborated in comment
Thanks.
> > > metrics=$(curl http://127.0.0.1:$PORT1/metrics)
> > > -regex="fdcache_op_count\{op=\"enqueue\"\} ([0-
> > > 9]+).*fdcache_op_count\{op=\"evict\"\} ([0-9]+).*"
> > > -enqueue=0
> > > -if [[ $metrics =~ $regex ]]
> > > -then
> > > - enqueue=${BASH_REMATCH[1]}
> > > - evict=${BASH_REMATCH[2]}
> > > +regex="fdcache_prefetch_count ([0-9])+"
> > > +# Check to see if prefetch cache is loaded
> > > +if [[ $metrics =~
[PATCH] strip: keep .ctf section in stripped file
Hello elfutils team,
This patch is meant to avoid remove the CTF section in
stripped files. Please let me know your thoughts.
Kind regard,
Guillermo
CTF debug format was designed to be present in stripped files, so
this section should not be removed, so a new --remove-ctf option
is added to indicate explicitly that .ctf section will be stripped
out from binary file.
Signed-off-by: Guillermo E. Martinez
---
ChangeLog | 23
libebl/eblsectionstripp.c | 4 +-
libebl/libebl.h| 2 +-
libelf/elf-knowledge.h | 7 +-
libelf/elf32_checksum.c| 2 +-
src/elfcmp.c | 4 +-
src/strip.c| 71 --
tests/Makefile.am | 6 +-
tests/run-strip-remove-keep-ctf.sh | 207 +
tests/testfile-ctf.bz2 | Bin 0 -> 3317 bytes
10 files changed, 304 insertions(+), 22 deletions(-)
create mode 100755 tests/run-strip-remove-keep-ctf.sh
create mode 100755 tests/testfile-ctf.bz2
diff --git a/ChangeLog b/ChangeLog
index f1a14b5c..2b608866 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2022-05-21 Guillermo E. Martinez
+
+ * libebl/eblsectionstripp.c (ebl_section_strip_p): Use
+ remove_ctf argument.
+ (SECTION_STRIP_P): Likewise.
+ * libebl/libebl.h (ebl_section_strip_p): Likewise.
+ * libelf/elf-knowledge.h (SECTION_STRIP_P): Update macro
+ definition to use remove_ctf to determine whether .ctf
+ section is stripped out.
+ * libelf/elf32_checksum.c (elfw2): Use false value for
+ remove_ctf parameter.
+ * src/elfcmp.c (main): Likewise.
+ * src/strip.c (options): Add --remove-ctf option set by
+ remove_ctf variable.
+ (set_remove_special_section_opt): Add new function.
+ (erratic_special_section_opt): Likewise.
+ (parse_opt): Parse new --remove-ctf option.
+ (handle_elf): Adjust .comment and use remove_ctf argument.
+ * tests/Makefile.am (TEST): Add run-strip-remove-keep-ctf.sh
+ and testfile-ctf.bz2.
+ * tests/run-strip-remove-keep-ctf.sh: Add new testcase.
+ * tests/testfile-ctf.bz2: Add new test harness.
+
2022-05-02 Mark Wielaard
* Makefile.am (AM_DISTCHECK_CONFIGURE_FLAGS): Remove
diff --git a/libebl/eblsectionstripp.c b/libebl/eblsectionstripp.c
index a5624ffe..f26cc170 100644
--- a/libebl/eblsectionstripp.c
+++ b/libebl/eblsectionstripp.c
@@ -37,7 +37,7 @@
bool
ebl_section_strip_p (Ebl *ebl, const GElf_Shdr *shdr,
const char *name, bool remove_comment,
-bool only_remove_debug)
+bool only_remove_debug, bool remove_ctf)
{
/* If only debug information should be removed check the name. There
is unfortunately no other way. */
@@ -66,5 +66,5 @@ ebl_section_strip_p (Ebl *ebl, const GElf_Shdr *shdr,
return false;
}
- return SECTION_STRIP_P (shdr, name, remove_comment);
+ return SECTION_STRIP_P (shdr, name, remove_comment, remove_ctf);
}
diff --git a/libebl/libebl.h b/libebl/libebl.h
index 731001d3..067b769e 100644
--- a/libebl/libebl.h
+++ b/libebl/libebl.h
@@ -205,7 +205,7 @@ extern bool ebl_relative_reloc_p (Ebl *ebl, int reloc);
/* Check whether section should be stripped. */
extern bool ebl_section_strip_p (Ebl *ebl,
const GElf_Shdr *shdr, const char *name,
-bool remove_comment, bool only_remove_debug);
+bool remove_comment, bool only_remove_debug,
bool remove_ctf);
/* Check if backend uses a bss PLT in this file. */
extern bool ebl_bss_plt_p (Ebl *ebl);
diff --git a/libelf/elf-knowledge.h b/libelf/elf-knowledge.h
index 6e005fa5..903a0f4f 100644
--- a/libelf/elf-knowledge.h
+++ b/libelf/elf-knowledge.h
@@ -34,7 +34,7 @@
/* Test whether a section can be stripped or not. */
-#define SECTION_STRIP_P(shdr, name, remove_comment) \
+#define SECTION_STRIP_P(shdr, name, remove_comment, remove_ctf) \
/* Sections which are allocated are not removed. */ \
(((shdr)->sh_flags & SHF_ALLOC) == 0 \
/* We never remove .note sections. */\
@@ -45,7 +45,10 @@
&& strncmp (name, ".gnu.warning.", sizeof ".gnu.warning." - 1) != 0\
/* We remove .comment sections only if explicitly told to do so. */\
&& (remove_comment \
- || strcmp (name, ".comment") != 0
+ || strcmp (name, ".comment") != 0) \
+ /* We remove .ctf sections only if explicitly told to do so. */\
+ && (remove_ctf \
+ || strcmp (name, ".ctf") != 0
/* Test whether `sh_info' field in section header contains a section
diff --git a/libelf/elf32_chec
