Re: [PATCH] gdb, configure: Add disable-formats option for configure

[Andrew Burgess <aburgess@redhat.com>]

Guinevere Larsen <guinevere@redhat.com> writes:

> I managed to reproduce a crash on Linux with a similar setup, but my 
> backtrace looks a little different. In my version, GDB is trying to 
> setup solib hooks as a "post start inferior" step, whereas on yours it 
> seems that GDB is trying to print the location at which the inferior is 
> stopped.
>
> In both cases, though, what happens is that we're calling 
> find_pc_section, which will try to create a map of sections for quick 
> lookup, but since we don't understand the file format, GDB thinks we 
> have no sections (and at least in the linux case, the pointer to the 
> sections is null). so the issue is not that the offset is -1, but rather 
> that this->the_bfd_section is 0x0.
>
> If I'm correct that this is the cause of the crash on windows as well as 
> linux, I think this should fix the crash. Could you sanity check it for me?
>
>
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 0e076fe36be..cc8bb253d11 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -1053,6 +1053,11 @@ update_section_map (struct program_space *pspace,
>     gdb_assert (pspace_info->section_map_dirty != 0
>                || pspace_info->new_objfiles_available != 0);
>
> +  /* If there are 0 sections, or the point to the sections is null, it 
> makes
> +     no sense to try and have a section map at all. */
> +  if (pspace_info->num_sections == 0 || pspace_info->sections == nullptr)
> + return;
> +
>     map = *pmap;
>     xfree (map);
>

This does fix the crash in my setup (Windows gdbserver with Linux GDB
configured to only support ELF), but I'm not sure this is the approach
that I would take.

While it is true that this change allows GDB to handle an objfile that
is can't otherwise understand, I wonder, how useful is it to even keep
the objfile around at all?

GDB realises that it can't handle the objfile in find_sym_fns, this is
where the "I'm sorry, Dave, I can't do that" message comes from.  The
backtrace at this point is:

  #0  find_sym_fns (abfd=0x21b4fa0) at ../../src/gdb/symfile.c:1808
  #1  0x0000000000c13002 in syms_from_objfile_1 (objfile=0x21d3bf0, addrs=0x0, add_flags=...) at ../../src/gdb/symfile.c:916
  #2  0x0000000000c1330c in syms_from_objfile (objfile=0x21d3bf0, addrs=0x0, add_flags=...) at ../../src/gdb/symfile.c:998
  #3  0x0000000000c13838 in symbol_file_add_with_addrs (abfd=..., name=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", add_flags=..., addrs=0x0, flags=..., parent=0x0) at ../../src/gdb/symfile.c:1104
  #4  0x0000000000c13bd1 in symbol_file_add_from_bfd (abfd=..., name=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", add_flags=..., addrs=0x0, flags=..., parent=0x0) at ../../src/gdb/symfile.c:1180
  #5  0x0000000000c13c20 in symbol_file_add (name=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", add_flags=..., addrs=0x0, flags=...) at ../../src/gdb/symfile.c:1193
  #6  0x0000000000c13ce6 in symbol_file_add_main_1 (args=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", add_flags=..., flags=..., reloff=0x0) at ../../src/gdb/symfile.c:1217
  #7  0x0000000000c13c8d in symbol_file_add_main (args=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", add_flags=...) at ../../src/gdb/symfile.c:1208
  #8  0x000000000081cfa3 in try_open_exec_file (exec_file_host=0x21aeab0 "target:C:/msys64/home/andrew/segv.exe", inf=0x1a1e9f0, add_flags=...) at ../../src/gdb/exec.c:202
  #9  0x000000000081d863 in exec_file_locate_attach (pid=42000, defer_bp_reset=0, from_tty=1) at ../../src/gdb/exec.c:344

The exception thrown in find_sym_fns is not caught until
try_open_exec_file.

The objfile itself is created in symbol_file_add_with_addrs, and
interestingly, if we checkout the end of this function we'll find this
line:

  gdb::observers::new_objfile.notify (objfile);

which is going to be completely skipped if we throw an exception as we
do in this case.

I'm tempted to suggest that what we should consider is removing the
objfile if it turns out that GDB doesn't understand it.  Attached at the
end of this email is a **very** rough patch which does this.  After
creating the objfile we immediately wrap it in an struct, now if we
throw an exception then the objfile is auto-removed from the program
space (and deleted).  Only if we reach the end of
symbol_file_add_with_addrs do we call a method on the local variable to
mark the objfile as successfully created.

I'd be tempted to convert the `if` checks you added into `gdb_assert`
calls, at least initially.  I'm not sure if it's possible to have a
"valid" objfile that we understand that would otherwise trigger that
`if`, hence asserts for now.

The patch below is really **very** rough.  I'm tempted to think that
objfile::make should return the object which auto-removes the objfile,
that way anyone who creates an objfile is forced to think about this
pattern of creation, validation, accepting...

Anyway, would be interested to hear your thoughts.

Thanks,
Andrew

---

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 1502fdbe500..17a7be6093b 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -854,6 +854,33 @@ init_entry_point_info (struct objfile *objfile)
     }
 }
 
+struct scoped_objfile_remover
+{
+  explicit scoped_objfile_remover (struct objfile *objfile)
+    : m_objfile (objfile)
+  {
+    /* Nothing.  */
+  }
+
+  ~scoped_objfile_remover ()
+  {
+    if (m_objfile != nullptr)
+      {
+	fprintf (stderr, "APB: Removing the objfile, something went wrong\n");
+	m_objfile->unlink ();
+      }
+  }
+
+  void release_objfile ()
+  {
+    m_objfile = nullptr;
+  }
+
+private:
+
+  struct objfile *m_objfile;
+};
+
 /* Process a symbol file, as either the main file or as a dynamically
    loaded file.
 
@@ -1061,6 +1088,7 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
 
   objfile *objfile
     = objfile::make (abfd, current_program_space, name, flags, parent);
+  scoped_objfile_remover remove_objfile_on_error (objfile);
 
   /* We either created a new mapped symbol table, mapped an existing
      symbol table file which has not had initial symbol reading
@@ -1112,6 +1140,9 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
   if (objfile->sf != nullptr)
     finish_new_objfile (objfile, add_flags);
 
+  /* No errors.  The objfile is officially added.  */
+  remove_objfile_on_error.release_objfile ();
+
   gdb::observers::new_objfile.notify (objfile);
 
   return objfile;