From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 5 Nov 2013 12:01:45 +0000 (-0500)
Subject: crash while re-reading symbols from objfile on ppc-aix.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=846060dfd8ec2bf0e78b4049a89b51438bfe0072;p=binutils-gdb.git

crash while re-reading symbols from objfile on ppc-aix.

This patch aims at fixing the following problem, where the user:

  . debugs its program
  . makes a modification and rebuilds it *without exiting the debugger*
  . returns to its debugging session and restarts the inferior

In that situation, the debugger notices that the underlying executable
has changed and that re-reading its symbols is needed. Shortly after
displaying a message informing the user of the situation, GDB crashes:

   (gdb) run
   [...]
   `/[...]/dest' has changed; re-reading symbols.
   zsh: 13434922 segmentation fault (core dumped)

The crash occurs while trying to allocate some memory on the bfd_bfd
obstack.  But, at some point in time, the whole obstack data gets
corrupted, nullified. So the memory allocation fails trying to call
a function at a NULL address. (side note: when debugging GDB in GDB,
top-gdb reports a SIGILL, while the shell makes it look like it was
a SIGSEGV - the discrepancy is not critical to the investigation
and therefore was not explored)

The corruption occurred because the region where the per_bfd data
got free'ed nearly after it got allocated! This is what happens,
in chronological order (see reread_symbols):

  1. GDB notices that the executable has changed, decides to
     re-read its symbols.

  2. Opens a new bfd, unrefs the old one

  3. Calls set_objfile_per_bfd (objfile);

  4. Re-initializes the objfile's obstack:
     obstack_init (&objfile->objfile_obstack);

I think that the normal behavior for set_objfile_per_bfd would
be to search for already-allocated shared per_bfd data, and
allocate new one if not found.  The critical difference between
a platform such as x86_64-linuxe where it works, and ppc-aix,
where it doesn't lies in the fact that bfd-data sharing is not
activated on ppc-aix, and as a result, the per-bfd data gets
allocated on the objfile's obstack instead of in the bfd objalloc:

      /* If the object requires gdb to do relocations, we simply fall
         back to not sharing data across users.  These cases are rare
         enough that this seems reasonable.  */
      if (abfd != NULL && !gdb_bfd_requires_relocations (abfd))
        {
          storage = bfd_zalloc (abfd, sizeof (struct objfile_per_bfd_storage));
          set_bfd_data (abfd, objfiles_bfd_data, storage);
        }
      else
        storage = OBSTACK_ZALLOC (&objfile->objfile_obstack,
                                  struct objfile_per_bfd_storage);

Allocating that per_bfd storage is of course nearly useless since
we end up free-ing right after in step (4) above. Eventually,
the memory region ends up being re-used, hence the corruption
leading to the crash.

This fix was simply to move the call to set_objfile_per_bfd after
the objfile's obstack re-initialization.

gdb/ChangeLog:

        * symfile.c (reread_symbols): Move call to set_objfile_per_bfd
        after re-initialization of OBJFILE's obstack.
---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dfd80c030a8..279e3e200f3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2013-11-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* symfile.c (reread_symbols): Move call to set_objfile_per_bfd
+	after re-initialization of OBJFILE's obstack.
+
 2013-11-12  Doug Evans  <dje@sebabeach.org>
 
 	* breakpoint.c (bpstat_check_breakpoint_conditions): Assert
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 13071897a6f..61e3e44e713 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2591,13 +2591,17 @@ reread_symbols (void)
 	  memset (&objfile->msymbol_demangled_hash, 0,
 		  sizeof (objfile->msymbol_demangled_hash));
 
-	  set_objfile_per_bfd (objfile);
-
 	  /* obstack_init also initializes the obstack so it is
 	     empty.  We could use obstack_specify_allocation but
 	     gdb_obstack.h specifies the alloc/dealloc functions.  */
 	  obstack_init (&objfile->objfile_obstack);
 
+	  /* set_objfile_per_bfd potentially allocates the per-bfd
+	     data on the objfile's obstack (if sharing data across
+	     multiple users is not possible), so it's important to
+	     do it *after* the obstack has been initialized.  */
+	  set_objfile_per_bfd (objfile);
+
 	  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack,
 						  original_name,
 						  strlen (original_name));