Hi Frank,
On Thu, 2024-10-10 at 17:24 -0400, Frank Ch. Eigler wrote:
> From be79d138989b9968f9c687ef62cc91b5b93e32b5 Mon Sep 17 00:00:00 2001
> From: "Frank Ch. Eigler" <f...@redhat.com>
> Date: Thu, 10 Oct 2024 16:30:19 -0400
> Subject: [PATCH] PR32218: debuginfod-client: support very long source file
> names
>
> debuginfod clients & servers may sometimes encounter very long source
> file names. Previously, the client would synthesize a path name like
> $CACHEDIR/$BUILDID/source-$PATHNAME
> where $PATHNAME was a funky ##-encoded version of the entire source
> path name. This can get too long to store as a single component
> of a file system pathname (e.g. linux/limits.h NAME_MAX), resulting
> on client-side errors even after a successful download.
Maybe add the bugzilla URL as an example of this.
I see it is in the subject, but it is nice to just click on something.
> New code switches encoding of the $PATHNAME part to use less escaping,
> and a merciless truncation to the tail part of the filename. (We keep
> the tail rather than the head, so that the extension is preserved,
> which makes some consumers happier.) To limit collision damage from
> truncation, we add also insert a goofy hash (4-byte elf_hash) of the
> name into the path name.
Is this 4-byte hash enough to prevent accidental collisions?
> The result is a relatively short name:
>
> $CACHEDIR/$BUILDID/source-$HASH-$NAMETAIL
>
> This is a transparent change to clients, who are not to make any
> assumptions about cache file naming structure.
How does it interact with existing source file cache files?
> However, one existing
> test did make such assumptions, so is fixed with some globness. A new
> test is also added, using a pre-baked tarball with a very long srcfile
> name. Some auxiliary makefiles are tweaked to satisfy the new
> libdebuginfod->libelf link order dependency.
Are you sure you want to use elf_hash? It returns an (arch dependent)
unsigned long int but the actual implementation only produces an
unsigned int. To simplify things and not need extra build dependencies
you might want to just include a hand-coded string hash function like
djb2? Which is really just 4 lines of code and probably a better hash
for strings than elf_hash.
> Signed-off-by: Frank Ch. Eigler <f...@redhat.com>
> ---
> debuginfod/Makefile.am | 4 +-
> debuginfod/debuginfod-client.c | 76 +++++++++++-------
> src/Makefile.am | 2 +-
> tests/Makefile.am | 7 +-
> .../bighello-sources/bighello.c | 7 ++
> .../bighello-sources/bighello.h | 1 +
> .../moremoremoremoremoremoremoremore | 1 +
> tests/debuginfod-tars/bighello.tar | Bin 0 -> 51200 bytes
> tests/run-debuginfod-longsource.sh | 69 ++++++++++++++++
> tests/run-debuginfod-section.sh | 16 ++--
> 10 files changed, 144 insertions(+), 39 deletions(-)
> create mode 100644 tests/debuginfod-tars/bighello-sources/bighello.c
> create mode 100644 tests/debuginfod-tars/bighello-sources/bighello.h
> create mode 120000
> tests/debuginfod-tars/bighello-sources/moremoremoremoremoremoremoremore
> create mode 100644 tests/debuginfod-tars/bighello.tar
> create mode 100755 tests/run-debuginfod-longsource.sh
>
> diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
> index 5ad4e188c4c3..dbba2a437c80 100644
> --- a/debuginfod/Makefile.am
> +++ b/debuginfod/Makefile.am
> @@ -70,10 +70,10 @@ bin_PROGRAMS += debuginfod-find
> endif
>
> debuginfod_SOURCES = debuginfod.cxx
> -debuginfod_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod)
> $(argp_LDADD) $(fts_LIBS) $(libmicrohttpd_LIBS) $(sqlite3_LIBS)
> $(libarchive_LIBS) $(rpm_LIBS) $(jsonc_LIBS) $(libcurl_LIBS) $(lzma_LIBS)
> -lpthread -ldl
> +debuginfod_LDADD = $(libdw) $(libdebuginfod) $(libelf) $(libeu)
> $(argp_LDADD) $(fts_LIBS) $(libmicrohttpd_LIBS) $(sqlite3_LIBS)
> $(libarchive_LIBS) $(rpm_LIBS) $(jsonc_LIBS) $(libcurl_LIBS) $(lzma_LIBS)
> -lpthread -ldl
>
> debuginfod_find_SOURCES = debuginfod-find.c
> -debuginfod_find_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod)
> $(argp_LDADD) $(fts_LIBS) $(jsonc_LIBS)
> +debuginfod_find_LDADD = $(libdw) $(libdebuginfod) $(libelf) $(libeu)
> $(argp_LDADD) $(fts_LIBS) $(jsonc_LIBS)
>
> if LIBDEBUGINFOD
> noinst_LIBRARIES = libdebuginfod.a
Move libelf after libdebuginfod. OK (but see above, isn't it simpler to
just include a small string hash function right in the code itself?)
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index 24ede19af385..2f56f6f4d784 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -1201,29 +1201,49 @@ perform_queries(CURLM *curlm, CURL **target_handle,
> struct handle_data *data, de
> /* Copy SRC to DEST, s,/,#,g */
>
> static void
> -path_escape (const char *src, char *dest)
> +path_escape (const char *src, char *dest, size_t dest_len)
> {
> - unsigned q = 0;
> + /* PR32218: Reversibly-escaping src character-by-character, for
> + large enough strings, risks ENAMETOOLONG errors. For long names,
> + a simple hash based generated name instead, while still
> + attempting to preserve the as much of the content as possible.
> + It's possible that absurd choices of incoming names will collide
> + or still get truncated, but c'est la vie.
> + */
> + const size_t max_dest_len = NAME_MAX/2; /* ENAMETOOLONG seen even on
> 300-ish byte paths. */
> + dest_len = dest_len > max_dest_len ? max_dest_len : dest_len; /* Reduce! */
> + const size_t hash_prefix_destlen = strlen(src)+10; /* DEADBEEF-string\0 */
> + dest_len = dest_len > hash_prefix_destlen ? hash_prefix_destlen :
> dest_len; /* Reduce further! */
What do these "Reduce" comments mean?
So we hope to get at least the original input path length plus 10 (8
char hash plus dash plus zero), but we are fine with less.
> - for (unsigned fi=0; q < PATH_MAX-2; fi++) /* -2, escape is 2 chars. */
> - switch (src[fi])
> - {
> - case '\0':
> - dest[q] = '\0';
> - return;
> - case '/': /* escape / to prevent dir escape */
> - dest[q++]='#';
> - dest[q++]='#';
> - break;
> - case '#': /* escape # to prevent /# vs #/ collisions */
> - dest[q++]='#';
> - dest[q++]='_';
> - break;
> - default:
> - dest[q++]=src[fi];
> - }
> + char *dest_write = dest + dest_len - 1;
> + (*dest_write--) = '\0'; /* Ensure a \0 there. */
OK, now we have a \0 terminator at the end of dest and dest_write will
point just before it.
> - dest[q] = '\0';
> + /* Copy from back toward front, preferring to keep the .extension. */
> + for (int fi=strlen(src)-1; fi >= 0 && dest_write >= dest; fi--)
> + {
> + char src_char = src[fi];
> + switch (src_char)
> + {
> + /* Pass through ordinary identifier chars. */
> + case '.': case '-': case '_':
> + case 'a'...'z':
> + case 'A'...'Z':
> + case '0'...'9':
> + *dest_write-- = src_char;
> + break;
> +
> + /* Replace everything else, esp. security-sensitive /. */
> + default:
> + *dest_write-- = '#';
> + break;
> + }
> + }
OK, work backwards copying/replacing chars. Stop if we are at the start
of the dest buffer or we copied over all of the src string. If we reach
the start of src then we should have exactly 9 chars left.
> + unsigned int name_hash = elf_hash (src);
Yeah, this is 4bytes everywhere, still would love to see it being an
int32_t if that is what you want.
> + char name_hash_str [9];
> + snprintf (name_hash_str, sizeof(name_hash_str), "%08x", name_hash);
> + memcpy (&dest[0], name_hash_str, 8); /* Overwrite the first few characters
> */
> + dest[8] = '-'; /* Add a bit of punctuation to make hash stand out */
> }
OK, so this always adds (or overwrites) a 8 char hash plus dash '-' in
front. Have you considered to only add it if the src doesn't fit the
dest_len?
> /* Attempt to update the atime */
> @@ -1332,13 +1352,14 @@ extract_section (int fd, const char *section, char
> *fd_path, char **usr_path)
> --i;
> }
>
> - escaped_name = malloc (strlen (section) * 2 + 1);
> + size_t escaped_size = strlen (section) * 2 + 1;
> + escaped_name = malloc (escaped_size);
> if (escaped_name == NULL)
> {
> rc = -ENOMEM;
> goto out;
> }
> - path_escape (section, escaped_name);
> + path_escape (section, escaped_name, escaped_size);
>
> rc = asprintf (&sec_path_tmp, "%s/section-%s.XXXXXX",
> fd_path, escaped_name);
OK we pass around the escaped_size.
> @@ -1861,13 +1882,13 @@ debuginfod_query_server_by_buildid (debuginfod_client
> *c,
> goto out;
> }
>
> - path_escape (filename, suffix);
> + path_escape (filename, suffix, sizeof(suffix));
> /* If the DWARF filenames are super long, this could exceed
> PATH_MAX and truncate/collide. Oh well, that'll teach
> them! */
> }
> else if (section != NULL)
> - path_escape (section, suffix);
> + path_escape (section, suffix, sizeof(suffix));
> else
> suffix[0] = '\0';
sizeof(suffix) == PATH_MAX
What is the relation to NAME_MAX?
> @@ -1879,7 +1900,8 @@ debuginfod_query_server_by_buildid (debuginfod_client
> *c,
> cache_path: $HOME/.cache
> target_cache_dir: $HOME/.cache/0123abcd
> target_cache_path: $HOME/.cache/0123abcd/debuginfo
> - target_cache_path: $HOME/.cache/0123abcd/source#PATH#TO#SOURCE ?
> + target_cache_path: $HOME/.cache/0123abcd/executable-.foo.bar
> (section)
> + target_cache_path: $HOME/.cache/0123abcd/source-HASH#PATH#TO#SOURCE
> */
>
The comment confuses me a little. is .foo and/or .bar the section name?
>
> cache_path = make_cache_path();
> @@ -1891,10 +1913,10 @@ debuginfod_query_server_by_buildid (debuginfod_client
> *c,
> xalloc_str (target_cache_dir, "%s/%s", cache_path, build_id_bytes);
> (void) mkdir (target_cache_dir, 0700); // failures with this mkdir would
> be caught later too
>
> - if (section != NULL)
> + if (suffix[0] != '\0') /* section, source queries */
> xalloc_str (target_cache_path, "%s/%s-%s", target_cache_dir, type,
> suffix);
> else
> - xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type,
> suffix);
> + xalloc_str (target_cache_path, "%s/%s", target_cache_dir, type);
> xalloc_str (target_cache_tmppath, "%s.XXXXXX", target_cache_path);
Maybe related to my confusion about the comment above. Why this change?
> /* XXX combine these */
> diff --git a/src/Makefile.am b/src/Makefile.am
> index e0267d96f60d..f50287e5734a 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -97,7 +97,7 @@ stack_LDADD = $(libebl) $(libelf) $(libdw) $(libeu)
> $(argp_LDADD) $(demanglelib)
> elfcompress_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> elfclassify_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> srcfiles_SOURCES = srcfiles.cxx
> -srcfiles_LDADD = $(libdw) $(libelf) $(libeu) $(argp_LDADD)
> $(libarchive_LIBS) $(libdebuginfod)
> +srcfiles_LDADD = $(libdw) $(libdebuginfod) $(libelf) $(libeu) $(argp_LDADD)
> $(libarchive_LIBS)
>
> installcheck-binPROGRAMS: $(bin_PROGRAMS)
> bad=0; pid=$$$$; list="$(bin_PROGRAMS)"; for p in $$list; do \
More, link libelf after libdebuginfod.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 424c184bc200..ffccb0cd0e1f 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -273,7 +273,8 @@ TESTS += run-srcfiles-self.sh \
> run-debuginfod-section.sh \
> run-debuginfod-IXr.sh \
> run-debuginfod-client-profile.sh \
> - run-debuginfod-find-metadata.sh
> + run-debuginfod-find-metadata.sh \
> + run-debuginfod-longsource.sh
> endif
> if !OLD_LIBMICROHTTPD
> # Will crash on too old libmicrohttpd
> @@ -607,6 +608,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> run-debuginfod-IXr.sh \
> run-debuginfod-ima-verification.sh \
> run-debuginfod-find-metadata.sh \
> + run-debuginfod-longsource.sh \
> debuginfod-rpms/fedora30/hello2-1.0-2.src.rpm \
> debuginfod-rpms/fedora30/hello2-1.0-2.x86_64.rpm \
> debuginfod-rpms/fedora30/hello2-debuginfo-1.0-2.x86_64.rpm \
> @@ -654,6 +656,9 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> debuginfod-tars/pacman-sources/PKGBUILD \
> debuginfod-tars/pacman-sources/README.md \
> debuginfod-tars/pacman-sources/hello.c \
> + debuginfod-tars/bighello.tar \
> + debuginfod-tars/bighello-sources/bighello.c \
> + debuginfod-tars/bighello-sources/bighello.h \
> run-pt_gnu_prop-tests.sh \
> testfile_pt_gnu_prop.bz2 testfile_pt_gnu_prop32.bz2 \
> run-getphdrnum.sh testfile-phdrs.elf.bz2 \
> diff --git a/tests/debuginfod-tars/bighello-sources/bighello.c
> b/tests/debuginfod-tars/bighello-sources/bighello.c
> new file mode 100644
> index 000000000000..e1145271b083
> --- /dev/null
> +++ b/tests/debuginfod-tars/bighello-sources/bighello.c
> @@ -0,0 +1,7 @@
> +#include <stdio.h>
> +#include
> "moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/bighello.h"
> +
> +int main() {
> + printf("%s\n", HELLO);
> + return 0;
> +}
> diff --git a/tests/debuginfod-tars/bighello-sources/bighello.h
> b/tests/debuginfod-tars/bighello-sources/bighello.h
> new file mode 100644
> index 000000000000..e3de04b850eb
> --- /dev/null
> +++ b/tests/debuginfod-tars/bighello-sources/bighello.h
> @@ -0,0 +1 @@
> +char *HELLO = "hello";
> diff --git
> a/tests/debuginfod-tars/bighello-sources/moremoremoremoremoremoremoremore
> b/tests/debuginfod-tars/bighello-sources/moremoremoremoremoremoremoremore
> new file mode 120000
> index 000000000000..945c9b46d684
> --- /dev/null
> +++ b/tests/debuginfod-tars/bighello-sources/moremoremoremoremoremoremoremore
> @@ -0,0 +1 @@
> +.
> \ No newline at end of file
> diff --git a/tests/debuginfod-tars/bighello.tar
> b/tests/debuginfod-tars/bighello.tar
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..6d5d1d67c72e51569a35db6cff28cab5efaca384
> GIT binary patch
> literal 51200
> zcmeI5eQX>@6~Nz~<M<LMj??tRX((CJrX+yZyLaci)FI6|j^lIdBqbkh5iQI4Zhc;H
> z-ZOi9U{gK<<)fj1f`s@|kf?+JRjLF6Ap{Zx1+{?qqZRzo|A<f{1r?|f1qw)A^k(O+
> z_tqOb3CVU#<|p}PXZF3B*_oX;v$Hd6XJ&R_(yUDz^?IYf*=X4nqd5Rq7gbf&!r-9i
> z=5x%|a-PcpEmu%8YF5o<GoWhOLaqSv;ML7WG_9snwq+TnD$_=^gh#7s*v;hxUY|Ho
> z^*__FjphC>^9H%p`q%OqHK%EyW-|FfQU9@UW%a+@&4K6IZ;;Eb|ExBc&x`gS2g-ml
> zP@HI3wF%RD?=?yg>p#C}@0nj!eOJ{A8ije`nwD2HAWvMQ{Io&#-)=OVwi6*%I{!N5
> z8nyq+GZ2E;j5z8a8}+4(|NQ<xo68n*!v4oW66}8l$NY-`v;V5F|8WpU`;T~<5FJE9
> zh@<|6{ZB55u>XbZQu{wB?0+1@(f+T<{zqvA!`i191XoBL^)Kvya!G>y&t}`-1+ejd
> zR@nbIh@<^q8~Yy>Z7@u95SKz6^)Kvya!G{!FL>{UIn4fRV*V!<;%NV`k^PTKHy9>5
> z2!L4XU)cZTk_7vo%jTEP|K!E^KMt;E`ycEKqK>bJ4kC{F7xq87B*Oj|3QO$2Cgy))
> zA&&N+)bW2*#)4rh3&c`I*OWNwU)cZTk_7udm|I*6%GQO7^`Eg3NBjT(wf|9<77R;%
> z2N6g83;Um3l3@SyYQ*|~G5;S6akT&I*#1Xddob)Z?;zr+e_{WVOCs!lE@J(^SpOLd
> zakPJ7|J!VSusmM#4kC{F7xq87B*Fd{)QI)}V*WoC;%NWE{<qouV0o-;|AW#|$N%FK
> zL>%=m?0<4eg#BOqHUuKpf5u20?O)jcNXw1ova)%PD!pLX$^y&YO>xw}u>Z*=0rp>0
> z)g|ly2MYzU{xc5ZX#c|gw^{69d8}-`gVIsQS3w|-`%I8b;%NVu+W+JCKeJi}*!Q1Q
> z@%^VbNJRgyEnWXBzW)~wakPKY|3~)RST0wg_l_#wVA#q6^!;aLI-Yw@kR0M@|JeW2
> z_g^Ew|D7qY?|*B9g{+F-g3ab~xc^U$qnPOXuh;4Kzbn&aTYgJvbYkMLyhq-~z5~B)
> z_u4p0UXTDH+rQr-RIW-6S7`s5rg`n3jsI2r{{LVezyB)Q|F!3K)2h^4RYQJz)2W&b
> zW%};5e$ckyFe2)B<)KBCRxnHmcw@svl!M>wr?;(LYg&#xQ#P#~JLN~#%CbCbo0c=R
> zW83!TL)Nyp$*jlNxf{>ehSRbwS>3(%!=m4Ky%A~u%SxlQ?8opgkLBloa#`H})&Boq
> za)qoW?0+15badiCN|OAfQgAn*uh?5;VcsvmxjnuJ6k!*vha%hw+hH{TtU`Hyy2y{n
> zeLm%#AUDnBI=EgR9sodJhZh66Z{Tv|QV!9@^cGo68A*3~XUK(K&yRZS6!!zaK5`H8
> z1jr>hKRwxt2V|ZecYnu&$qnjZ*}T&43B0n$(<4{r<+6jjq=#PAODmpU`fg8;+-G?{
> z$))ndGQq!HygYaD@+7ytlwgV89M_xUdgSis*GDdu{n7JI^-9vakL&gE^6cZ~NbcKs
> zdgM}i?>O&Nv0N?{s^%j+y^GaDnwLMhym(=tZcgTN1NCZu-LzVB{d2qW{rQ~IY$zF)
> z1<V(YPw}1_J9HdUC*TZ*k-h{{n?J?<1j^nBz}f-ofAs0o-@WjkuiyK?`7geC%U6eg
> z==47MB+B4*V4Ns}U^US#gWx&(yBx4AZ-`L9xT3=E0`Qwa8?!B^3A$c4Co4MsD=kOI
> zQ3IeKODhj!6XW|v^o){G2L0n~4s^XX)3CUT&aSEhOAS)c$FCb{-D<AEtKP=Le(;4a
> zt;Q1I%>$m_>)qLSJ(zdmTp*m^+8#wuzAIFY0)apv5C{YU!S%r3HoWoQrAPmiE<N7)
> z+yFr7lV_Zi`)ujaU!>2n%XbHVJpkZt|IGlvhOI?#x1*oEDE{L)*L6`}`}6q8=iXg9
> z`G?Y@FD%@5Y*ag={i^iLGk0z*f_oGGH{qA3lnq-y!Qw;tvy0wSKk6$Mp|l5oolQSN
> z5zNAxpJ9M|ZtB$8sYPK-|EEm;V}D`J$%WF-|FXCA^H(}b($l5qUUfG5F?8`5!upv0
> zi9NUBnzz=b<E4}5FMQB--KRUd2LPmp&n`(1<zM^^3YH%4{JjEDI{D0nf4=IvETxCf
> z;(1nneu)VL0)apv5C{YUfk2Qv@O@hlwg430rETfhjqhOb9b|gJb!P!!$AasY0N}v|
> z*L?^8dj9IVU*XF5E)$VHcm$-mUTMp^uJkF%dw+@V@SlIlb?-!h{awBIP7~jqzWkEw
> z^4Iln2c80eQ!lwLdo|qCd!VQ9o(<g}NS}bcx9)yR_6~dn$l|I4fVJSdw_?C>Pw%Hv
> zBkNZmU)M$N!UY0>Kp+qZ1OkCTu!y|TTL-zo*Fn?TNnCr0tm7`Nk6zFHMXtA*`)NJ%
> z7Vf8KF!pSQ@b$~Ij+@T^{Z$tqGvN$hZ%ogD#Z`;zIgxYxJnbyO=TD@YyOiIXxSyak
> zpBMN#P<qNGd3s*Ob)d*C++E=5Q}`L~r+m|%v^89w;(OKguICh&ce$U+b%FaI<LBds
> z){lqD(>i!gFJF&6HZn3K?-)CDe5b7C6iw0OjGD=*1yz%G95JeLsqF9*{e_(&=?qEV
> zrDfWrGm6%q-3i0@59MaH%wPeI+&^TuEX%ZNa;;Jcrv&@%A1Yf`W74!DPmet?q|Ozp
> zd37+Kkrid2Ip3@rv(16IUHO6Xq}gvZ97CzV(ffw7aCBrygOLM6yOt3=FfpVqb9!ue
> zNQ3*vhjMUqbVvpB=#bi;1=BJerQ)T+k6D!bxGlR)r|g)Oe%q)wDqbGda2U&NNRGu9
> z3S;*sTkkFbj1Au*Wy_kse`AW*FW$dlnHE1sZj=nQ|C^VIhipQL+lHukPH=r(6oN#_
> zP(0*bsSM?H%QAVUydLEF!MQHaMqsMo=hL@iA->7h{!2_aH3$H=-OMHE3WJ(bn%NU+
> zRTtVhNlKqUC)I`4Ly}g1ocTK6itGDVb()p~Qo8f=IQy2yxh_dseG-$yb@?f&YYG6?
> zboBCt?5n?w5*?inMr2w#4bmOZ-F<g=M-M=1qqMzeQ+LPKEn7EAH+1#h0?@(FrU6!M
> zfbQ<`O#q!a=;#^h-rapy_xPs0n^S!Ns}<P0c`*>Ri+vx$F5m%H3~9)+Y)b#>Sr3xL
> zwi8L1Z)qzl(>Jk<Z%u9JV1>kfdQ$*W=Q>!Eq5`|IbJe|_-RC={ccxMxN%e+RlUr8P
> ztXW1?#zJ9x4xu^Su$}f3UJ#0m9Xc+LXa%$+D_9}9f5vH9#-5sC8MaxG`)A5_WqOad
> z&qMC7fuA&Pkq4F=Oo8og7>L?72mb!a3IzAKMja<&zh_j&f?-175;x~A*8GZJP{HxQ
> zD)_4#GAGRHNU#b{^YIRCH^Vj`C4U>US3SO6CARYL?{eEm;I5?<^!VdF0>@JlYzACU
> zhtPQC7(2evAHNZg@OWtaNPL>fZ}P{3#J4Vz|L<KS{KqVw8{k>mXBQx<9Dd2<Z(1_G
> zd5-CA^!Jq!FEIIA{Ba@i?~Ckv+9Tn<r#?vY@*!@K0AmL-!(OieaGMmpe@bERLi+r2
> zCI(SHwZZT6h}~R1oNkHbXDJV0ep0=V4wYM#<+In{w}|g^kS2?NGsvOqnq&Ftfmwe4
> z(71e<U007ku3+zS@O=%BhwdNZG?ypiPQBq1ET2ou;VG8xI)A+3Q?VUQX@IVmCr#Zc
> z*DzqxsF{}1fU02|HM8j$w(iX6m3qT6a4%Z5q1Wn-$#Pw<It{z2ms@jCY0S*l4acY|
> zYP5ttWm=|Qw(at~Zdi^z4^wt|#?Y&+nVEUe^?*;u0bQGD!17mY$FQm_#|MrK9~{+3
> z5AD}=(D%Rd(D1?W5zzIqL&x>e5<gqoe+2ZgiNpJbC-lPy4jdglrXL&LH!-R&-BVYj
> zopjk8?+S76oga$(>vW@9cFMr61uwJgl)W8x;q7+1UTrq?>9SR=8=#LL*755PmfmU_
> zRnQHq3VOA!v%zWEF)#|ptmw0*0lGffZ1Ol+SE%d94vx@8V!;3?&G{LpJPAtEu|0R1
> zj!erj>{(FQ(7a}~lv%qmYuL_wAZW5>)~o$yl?M&)8}G+@XX4Z4<}@hPd8;`;<GGIQ
> zoq4}uH_e9C=Fx3XY@=RA1%5PJcR*p;R2*Z@0R^ia6uZF+PBEr=MNe1lMW3hP)wriY
> z$JKeOJY!ZssWlw-<0arlHrZ@~QfbW07?uMHf0ALB-t@A`u2d;Ij%`l191N`*ldYOw
> zw#!y+(Q8^$4gb{Sq;0&Pe09?@=$Ms+r;owR|G$IJ_1Gu)&(FMnqJ7)+Tu)H{LEH||
> zw%3yOBhx-*uh&_G>?n~B>C?Vs+OLf7jf0SVBgRAe)W4qQ69m-H5@CHD;QqsP8+83N
> zUvY-()AiHd<Z%AWe6E7@X+Gl|*MARB7@rTY>`0&H70l3{Uz#5&@{H1a2oWy-_W`iE
> z2uh#kA0FfSlpf`u<jDO90Grz&Kh1BP;`%fn63##IIo8iF-aj;-!Zt_qbpQyZPx+zi
> z$EO=iY9Eh~=4Tdo`Xq<?IENF~e}b1Q#ZS+(-or5X^Hv0p*5`CcU*`Ry%pRtf?uq8X
> zzWeQvzW-eX5)4%s1rtw&_4)H^aib?e^KwKu|KI2ORDbl{0=n_(sg<DeBYqOnr+xag
> zf1md06XEoK71A&9@kxm{yw8#%C!GFoah(Zo0|?DG(tiH1Jo(A}JpgP7=+k^8Z6GH(
> zg8byZ$nQ;>Po#M?`aT1Fhk@n`NssFBWq$pnPy5xM=U-hWIfC@b{Z~kz<})vF{jmKa
> zJ#tg{93HrS8c(Kq!@XGgL|XFN6y=}Bd3{`;(jh{(Gpy(7lbs+)AJ-8DA$@Xhmb`Bs
> zg!LUBaF_%+<Ry2z<n=#<{2s5z`Gw|%>?c7^`2NLlcu0hi#u<-*{UXA$_gqAO7uWv|
> zDRRR4_g_T+9M{J$Cj<%W<M=-uhV0FGTJH)f^Ls7GEY}a0|JS%a+Q%T27P*gHL_d#n
> zW@vwX!{kAFK{q7Ac(CBPC4OV#H~^oO>3*bs0q5!I-VO4NA<y@HN{AEk0-D(<{iAR9
> h6dn&LguDWQKp+qZ1OkCTAP@)y0)apv5C{aZ^Iv?1%}oFR
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/run-debuginfod-longsource.sh
> b/tests/run-debuginfod-longsource.sh
> new file mode 100755
> index 000000000000..ae071d1b700e
> --- /dev/null
> +++ b/tests/run-debuginfod-longsource.sh
> @@ -0,0 +1,69 @@
> +#!/usr/bin/env bash
> +#
> +# Copyright (C) 2024 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 <http://www.gnu.org/licenses/>.
> +
> +. $srcdir/debuginfod-subr.sh
> +
> +# for test case debugging, uncomment:
> +set -x
Missing # ? Or use it just unconditionally?
> +unset VALGRIND_CMD
> +
> +DB=${PWD}/.debuginfod_tmp.sqlite
> +tempfiles $DB
> +export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
> +
> +# Set up directories for scanning
> +mkdir Z
> +cp -rvp ${abs_srcdir}/debuginfod-tars/bighello.tar Z
> +
> +# This variable is essential and ensures no time-race for claiming ports
> occurs
> +# set base to a unique multiple of 100 not used in any other
> 'run-debuginfod-*' test
> +base=14200
> +get_ports
> +
> +# We use -t0 and -g0 here to turn off time-based scanning & grooming.
> +# For testing purposes, we just sic SIGUSR1 at the process.
> +
> +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS=
> ${abs_builddir}/../debuginfod/debuginfod $VERBOSE \
> + -Ztar -p $PORT1 -d $DB -t0 -g0 -v ./Z > vlog$PORT1 2>&1 &
> +PID1=$!
> +tempfiles vlog$PORT1
> +errfiles vlog$PORT1
> +# Server must become ready
> +wait_ready $PORT1 'ready' 1
> +# And initial scan should be done
> +wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
> +
> +kill -USR1 $PID1 # run another index pass to make sure the srcdef/srcref
> stuff is fully located
> +
> +# Wait till both files are in the index.
> +wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
> +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> +wait_ready $PORT1 'thread_busy{role="scan"}' 0
> +
> +export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1
> +
> +########################################################################
> +
> +# Build-id for a.out in said tarball
> +BUILDID=7fc69cb0e8fb9d4b57e594271b9941b67410aaaa
> +
> +# Download short & long files
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv source $BUILDID
> /tmp/bighello-sources/bighello.c
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv source $BUILDID
> /tmp/bighello-sources/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/moremoremoremoremoremoremoremore/bighello.h
> +
> +exit 0
Right, looks like a test that might fail before this patch.
> diff --git a/tests/run-debuginfod-section.sh b/tests/run-debuginfod-section.sh
> index 6ac5968872e3..1746d7ef7d43 100755
> --- a/tests/run-debuginfod-section.sh
> +++ b/tests/run-debuginfod-section.sh
> @@ -95,11 +95,11 @@ testrun ${abs_top_builddir}/debuginfod/debuginfod-find
> -vvv section $RPM_BUILDID
> # Verify that the downloaded files match the contents of the original
> sections
> tempfiles ${BUILDID}.debug_info
> objcopy F/prog.debug -O binary --only-section=.debug_info
> --set-section-flags .debug_info=alloc $BUILDID.debug_info
> -cmp ${BUILDID}.debug_info
> ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
> +cmp ${BUILDID}.debug_info
> ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-*.debug_info
>
> tempfiles ${BUILDID}.text
> objcopy F/prog -O binary --only-section=.text ${BUILDID}.text
> -cmp ${BUILDID}.text ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
> +cmp ${BUILDID}.text ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-*.text
>
> # Download the original debuginfo/executable files.
> DEBUGFILE=`env LD_LIBRARY_PATH=$ldpath
> ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $RPM_BUILDID`
> @@ -110,11 +110,11 @@ testrun ${abs_top_builddir}/debuginfod/debuginfod-find
> -vvv executable $BUILDID
> if test "$(arch)" == "x86_64"; then
> tempfiles DEBUGFILE.debug_info
> objcopy $DEBUGFILE -O binary --only-section=.debug_info
> --set-section-flags .debug_info=alloc DEBUGFILE.debug_info
> - testrun diff -u DEBUGFILE.debug_info
> ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
> + testrun diff -u DEBUGFILE.debug_info
> ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-*.debug_info
>
> tempfiles EXECFILE.text
> objcopy $EXECFILE -O binary --only-section=.text EXECFILE.text
> - testrun diff -u EXECFILE.text
> ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
> + testrun diff -u EXECFILE.text
> ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-*.text
> fi
>
> # Kill the server.
> @@ -123,10 +123,10 @@ wait $PID1
> PID1=0
>
> # Delete the section files from the cache.
> -rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
> -rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
> -rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
> -rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-*.text
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-*.debug_info
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-*.text
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-*.debug_info
>
> # Verify that the client can extract sections from the debuginfo or
> executable
> # if they're already in the cache.
Indeed, looks like too specific an expected cache name.
Cheers,
Mark
