[PATCH 1/2] Add file name to "Loadable section ... outside of ELF segments" warning

[PATCH 1/2] Add file name to "Loadable section ... outside of ELF segments" warning

[Keith Seitz via Gdb-patches]

As requested in gdb/23034, I would like to extend the warning message
GDB displays when it detects a loadable section that is outside any
ELF segment.

Before:

$ gdb -q --ex "b systemctl_main" -ex "r" -batch --args systemctl kexec
Breakpoint 1 at 0xc24d: file ../src/systemctl/systemctl.c, line 8752.
warning: Loadable section ".note.gnu.property" outside of ELF segments
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
warning: Loadable section ".note.gnu.property" outside of ELF segments
warning: Loadable section ".note.gnu.property" outside of ELF segments
warning: Loadable section ".note.gnu.property" outside of ELF segments
warning: Loadable section ".note.gnu.property" outside of ELF segments
[snip]

Breakpoint 1, systemctl_main (argv=0x7fffffffd348, argc=2) at ../src/systemctl/systemctl.c:8752
8752	                r = systemctl_main(argc, argv);

After:

warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libcap.so.2
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libacl.so.1
[snip]

I think this is eminently more useful output.

gdb/ChangeLog
2020-11-12  Keith Seitz  <keiths@redhat.com>

       PR gdb/23034
       * elfread.c (elf_symfile_segments): Output a BFD file name
       for the "Loadable section ... outside of ELF segments" warning.
---
 gdb/ChangeLog | 6 ++++++
 gdb/elfread.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index f362bc8a62..7915dcdcce 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -149,8 +149,8 @@ elf_symfile_segments (bfd *abfd)
 	 binaries are not relocatable.  */
       if (bfd_section_size (sect) > 0 && j == num_segments
 	  && (bfd_section_flags (sect) & SEC_LOAD) != 0)
-	warning (_("Loadable section \"%s\" outside of ELF segments"),
-		 bfd_section_name (sect));
+	warning (_("Loadable section \"%s\" outside of ELF segments\n  in %s"),
+		 bfd_section_name (sect), bfd_get_filename (abfd));
     }
 
   return data;
-- 
2.26.2

[PATCH 2/2] Exclude debuginfo files from "outside of ELF segments" warning

[Keith Seitz via Gdb-patches]

When GDB loads an ELF file, it will warn when a section is not located
in an ELF segment:

$ ./gdb -q -iex "set build-id-verbose 0" --ex "b systemctl_main" -ex "r" -batch --args systemctl kexec
Breakpoint 1 at 0xc24d: file ../src/systemctl/systemctl.c, line 8752.
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libgcc_s.so.1
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libcap.so.2
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libacl.so.1
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libcryptsetup.so.12
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libgcrypt.so.20
warning: Loadable section ".note.gnu.property" outside of ELF segments
  in .gnu_debugdata for /lib64/libip4tc.so.2
[snip]

This has feature has also been reported by various users, most notably
the Fedora-EOL'd bug 1553086.

Mark Wielaard explains the issue quite nicely in

   https://sourceware.org/bugzilla/show_bug.cgi?id=24717#c2

The short of it is, the ELF program headers for debuginfo files are
not suited to this particular use case. Consequently, the warning
generated above really is useless and should be ignored.

This patch follows the same heuristic that BFD itself uses.

gdb/ChangeLog
2020-11-12  Keith Seitz  <keiths@redhat.com>

       https://bugzilla.redhat.com/show_bug.cgi?id=1553086
       * elfread.c (elf_symfile_segments): Omit "Loadable section ...
       outside of ELF segments" warning for debugin
---
 gdb/ChangeLog | 6 ++++++
 gdb/elfread.c | 7 ++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 7915dcdcce..ccd7ce50d5 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -147,7 +147,12 @@ elf_symfile_segments (bfd *abfd)
 	 RealView) use SHT_NOBITS for uninitialized data.  Since it is
 	 uninitialized, it doesn't need a program header.  Such
 	 binaries are not relocatable.  */
