From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15560 invoked by alias); 7 Jan 2014 19:54:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 15455 invoked by uid 89); 7 Jan 2014 19:54:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2014 19:54:43 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s07JsXPf032623 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 7 Jan 2014 14:54:37 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s07JsSxJ014339; Tue, 7 Jan 2014 14:54:29 -0500 Message-ID: <52CC5B74.6010901@redhat.com> Date: Tue, 07 Jan 2014 19:54:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Tom Tromey CC: Doug Evans , Hui Zhu , Sergio Durigan Junior , gdb-patches ml , Edjunior Barbosa Machado , Nick Clifton , Binutils Development Subject: Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983 References: <52C8358B.7080101@mentor.com> <52C97EC0.3080807@mentor.com> <87k3edseia.fsf@fleche.redhat.com> <52CA8A7F.7090907@mentor.com> <878uuslt1j.fsf@fleche.redhat.com> <52CBF47C.9010002@redhat.com> <87k3ebheqh.fsf@fleche.redhat.com> In-Reply-To: <87k3ebheqh.fsf@fleche.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-01/txt/msg00161.txt.bz2 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 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 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, ""); 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 (""); + bfd_set_filename (nbfd, ""); 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 <> 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