Bug 30505 - unchecked "gdb-add-index" invocation
Summary: unchecked "gdb-add-index" invocation
Status: NEW
Alias: None
Product: debugedit
Classification: Unclassified
Component: find-debuginfo (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-01 11:04 UTC by Laszlo Ersek
Modified: 2023-12-10 22:16 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Ersek 2023-06-01 11:04:29 UTC
As of commit 5bade25a11b5, "scripts/find-debuginfo.in" contains:

>|   # Add .gdb_index if requested.
>|   if $include_gdb_index; then
>|     if type gdb-add-index >/dev/null 2>&1; then
>|       gdb-add-index "$f"
>|     else
>|       echo >&2 "*** ERROR: GDB index requested, but no gdb-add-index installed"
>|       exit 2
>|     fi
>|   fi

If the objcopy invocation underlying "gdb-add-index" fails, then
"gdb-add-index" propagates that issue through its exit status; however,
"find-debuginfo" ignores it (see above).

This allows RPM builds to succeed if debuginfo extraction is started
with "-i" (i.e., GDB index is being requested), gdb-add-index is
available, but gdb-add-index fails for some reason.

One example is a shared library object that contains Perl bindings for a
C-language library. Perl's ExtUtils::Install module installs artifacts
in read-only mode. On a read-only .so, objcopy fails, hence
gdb-add-index fails. Yet the RPM build succeeds, resulting in such
debuginfo (*.so.debug) files that lack the .gdb_index section. The
expected behavior is that the RPM build fail, similarly to how it
currently fails if "-i" is passed to "find-debuginfo", but
"gdb-add-index" is unavailable (see the"exit 2" statement in the quote).

Installing files in read-only mode is arguably a bug in
ExtUtils::Install itself
<https://rt.cpan.org/Public/Bug/Display.html?id=40976>; however, that
does not mean "find-debuginfo" should ignore "gdb-add-index" failures.

I figure something like the following might work:

>| diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
>| index 7dec3c3bfdc5..e0d931175cd9 100755
>| --- a/scripts/find-debuginfo.in
>| +++ b/scripts/find-debuginfo.in
>| @@ -470,6 +470,10 @@ do_file()
>|    if $include_gdb_index; then
>|      if type gdb-add-index >/dev/null 2>&1; then
>|        gdb-add-index "$f"
>| +      exit_status=$?
>| +      if test $exit_status -ne 0; then
>| +        exit $exit_status
>| +      fi
>|      else
>|        echo >&2 "*** ERROR: GDB index requested, but no gdb-add-index installed"
>|        exit 2

However, "find-debuginfo" seems to have an (undocumented) convention
where it uses only exit statuses 1 and 2. In that case, propagating the
exit status from gdb-add-index may not be right. Then one alternative
would be:

>| diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
>| index 7dec3c3bfdc5..99cb3b906a00 100755
>| --- a/scripts/find-debuginfo.in
>| +++ b/scripts/find-debuginfo.in
>| @@ -469,7 +469,7 @@ do_file()
>|    # Add .gdb_index if requested.
>|    if $include_gdb_index; then
>|      if type gdb-add-index >/dev/null 2>&1; then
>| -      gdb-add-index "$f"
>| +      gdb-add-index "$f" || exit 1
>|      else
>|        echo >&2 "*** ERROR: GDB index requested, but no gdb-add-index installed"
>|        exit 2

Note that the problem (not checking the exit status of gdb-add-index)
dates back to the original introduction of gdb-add-index to
find-debuginfo:

  https://github.com/rpm-software-management/rpm/commit/9570a7f6af15
  https://github.com/rpm-software-management/rpm/commit/41c4dcf507e2
  https://github.com/rpm-software-management/rpm/commit/67d3df338875
  https://github.com/rpm-software-management/rpm/commit/04b0805a756c
Comment 1 Mark Wielaard 2023-06-01 11:27:04 UTC
Thanks for reporting this (and providing different patches to fix it).

I think originally we ignored errors like these (same for add_minidebug) because these are "just nice to have extras". But given that people explicitly opt in to these "extras" (or at least not opt-out) we should probably do as you suggest and explicitly report errors.