[PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983

Pedro Alves palves@redhat.com
Tue Jan 7 19:54:00 GMT 2014


On 01/07/2014 05:40 PM, Tom Tromey wrote:

> If you don't mind I think it would be good -- and easy -- to also
> implement Doug's suggestion, say a "bfd_set_filename" macro.

Sure thing, though in Doug's suggestion that would be used
to free the previous name IIRC, which now won't be necessary.
We can still use it to add some documentation though.
Done.

> Pedro> +/* A wrapper for bfd_strdup that never returns NULL.  */
> Pedro> +
> Pedro> +char *gdb_bfd_strdup (bfd *abfd, const char *str);
> 
> This can be marked ATTRIBUTE_RETURNS_NONNULL.

Indeed.  Below's the updated patch.

---------
Subject: [PATCH] Switch back to allocating the bfd's filename in the bfd's
 memory.

Don't use xstrdup in bfd, as it aborts on error.

bfd/
2014-01-07  Pedro Alves  <palves@redhat.com>

	PR binutils/11983
	* bfd-in.h (bfd_set_filename): New macro.
	* bfd-in2.h: Regenerate.
	* archive.c (_bfd_get_elt_at_filepos): Don't xstrdup the filename.
	Use bfd_set_filename.  On error, set the bfd's filename to a const
	string.
	* elfcode.h: Don't include libiberty.h.
	(bfd_from_remote_memory): Don't xstrdup the bfd's filename.  Use
	bfd_set_filename.
	* ieee.c: Don't include libiberty.h.
	(ieee_object_p): Don't xstrdup the bfd's filename.  Use
	bfd_set_filename.
	* mach-o.c (bfd_mach_o_fat_member_init): Don't xstrdup the
	architecture's printable name, it's const.  If allocating a
	filename, allocate it on the bfd's memory.  Use bfd_set_filename.
	* oasys.c: Don't include libiberty.h.
	(oasys_openr_next_archived_file): Don't xstrdup the bfd's
	filename.  Use bfd_set_filename.
	* opncls.c (_bfd_delete_bfd): Don't free the bfd's filename.
	(bfd_fopen, bfd_openstreamr, bfd_openr_iovec, bfd_openw)
	(bfd_create): Use bfd_strdup to copy the filename and handle
	memory error.  Use bfd_set_filename.
	(bfd_strdup): New function.
	* syms.c (_bfd_stab_section_find_nearest_line): Delete comment.
	* vms-lib.c: Don't include libiberty.h.
	(_bfd_vms_lib_get_module): Don't xstrdup the bfd's filename.  Use
	bfd_set_filename.

gdb/
2014-01-07  Pedro Alves  <palves@redhat.com>

	PR binutils/11983
	* gdb_bfd.c (gdb_bfd_strdup): New function.
	* gdb_bfd.h (gdb_bfd_strdup): Declare.
	* solib-aix.c (solib_aix_bfd_open): Don't free the bfd's previous
	filename.  Use gdb_bfd_strdup instead of xstrdup.  Use
	bfd_set_filename.
	* solib-spu.c (spu_bfd_open): Likewise.
	* spu-linux-nat.c (spu_bfd_open): Likewise.
	* symfile-mem.c (symbol_file_add_from_memory): Don't free the
	bfd's previous filename.  If passed a filename, dup it into bfd's
	memory.  If not, set the bfd's filename to a const read only
	string.  Use bfd_set_filename.
	(add_vsyscall_page): Free filename.
	* solib-darwin.c (darwin_bfd_open): Don't free the bfd's previous
	filename.  Use gdb_bfd_strdup instead of xstrdup.  Use
	bfd_set_filename.
---
 bfd/archive.c       |  3 ++-
 bfd/bfd-in.h        |  6 ++++++
 bfd/bfd-in2.h       |  9 ++++++++-
 bfd/elfcode.h       |  3 +--
 bfd/ieee.c          |  3 +--
 bfd/mach-o.c        |  6 +++---
 bfd/oasys.c         |  3 +--
 bfd/opncls.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
 bfd/syms.c          |  3 ---
 bfd/vms-lib.c       |  3 +--
 gdb/gdb_bfd.c       | 14 +++++++++++++
 gdb/gdb_bfd.h       |  5 +++++
 gdb/solib-aix.c     |  3 +--
 gdb/solib-darwin.c  |  3 +--
 gdb/solib-spu.c     |  3 +--
 gdb/spu-linux-nat.c |  3 +--
 gdb/symfile-mem.c   |  6 +++---
 17 files changed, 100 insertions(+), 34 deletions(-)

diff --git a/bfd/archive.c b/bfd/archive.c
index dc39751..039d8cb 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -705,7 +705,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
   else
     {
       n_nfd->origin = n_nfd->proxy_origin;
-      n_nfd->filename = xstrdup (filename);
+      bfd_set_filename (n_nfd, filename);
     }
 
   n_nfd->arelt_data = new_areldata;
@@ -718,6 +718,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
 
   free (new_areldata);
   n_nfd->arelt_data = NULL;
+  bfd_set_filename (n_nfd, "<out of memory>");
   return NULL;
 }
 
diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 3afd71b..ca372f6 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -519,6 +519,12 @@ extern void warn_deprecated (const char *, const char *, int, const char *);
 
 #define bfd_set_cacheable(abfd,bool) (((abfd)->cacheable = bool), TRUE)
 
+/* Set the BFD's filename to point to NAME.  NAME should have at least
+   the same lifetime as the BFD itself.  Most frequently the caller
+   allocates it in the BFD's memory or passes in a constant string.
+   Returns NAME.  */
+#define bfd_set_filename(abfd, name) ((abfd)->filename = (name))
+
 extern bfd_boolean bfd_cache_close
   (bfd *abfd);
 /* NB: This declaration should match the autogenerated one in libbfd.h.  */
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 71996db..c11e29b 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -526,6 +526,11 @@ extern void warn_deprecated (const char *, const char *, int, const char *);
 
 #define bfd_set_cacheable(abfd,bool) (((abfd)->cacheable = bool), TRUE)
 
+/* Set the BFD's filename to NAME.  NAME should have at least the same
+   lifetime as the BFD itself.  Usually the caller allocates it in the
+   BFD's memory.  Returns NAME.  */
+#define bfd_set_filename(abfd, name) ((abfd)->filename = (name))
+
 extern bfd_boolean bfd_cache_close
   (bfd *abfd);
 /* NB: This declaration should match the autogenerated one in libbfd.h.  */
@@ -1029,7 +1034,7 @@ bfd *bfd_openr (const char *filename, const char *target);
 
 bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
 
