[Bug find-debuginfo/30505] New: unchecked "gdb-add-index" invocation

[Bug find-debuginfo/30505] New: unchecked "gdb-add-index" invocation

[lersek at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30505

            Bug ID: 30505
           Summary: unchecked "gdb-add-index" invocation
           Product: debugedit
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: find-debuginfo
          Assignee: unassigned at sourceware dot org
          Reporter: lersek at redhat dot com
                CC: debugedit at sourceware dot org
  Target Milestone: ---

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

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug find-debuginfo/30505] unchecked "gdb-add-index" invocation

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30505

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #1 from Mark Wielaard <mark at klomp dot org> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug find-debuginfo/30505] unchecked "gdb-add-index" invocation

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30505

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugzilla.redhat.com
                   |                            |/show_bug.cgi?id=1793746

-- 
You are receiving this mail because:
You are on the CC list for the bug.