rfc patch PR32760: find-debuginfo for static libraries
Frank Ch. Eigler
fche@redhat.com
Fri Jun 13 20:35:05 GMT 2025
Hi -
Thanks for the review.
> > [...]
> > find-debuginfo looks for archive ".a" files in the build tree, and
> > runs debugedit only to enumerate and rewrite source paths. This
> > allows ultimate users of the .a files to receive debuginfo, with
> > usable (and debuginfod-resolvable) source file paths.
>
> Is the thing between brackets (and debuginfod-resolvable) really true?
Absolutely!
> But how does the debuginfod-resolvable part then work? There is no
> reference to where these objects and debuginfo came from in the
> binary/packages that incorporate them is there?
Except the source file full path (canonicalized during rewriting).
That normally includes substrings like the rpm n-v-r, which makes them
unique enough to locate in the n-static-debugsource-v-r.rpm .
> > +If -a is given, then static libraries will be ignored. Otherwise,
> > +they receive only with source-path rewriting and collection. They are
> > +not stripped, since they have no persistent build-ids to accommodate
> > +eventual reunification.
>
> Can I convince you to use a long option like --no-ar-files ?
> Having -a meaning no ar file processing seems a little cryptic.
> We already have too many slightly cryptic since char flags.
No problem.
> > +# Handle ELF archives
> > +do_ar_file()
> > +{
> > + local nlinks="$1" inum="$2" f="$3" id link linked
> > + local tmpa="$temp/output.a"
> > + local res=0
>
> Question about the use of $temp here with a fixed file name. Isn't
> $temp also a "fixed" output dir script wide? If so and if do_ar_file
> can be called in parallel, it seems the tmpa file can clash with other
> do_ar_file runs.
Sorry, I updated the patch on the try-branch after sending it here,
adding the "$inum" into the string.
+ local tmpa="$temp/$inum-output.a"
> > + # Create empty output .a; mktemp would create a 0-byte file, which ar rv doesn't like
> > + ${AR} r "$tmpa" # no members
> > +
> > + ${AR} tvO "$f" | while read line; do
> > + echo "$line"
>
> Probably want to hide this behind $verbose && ...
Will just drop it, there's better diagnostic just below.
> [...]
> > + local tmpdir="$temp/archive-member" # super short lived
>
> Same question for tmpdir as tmpa above, can this clash on concurrent
> runs?
Yeah, previously fixed the same way ($inum).
> > + # extract the file by offset
> > + (cd "$tmpdir"; dd status=noxfer if="$f" of="$member_dn$member_bn" bs=1 skip="$offset" count="$size")
> > + if [ $? -ne 0 ]; then
> > + res=1
> > + fi
>
> Maybe add a comment here why you cannot just use ar to extract it by
> name.
Sure.
> [...]
> > + $verbose && echo found $(tr -dc '\0' < "$SOURCEFILE" | wc -c) source files
>
> Nice, but shouldn't that be prefixed with the ar file name?
Added a similar $verbose print of the "extracting debuginfo from ..."
message from the non-archive case.
> Should the same be done for none-ar-file processing?
Sure.
> > + local ar_re="^.*\.a$"
> > + if [[ $f =~ $ar_re ]]; then # treat as static archive
> > + do_ar_file "$1" "$2" "$3"
> > + return
> > + fi
> > +
> > get_debugfn "$f"
> > [ -f "${debugfn}" ] && return 0
> >
>
> I think the check for ar files should go after this check whether the
> file is (already) under /usr/lib/debug. The contract seems to be that
> we don't process files already there.
OK.
> > +int FOO (void);
> > +int FOO (void) {
> > + return 0;
> > +}
>
> SHOUTING foo now? :)
YES! YES!!!!
> > +# expect original source tree refs
> > +AT_CHECK([$READELF --debug-dump=line subdir_build/dupes.a | grep `pwd`], [0], [stdout], [ignore])
> > +
> > +AT_CHECK([which find-debuginfo], [0], [stdout], [ignore])
>
> What is this checking?
That the script is properly installed. Kinda trivial, maybe
pointless, also harmless. :)
Patch follows (also in git at users/fche/try-pr32760v10)
------------------------------------------------------------------------
commit 5b58b4e62613d7c448e8f4fe1fc3e327aebf72fb (HEAD -> main, origin/remotes/origin/users/fche/try-pr32760v10)
Author: Frank Ch. Eigler <fche@redhat.com>
Date: Thu Mar 6 14:45:36 2025 -0500
PR32760: find-debuginfo: handle static libraries
Formerly, static .a libraries with debuginfo were ignored by
find-debuginfo. If unstripped, then the raw build-directory paths in
them would leak into the downstream package. New code in
find-debuginfo looks for archive ".a" files in the build tree, and
runs debugedit only to enumerate and rewrite source paths. This
allows ultimate users of the .a files to receive debuginfo, with
usable (and debuginfod-resolvable) source file paths.
This works by having find-debuginfo find .a files, extracting all
the .o files one at a time, running debugedit, then repacking the
files back into the .a. do_file() delegates to do_ar_file(),
doing rather less work than for .so/exec files.
New autotest case covers duplicate-member-name functionality,
including with cruelly whitespace named members.
Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
diff --git a/Makefile.am b/Makefile.am
index 562ffa46fa30..99555bdad153 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,6 +36,7 @@ do_subst = ($(SED) -e 's,[@]PACKAGE[@],$(PACKAGE),g' \
-e 's,[@]READELF[@],$(READELF),g' \
-e 's,[@]OBJCOPY[@],$(OBJCOPY),g' \
-e 's,[@]NM[@],$(NM),g' \
+ -e 's,[@]AR[@],$(AR),g' \
-e 's,[@]DWZ_J[@],$(DWZ_J),g')
find-debuginfo: $(top_srcdir)/scripts/find-debuginfo.in Makefile
diff --git a/configure.ac b/configure.ac
index ad4d70c63995..47771d4ca807 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,6 +47,7 @@ AC_CHECK_TOOL([LD], [ld])
AC_CHECK_TOOL([READELF], [readelf])
AC_CHECK_TOOL([OBJCOPY], [objcopy])
AC_CHECK_TOOL([NM], [nm])
+AC_CHECK_TOOL([AR], [ar])
AM_MISSING_PROG(HELP2MAN, help2man)
# Whether dwz support -j.
diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
index 4cc49f2f8cd0..38d460ccabb6 100755
--- a/scripts/find-debuginfo.in
+++ b/scripts/find-debuginfo.in
@@ -5,6 +5,7 @@
# for inclusion in package file lists.
# Copyright (C) 2002-2021 rpm and debugedit contributors
+# Copyright (C) 2025 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -28,7 +29,7 @@ automagically generates debug info and file lists
Options:
[--strict-build-id] [-g] [-r] [-m] [-i] [-n] [-q] [-v]
[--keep-section SECTION] [--remove-section SECTION]
-[--g-libs]
+[--g-libs] [--no-ar-files]
[-j N] [--jobs N]
[-o debugfiles.list]
[-S debugsourcefiles.list]
@@ -95,6 +96,11 @@ will be called /usr/debug/src/<BASE>. This makes sure the debug source
dirs are unique between package version, release and achitecture (Use
--unique-debug-src-base "%{name}-%{VERSION}-%{RELEASE}.%{_arch}")
+If --no-ar-files is given, then static libraries will be ignored. Otherwise,
+they receive only with source-path rewriting and collection. They are
+not stripped, since they have no persistent build-ids to accommodate
+eventual reunification.
+
The -q or --quiet flag silences all non-error output from the script.
The -v or --verbose flag add more output for all files processed.
When neither -q or -v is given then only output for each pass is given.
@@ -115,6 +121,7 @@ PATH=${install_dir}:$PATH
READELF=${READELF:=@READELF@}
OBJCOPY=${OBJCOPY:=@OBJCOPY@}
NM=${NM:=@NM@}
+AR=${AR:=@AR@}
# With -g arg, pass it to strip on libraries or executables.
strip_g=false
@@ -167,6 +174,9 @@ quiet=false
# add more non-error output
verbose=false
+# process static libraries
+process_ar=true
+
BUILDDIR=.
out=debugfiles.list
srcout=
@@ -260,6 +270,9 @@ while [ $# -gt 0 ]; do
srcout=$2
shift
;;
+ --no-ar-files)
+ process_ar=false
+ ;;
-q|--quiet)
quiet=true
verbose=false
@@ -443,11 +456,19 @@ trap 'rm -rf "$temp"' EXIT
# Build a list of unstripped ELF files and their hardlinks
touch "$temp/primary"
touch "$temp/linked"
-find "$RPM_BUILD_ROOT" ! -path "${debugdir}/*.debug" -type f \
- \( -perm -0100 -or -perm -0010 -or -perm -0001 \) \
- -print | LC_ALL=C sort |
-file -N -f - | sed -n -e 's/^\(.*\):[ ]*.*ELF.*, not stripped.*/\1/p' |
-xargs --no-run-if-empty stat -c '%h %D_%i %n' |
+(
+ find "$RPM_BUILD_ROOT" ! -path "${debugdir}/*.debug" -type f \( -perm -0100 -or -perm -0010 -or -perm -0001 \) -print |
+ file -N -f - |
+ sed -n -e 's/^\(.*\):[ ]*.*ELF.*, not stripped.*/\1/p';
+
+ # plus static libraries
+ $process_ar && find "$RPM_BUILD_ROOT" -type f -name '*.a' -print |
+ file -N -f - |
+ sed -n -e 's/^\(.*\):[ ]*current ar archive.*/\1/p';
+ true
+) |
+ env LC_ALL=C sort |
+ xargs --no-run-if-empty stat -c '%h %D_%i %n' |
while read nlinks inum f; do
if [ $nlinks -gt 1 ]; then
var=seen_$inum
@@ -461,14 +482,123 @@ while read nlinks inum f; do
echo "$nlinks $inum $f" >>"$temp/primary"
done
+
+# Handle ELF archives
+do_ar_file()
+{
+ local nlinks="$1" inum="$2" f="$3" id link linked
+ local tmpa="$temp/$inum-output.a"
+ local res=0
+
+ # See also cpio SOURCEFILE copy. Directories must match up.
+ debug_base_name="$RPM_BUILD_DIR"
+ debug_dest_name="/usr/src/debug"
+ if [ ! -z "$unique_debug_src_base" ]; then
+ debug_base_name="$BUILDDIR"
+ debug_dest_name="/usr/src/debug/${unique_debug_src_base}"
+ fi
+
+ $verbose && echo "extracting debug info from $f"
+
+ # Extract members from archive, one at a time. There may be
+ # duplicate names, so we can't just extract the entire archive in
+ # one go (overwriting each other). Instead, we pick off member
+ # files one-by-one, into synthetic subdirectories, adding processed
+ # versions to a new archive, in order.
+
+ # Create empty output .a; mktemp would create a 0-byte file, which ar rv doesn't like
+ ${AR} r "$tmpa" # no members
+
+ ${AR} tvO "$f" | while read line; do
+ local pattern='^[rwx-]+ [0-9]+/[0-9]+ +([0-9]+) (.................) (.*) (0x[0-9a-f]+)$'
+ if [[ $line =~ $pattern ]]; then
+ local size=${BASH_REMATCH[1]}
+ local date=${BASH_REMATCH[2]}
+ local member=${BASH_REMATCH[3]}
+ local offset
+ (( offset=${BASH_REMATCH[4]} )) # convert from hexadecimal
+ $verbose && echo "considering ${f#$RPM_BUILD_DIR/} ${member} size ${size} at ${offset}"
+ local tmpdir="$temp/$inum-archive-member" # super short lived
+ local member_dn=$(dirname "$member")
+ if [ "$member_dn" = "." ]; then
+ member_dn="" # empty
+ else
+ member_dn="${member_dn}/" # or suffixed with /
+ fi
+ local member_bn=$(basename "$member")
+
+ # (re)create a directory to hold the (pathname-inclusive) member
+ mkdir -p "$tmpdir/$member_dn"
+
+ # extract the file by offset, because extracting it by name
+ # is hard, in case the same name exists multiple times. A
+ # distinct instance-number would have to be given to ar ("N ###"),
+ # kept on a per-name basis.
+ (cd "$tmpdir"; dd status=none if="$f" of="$member_dn$member_bn" bs=1 skip="$offset" count="$size")
+ if [ $? -ne 0 ]; then
+ res=1
+ fi
+ # preserve timestamp from original file, though debugedit may lose it
+ touch -d "$date" "$tmpdir/$member_dn$member_bn"
+
+ if file "$tmpdir/$member_dn$member_bn" |
+ grep -qE 'ELF.*, not stripped'; then
+ debugedit -b "$debug_base_name" -d "$debug_dest_name" \
+ -l "$SOURCEFILE" "$tmpdir/$member_dn$member_bn"
+ if [ $? -ne 0 ]; then
+ res=1
+ $verbose && echo "failed processing ELF object ${member}"
+ else
+ $verbose && echo "processed ELF object ${member}"
+ fi
+ else
+ $verbose && echo "skipped ${member}"
+ fi
+
+ # add the file; qP mode, so strict append, no dupe elimination, path preserved
+ (cd "$tmpdir"; ${AR} qP "$tmpa" "$member_dn$member_bn")
+
+ # remove the entire temporary directory, in case another
+ # member object comes later with a conflicting name
+ rm -rf "$tmpdir"
+ else
+ $verbose && echo "skipping archive $f with unparseable contents"
+ res=1
+ fi
+ done
+ if [ $res -eq 0 ]; then
+ # replace original archive with new version
+ ${AR} sP "$tmpa" # ranlib, preserve paths
+ mv "$tmpa" "$f"
+ fi
+ rm -f "$tmpa"
+
+ $verbose && echo found $(tr -dc '\0' < "$SOURCEFILE" | wc -c) source files
+
+ # NB: no need to strip or dwz-compress or gdbindex or
+ # ELFBINSFILE-collect objects / archives. These operations only
+ # make sense on the final binaries that the static archives are
+ # linked into.
+
+ return $res
+}
+
+
# Strip ELF binaries
do_file()
{
local nlinks=$1 inum=$2 f=$3 id link linked
+ # reject files already located under /usr/lib/debug, presumably processed
get_debugfn "$f"
[ -f "${debugfn}" ] && return 0
+ local ar_re="^.*\.a$"
+ if [[ $f =~ $ar_re ]]; then # treat as static archive
+ do_ar_file "$1" "$2" "$3"
+ return
+ fi
+
$verbose && echo "extracting debug info from $f"
# See also cpio SOURCEFILE copy. Directories must match up.
debug_base_name="$RPM_BUILD_DIR"
@@ -490,6 +620,8 @@ do_file()
$strict && return 2
fi
+ $verbose && echo found $(tr -dc '\0' < "$SOURCEFILE" | wc -c) source files
+
# debugedit makes sure to to get write permission to the file and
# restores original state after modifications. Other utilities
# might not.
@@ -835,3 +967,4 @@ if ((nout > 0)); then
fi
$quiet || echo "find-debuginfo: done" 2>&1
+exit 0
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b1d39f4ac1fc..b6c39a9b1b93 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -32,12 +32,14 @@ $(srcdir)/package.m4: $(top_srcdir)/configure.ac
# The tests
TESTSUITE_AT = \
testsuite.at \
- debugedit.at
+ debugedit.at \
+ find-debuginfo.at
# Some source files that are needed by the tests
EXTRA_DIST += data/SOURCES/foo.c \
data/SOURCES/bar.c \
data/SOURCES/baz.c \
+ data/SOURCES/dupe.c \
data/SOURCES/foobar.h
TESTSUITE = $(srcdir)/testsuite
diff --git a/tests/atlocal.in b/tests/atlocal.in
index 9da28786ba26..2f2ff4a80646 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -8,6 +8,7 @@ PATH=@abs_builddir@:@abs_top_builddir@:$top_srcdir:$srcdir:$PATH
CC="@CC@"
CFLAGS=""
LD="@LD@"
+AR="@AR@"
LDFLAGS=""
READELF="@READELF@"
READELF_VERSION_OK="@READELF_VERSION_OK@"
diff --git a/tests/data/SOURCES/dupe.c b/tests/data/SOURCES/dupe.c
new file mode 100644
index 000000000000..8491786395fb
--- /dev/null
+++ b/tests/data/SOURCES/dupe.c
@@ -0,0 +1,4 @@
+int FOO (void);
+int FOO (void) {
+ return 0;
+}
diff --git a/tests/find-debuginfo.at b/tests/find-debuginfo.at
new file mode 100644
index 000000000000..cd41c3c378c5
--- /dev/null
+++ b/tests/find-debuginfo.at
@@ -0,0 +1,81 @@
+# debugedit.at: Tests for the find-debuginfo tool
+#
+# Copyright (C) 2025 Red Hat Inc.
+#
+# This program 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 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 see <http://www.gnu.org/licenses/>.
+
+# Recommended usage for verbose logging to the console:
+# make check TESTSUITEFLAGS=-v
+
+AT_BANNER([find-debuginfo])
+AT_TESTED([find-debuginfo])
+
+m4_define([BUILDO],[[
+mkdir -p "$1"
+cp "${abs_srcdir}"/data/SOURCES/dupe.c "$1"
+$CC $CFLAGS -g3 -c "`pwd`/$1/dupe.c" -DFOO=$2 -o "$1/dupe.o"
+echo built "$1/dupe.o"
+]])
+
+
+AT_SETUP([find-debuginfo help])
+AT_KEYWORDS([find-debuginfo] [find-debuginfo])
+AT_CHECK([find-debuginfo --help],[0],[ignore],[ignore])
+AT_CLEANUP
+
+
+AT_SETUP([find-debuginfo on .a])
+# build a little .a
+export HOME=${PWD}
+BUILDO([subdir_foob],[foo])
+touch -t 197001010101 subdir_foob/dupe.o # set distinct timestamps
+BUILDO([subdir_barb],[bar])
+touch -t 197001020101 subdir_barb/dupe.o
+BUILDO([subdir_zo b],[jam])
+touch -t 197001030101 subdir_zo?b/dupe.o
+BUILDO([subdir_club],[pix])
+touch -t 197001040101 subdir_club/dupe.o
+mkdir -p subdir_build
+$AR rv subdir_build/dupes.a subdir_foob/dupe.o subdir_barb/dupe.o # without P, these duplicate members "/dupe.o"
+$AR qP subdir_build/dupes.a subdir_zo?b/dupe.o subdir_club/dupe.o # with P, these are subdirectoried members
+# NB: find-debugedit can't handle file names with embedded whitespace; find | xargs used here and there
+
+# list old .a contents
+AT_CHECK([$AR tPv subdir_build/dupes.a],[0],[stdout],[ignore])
+AT_CHECK([test `wc -l < stdout` -eq 4],[0],[ignore],[ignore])
+
+# expect original source tree refs
+AT_CHECK([$READELF --debug-dump=line subdir_build/dupes.a | grep `pwd`], [0], [stdout], [ignore])
+
+# (which is not everpresent)
+AT_CHECK([type -p find-debuginfo], [0], [stdout], [ignore])
+
+# run conversion
+AT_CHECK([[env RPM_BUILD_DIR=${PWD} RPM_BUILD_ROOT=${PWD} RPM_PACKAGE_NAME=pkg RPM_PACKAGE_VERSION=ver RPM_PACKAGE_RELEASE=rel RPM_ARCH=arch \
+ bash -x `type -p find-debuginfo` -S sourcefiles.list -v ${PWD}/subdir_build]], [0], [stdout], [ignore])
+
+# list new .a contents
+AT_CHECK([$AR tPv subdir_build/dupes.a],[0],[stdout],[ignore])
+AT_CHECK([test `wc -l < stdout` -eq 4],[0],[ignore],[ignore])
+
+# expect non-empty source file list
+AT_CHECK([grep . ${PWD}/subdir_build/sourcefiles.list | tr '\0' '\n'], [0], [stdout], [ignore])
+
+# expect proper source tree
+AT_CHECK([$READELF --debug-dump=line subdir_build/dupes.a | grep /usr/src/debug], [0], [stdout], [ignore])
+
+# no traces of old source tree
+AT_CHECK([$READELF --debug-dump=line subdir_build/dupes.a | grep `pwd`], [1], [stdout], [stderr])
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index baf7ca2c81c6..f26cfbe1c6f1 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -8,5 +8,5 @@ m4_define([AT_SKIP_TEST],[exit 77])
AT_INIT
AT_TESTED([debugedit])
-
m4_include([debugedit.at])
+m4_include([find-debuginfo.at])
More information about the Debugedit
mailing list