-bfd *bfd_openstreamr (const char *, const char *, void *);
+bfd *bfd_openstreamr (const char * filename, const char * target, void * stream);
 
 bfd *bfd_openr_iovec (const char *filename, const char *target,
     void *(*open_func) (struct bfd *nbfd,
@@ -1060,6 +1065,8 @@ bfd_boolean bfd_make_readable (bfd *abfd);
 
 void *bfd_alloc (bfd *abfd, bfd_size_type wanted);
 
+char *bfd_strdup (bfd *abfd, const char *str);
+
 void *bfd_zalloc (bfd *abfd, bfd_size_type wanted);
 
 unsigned long bfd_calc_gnu_debuglink_crc32
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 0328748..728faf5 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -71,7 +71,6 @@
 #include "bfdlink.h"
 #include "libbfd.h"
 #include "elf-bfd.h"
-#include "libiberty.h"
 
 /* Renaming structures, typedefs, macros and functions to be size-specific.  */
 #define Elf_External_Ehdr	NAME(Elf,External_Ehdr)
@@ -1803,7 +1802,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       bfd_set_error (bfd_error_no_memory);
       return NULL;
     }
-  nbfd->filename = xstrdup ("<in-memory>");
+  bfd_set_filename (nbfd, "<in-memory>");
   nbfd->xvec = templ->xvec;
   bim->size = contents_size;
   bim->buffer = contents;
diff --git a/bfd/ieee.c b/bfd/ieee.c
index e1734ec..132b287 100644
--- a/bfd/ieee.c
+++ b/bfd/ieee.c
@@ -33,7 +33,6 @@
 #include "ieee.h"
 #include "libieee.h"
 #include "safe-ctype.h"
-#include "libiberty.h"
 
 struct output_buffer_struct
 {
@@ -1823,7 +1822,7 @@ ieee_object_p (bfd *abfd)
     goto got_wrong_format;
   ieee->mb.module_name = read_id (&(ieee->h));
   if (abfd->filename == (const char *) NULL)
-    abfd->filename = xstrdup (ieee->mb.module_name);
+    bfd_set_filename (abfd, ieee->mb.module_name);
 
   /* Determine the architecture and machine type of the object file.  */
   {
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 6640a6a..0364a40 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -4353,16 +4353,16 @@ bfd_mach_o_fat_member_init (bfd *abfd,
   if (ap)
     {
       /* Use the architecture name if known.  */
-      abfd->filename = xstrdup (ap->printable_name);
+      bfd_set_filename (abfd, ap->printable_name);
     }
   else
     {
       /* Forge a uniq id.  */
       const size_t namelen = 2 + 8 + 1 + 2 + 8 + 1;
-      char *name = xmalloc (namelen);
+      char *name = bfd_alloc (abfd, namelen);
       snprintf (name, namelen, "0x%lx-0x%lx",
                 entry->cputype, entry->cpusubtype);
-      abfd->filename = name;
+      bfd_set_filename (abfd, name);
     }
 
   areltdata = bfd_zmalloc (sizeof (struct areltdata));
diff --git a/bfd/oasys.c b/bfd/oasys.c
index b8e457e..b96187f 100644
--- a/bfd/oasys.c
+++ b/bfd/oasys.c
@@ -26,7 +26,6 @@
 #include "libbfd.h"
 #include "oasys.h"
 #include "liboasys.h"
-#include "libiberty.h"
 
 /* Read in all the section data and relocation stuff too.  */
 
@@ -1117,7 +1116,7 @@ oasys_openr_next_archived_file (bfd *arch, bfd *prev)
 	{
 	  p->abfd = _bfd_create_empty_archive_element_shell (arch);
 	  p->abfd->origin = p->pos;
-	  p->abfd->filename = xstrdup (p->name);
+	  bfd_set_filename (p->abfd, p->name);
 
 	  /* Fixup a pointer to this element for the member.  */
 	  p->abfd->arelt_data = (void *) p;
diff --git a/bfd/opncls.c b/bfd/opncls.c
index 54744ce..7466921 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -123,8 +123,6 @@ _bfd_delete_bfd (bfd *abfd)
       objalloc_free ((struct objalloc *) abfd->memory);
     }
 
-  if (abfd->filename)
-    free ((char *) abfd->filename);
   free (abfd->arelt_data);
   free (abfd);
 }
@@ -228,7 +226,11 @@ bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
 
   /* PR 11983: Do not cache the original filename, but
      rather make a copy - the original might go away.  */
-  nbfd->filename = xstrdup (filename);
+  if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
 
   /* Figure out whether the user is opening the file for reading,
      writing, or both, by looking at the MODE argument.  */
@@ -395,7 +397,12 @@ bfd_openstreamr (const char *filename, const char *target, void *streamarg)
   nbfd->iostream = stream;
   /* PR 11983: Do not cache the original filename, but
      rather make a copy - the original might go away.  */
-  nbfd->filename = xstrdup (filename);
+  if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = read_direction;
 
   if (! bfd_cache_init (nbfd))
@@ -589,7 +596,11 @@ bfd_openr_iovec (const char *filename, const char *target,
 
   /* PR 11983: Do not cache the original filename, but
      rather make a copy - the original might go away.  */
-  nbfd->filename = xstrdup (filename);
+  if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
   nbfd->direction = read_direction;
 
   /* `open_p (...)' would get expanded by an the open(2) syscall macro.  */
@@ -656,7 +667,11 @@ bfd_openw (const char *filename, const char *target)
 
   /* PR 11983: Do not cache the original filename, but
      rather make a copy - the original might go away.  */
-  nbfd->filename = xstrdup (filename);
+  if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
   nbfd->direction = write_direction;
 
   if (bfd_open_file (nbfd) == NULL)
@@ -808,7 +823,11 @@ bfd_create (const char *filename, bfd *templ)
     return NULL;
   /* PR 11983: Do not cache the original filename, but
      rather make a copy - the original might go away.  */
-  nbfd->filename = xstrdup (filename);
+  if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
   if (templ)
     nbfd->xvec = templ->xvec;
   nbfd->direction = no_direction;
@@ -991,6 +1010,31 @@ bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size)
 
 /*
 FUNCTION
+	bfd_strdup
+
+SYNOPSIS
+	char *bfd_strdup (bfd *abfd, const char *str);
+
+DESCRIPTION
+        Copy a string into memory attached to <<abfd>> and return a
+        pointer to it.
+*/
+
+char *
+bfd_strdup (bfd *abfd, const char *str)
+{
+  bfd_size_type size;
+  char *p;
+
+  size = strlen (str) + 1;
+  p = bfd_alloc (abfd, size);
+  if (p != NULL)
+    memcpy (p, str, size);
+  return p;
+}
+
+/*
+FUNCTION
 	bfd_zalloc
 
 SYNOPSIS
diff --git a/bfd/syms.c b/bfd/syms.c
index 27b40eb..aa4718f 100644
--- a/bfd/syms.c
+++ b/bfd/syms.c
@@ -1383,9 +1383,6 @@ _bfd_stab_section_find_nearest_line (bfd *abfd,
 	{
 	  size_t len;
 
-	  /* Don't free info->filename here.  objdump and other
-	     apps keep a copy of a previously returned file name
-	     pointer.  */
 	  len = strlen (file_name) + 1;
 	  info->filename = (char *) bfd_alloc (abfd, dirlen + len);
 	  if (info->filename == NULL)
diff --git a/bfd/vms-lib.c b/bfd/vms-lib.c
index 407c186..709c457 100644
--- a/bfd/vms-lib.c
+++ b/bfd/vms-lib.c
@@ -25,7 +25,6 @@
 #include "libbfd.h"
 #include "safe-ctype.h"
 #include "bfdver.h"
-#include "libiberty.h"
 #include "vms.h"
 #include "vms/lbr.h"
 #include "vms/dcx.h"
@@ -1377,7 +1376,7 @@ _bfd_vms_lib_get_module (bfd *abfd, unsigned int modidx)
     default:
       break;
     }
-  res->filename = xstrdup (name);
+  bfd_set_filename (res, name);
 
   tdata->cache[modidx] = res;
 
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 5230d21..9d84815 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -634,6 +634,20 @@ gdb_bfd_requires_relocations (bfd *abfd)
   return gdata->needs_relocations;
 }
 
+/* See gdb_bfd.h.  */
+
+char *
+gdb_bfd_strdup (bfd *abfd, const char *str)
+{
+  char *p;
+
+  p = bfd_strdup (abfd, str);
+  if (p == NULL)
+    malloc_failure (0);
+
+  return p;
+}
+
 

 
 /* A callback for htab_traverse that prints a single BFD.  */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index d415005..8bcdf9a 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -70,6 +70,11 @@ const gdb_byte *gdb_bfd_map_section (asection *section, bfd_size_type *size);
 
 int gdb_bfd_crc (struct bfd *abfd, unsigned long *crc_out);
 
+/* A wrapper for bfd_strdup that never returns NULL.  */
+
+char *gdb_bfd_strdup (bfd *abfd, const char *str)
+  ATTRIBUTE_RETURNS_NONNULL;
+
 

 
 /* A wrapper for bfd_fopen that initializes the gdb-specific reference
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index fefa51f..f01c307 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -728,8 +728,7 @@ solib_aix_bfd_open (char *pathname)
      to allow commands listing all shared libraries to display that
      synthetic name.  Otherwise, we would only be displaying the name
      of the archive member object.  */
-  xfree (bfd_get_filename (object_bfd));
-  object_bfd->filename = xstrdup (pathname);
+  bfd_set_filename (object_bfd, gdb_bfd_strdup (object_bfd, pathname));
 
   gdb_bfd_unref (archive_bfd);
   do_cleanups (cleanup);
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index ba807a2..d8e64dc 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -621,8 +621,7 @@ darwin_bfd_open (char *pathname)
   /* The current filename for fat-binary BFDs is a name generated
      by BFD, usually a string containing the name of the architecture.
      Reset its value to the actual filename.  */
-  xfree (bfd_get_filename (res));
-  res->filename = xstrdup (pathname);
+  bfd_set_filename (res, gdb_bfd_strdup (res, pathname));
 
   gdb_bfd_unref (abfd);
   return res;
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index abb8c15..adcc72e 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -382,8 +382,7 @@ spu_bfd_open (char *pathname)
 
 	  strcat (buf, original_name);
 
-	  xfree ((char *)abfd->filename);
-	  abfd->filename = xstrdup (buf);
+	  bfd_set_filename (abfd, gdb_bfd_strdup (abfd, buf));
 	}
     }
 
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index e9b155b..e07f854 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -341,8 +341,7 @@ spu_bfd_open (ULONGEST addr)
 	  bfd_get_section_contents (nbfd, spu_name, buf, 20, sect_size - 20);
 	  buf[sect_size - 20] = '\0';
 
-	  xfree ((char *)nbfd->filename);
-	  nbfd->filename = xstrdup (buf);
+	  bfd_set_filename (nbfd, gdb_bfd_strdup (nbfd, buf));
 	}
     }
 
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..9bf6318 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -101,11 +101,10 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
     error (_("Failed to read a valid object file image from memory."));
 
   gdb_bfd_ref (nbfd);
-  xfree (bfd_get_filename (nbfd));
   if (name == NULL)
-    nbfd->filename = xstrdup ("shared object read from target memory");
+    bfd_set_filename (nbfd, "shared object read from target memory");
   else
-    nbfd->filename = name;
+    bfd_set_filename (nbfd, gdb_bfd_strdup (nbfd, name));
 
   cleanup = make_cleanup_bfd_unref (nbfd);
 
@@ -225,6 +224,7 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
       args.from_tty = 0;
       catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
 			&args, RETURN_MASK_ALL);
+      xfree (args.name);
     }
 }
 
-- 
1.7.11.7




More information about the Gdb-patches mailing list