-      if (bfd_section_size (sect) > 0 && j == num_segments
+
+      /* Exclude debuginfo files from this warning, too, since those
+	 are often not strictly compliant with the standard. See, e.g.,
+	 ld/24717 for more discussion.  */
+      if (!is_debuginfo_file (abfd)
+	  && bfd_section_size (sect) > 0 && j == num_segments
 	  && (bfd_section_flags (sect) & SEC_LOAD) != 0)
 	warning (_("Loadable section \"%s\" outside of ELF segments\n  in %s"),
 		 bfd_section_name (sect), bfd_get_filename (abfd));
-- 
2.26.2

Re: [PATCH 1/2] Add file name to "Loadable section ... outside of ELF segments" warning

[Tom Tromey]

Keith> gdb/ChangeLog
Keith> 2020-11-12  Keith Seitz  <keiths@redhat.com>

Keith>        PR gdb/23034
Keith>        * elfread.c (elf_symfile_segments): Output a BFD file name
Keith>        for the "Loadable section ... outside of ELF segments" warning.

Thank you.  This seems like a nice improvement, please check it in.

Tom

Re: [PATCH 2/2] Exclude debuginfo files from "outside of ELF segments" warning

[Tom Tromey]

>>>>> "Keith" == Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:

Keith> The short of it is, the ELF program headers for debuginfo files are
Keith> not suited to this particular use case. Consequently, the warning
Keith> generated above really is useless and should be ignored.

Keith> +      /* Exclude debuginfo files from this warning, too, since those
Keith> +	 are often not strictly compliant with the standard. See, e.g.,
Keith> +	 ld/24717 for more discussion.  */
Keith> +      if (!is_debuginfo_file (abfd)
Keith> +	  && bfd_section_size (sect) > 0 && j == num_segments

I didn't know about is_debuginfo_file, so I took a quick look, and it
says:

/* Determine if a bfd is a debuginfo file.  Unfortunately there
   is no defined method for detecting such files, so we have to
   use heuristics instead.  */

I wonder if gdb should just use its own idea here, though -- like if the
objfile has a backlink, it is a separate debug file.

I'm ok with this patch though.

thanks,
Tom

Re: [PATCH 2/2] Exclude debuginfo files from "outside of ELF segments" warning

[Keith Seitz via Gdb-patches]

On 11/12/20 1:45 PM, Tom Tromey wrote:
> I didn't know about is_debuginfo_file, so I took a quick look, and it
> says:
> 
> /* Determine if a bfd is a debuginfo file.  Unfortunately there
>    is no defined method for detecting such files, so we have to
>    use heuristics instead.  */
> 
> I wonder if gdb should just use its own idea here, though -- like if the
> objfile has a backlink, it is a separate debug file.

Yeah, I debated that, too, but ultimately, I thought it best to do what
BFD does. Camaraderie? :-)

> I'm ok with this patch though.

I'll wait a day or two and see if anyone else chimes in.

I appreciate the speedy review!

Keith

Re: [PATCH 2/2] Exclude debuginfo files from "outside of ELF segments" warning

[Tom Tromey]

>> I wonder if gdb should just use its own idea here, though -- like if the
>> objfile has a backlink, it is a separate debug file.

Keith> Yeah, I debated that, too, but ultimately, I thought it best to do what
Keith> BFD does. Camaraderie? :-)

Makes sense to me.  You can check it in.

thanks,
Tom

Re: [PATCH 2/2] Exclude debuginfo files from "outside of ELF segments" warning

[Keith Seitz via Gdb-patches]

On 11/13/20 5:50 AM, Tom Tromey wrote:
>>> I wonder if gdb should just use its own idea here, though -- like if the
>>> objfile has a backlink, it is a separate debug file.
> 
> Keith> Yeah, I debated that, too, but ultimately, I thought it best to do what
> Keith> BFD does. Camaraderie? :-)
> 
> Makes sense to me.  You can check it in.
> 

I have pushed both patches. Thank you for taking a look.

Keith