This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[3/17] clean up allocate of BFD filenames
- From: Tom Tromey <tromey at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 14 Dec 2011 14:09:21 -0700
- Subject: [3/17] clean up allocate of BFD filenames
Currently BFD filename allocation is kind of a mess. Generally they are
xmalloc'd, but sometimes they are not.
This patch tries to clean it up by adding a new function that
reallocates the filename on the BFD's obstack. This synchronizes the
lifetime nicely and means we don't have to separately decide when to
free the filename.
This patch may be missing some spots.
Tom
>From aa099058c20639870851dcea05fda185b2335dd6 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Wed, 7 Dec 2011 10:03:56 -0700
Subject: [PATCH 03/18] clean up allocation of bfd filenames
* symfile.c (symfile_bfd_open): Don't copy name.
(load_command): Open the new BFD before freeing the old.
* symfile-mem.c (symbol_file_add_from_memory): Don't copy name.
Call gdb_bfd_stash_filename.
* spu-linux-nat.c (spu_bfd_open): Don't copy name.
* solib-spu.c (spu_bfd_fopen): Don't copy name. Call
gdb_bfd_stash_filename.
* solib-darwin.c (darwin_solib_get_all_image_info_addr_at_init):
Free found_pathname.
* rs6000-nat.c (add_vmap): Don't copy filename.
* remote.c (remote_bfd_open): Call gdb_bfd_stash_filename.
* machoread.c (macho_add_oso_symfile): Call
gdb_bfd_stash_filename.
(macho_symfile_read_all_oso): Arrange to free archive_name.
(macho_check_dsym): Don't copy filename. Call
gdb_bfd_stash_filename.
* jit.c (bfd_open_from_target_memory): Don't copy the filename.
* gdb_bfd.c (gdb_bfd_stash_filename): New function.
* gdb_bfd.h (gdb_bfd_stash_filename): Declare.
* gcore.c (create_gcore_bfd): Call gdb_bfd_stash_filename.
* exec.c (exec_close): Don't free the BFD's filename.
(exec_file_attach): Don't copy the filename.
* corelow.c (core_close): Don't free the BFD's filename.
(core_open): Call gdb_bfd_stash_filename.
* corefile.c (reopen_exec_file): Remove #if 0 code.
---
gdb/corefile.c | 5 -----
gdb/corelow.c | 6 +++---
gdb/exec.c | 12 +++---------
gdb/gcore.c | 1 +
gdb/gdb_bfd.c | 16 ++++++++++++++++
gdb/gdb_bfd.h | 6 ++++++
gdb/jit.c | 3 +--
gdb/machoread.c | 18 +++++++++---------
gdb/remote.c | 15 ++++++++++-----
gdb/rs6000-nat.c | 11 +++++------
gdb/solib-darwin.c | 1 +
gdb/solib-spu.c | 3 ++-
gdb/spu-linux-nat.c | 2 +-
gdb/symfile-mem.c | 8 ++++++--
gdb/symfile.c | 29 ++++++++++++++---------------
15 files changed, 78 insertions(+), 58 deletions(-)
diff --git a/gdb/corefile.c b/gdb/corefile.c
index ce3b755..3331f59 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -149,10 +149,6 @@ close_exec_file (void)
void
reopen_exec_file (void)
{
-#if 0 /* FIXME */
- if (exec_bfd)
- bfd_reopen (exec_bfd);
-#else
char *filename;
int res;
struct stat st;
@@ -176,7 +172,6 @@ reopen_exec_file (void)
bfd_cache_close_all ();
do_cleanups (cleanups);
-#endif
}
/* If we have both a core file and an exec file,
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 47903d8..8a0f2c6 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -226,9 +226,7 @@ core_close (int quitting)
core_data = NULL;
core_has_fake_pid = 0;
- name = bfd_get_filename (core_bfd);
gdb_bfd_unref (core_bfd);
- xfree (name);
core_bfd = NULL;
}
core_vec = NULL;
@@ -329,6 +327,8 @@ core_open (char *filename, int from_tty)
if (temp_bfd == NULL)
perror_with_name (filename);
+ gdb_bfd_stash_filename (temp_bfd);
+
if (!bfd_check_format (temp_bfd, bfd_core)
&& !gdb_check_format (temp_bfd))
{
@@ -344,7 +344,7 @@ core_open (char *filename, int from_tty)
/* Looks semi-reasonable. Toss the old core file and work on the
new. */
- discard_cleanups (old_chain); /* Don't free filename any more */
+ do_cleanups (old_chain);
unpush_target (&core_ops);
core_bfd = temp_bfd;
old_chain = make_cleanup (core_close_cleanup, 0 /*ignore*/);
diff --git a/gdb/exec.c b/gdb/exec.c
index 37eb018..19a79ed 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -101,10 +101,8 @@ exec_close (void)
if (exec_bfd)
{
bfd *abfd = exec_bfd;
- char *name = bfd_get_filename (abfd);
gdb_bfd_unref (abfd);
- xfree (name);
/* Removing target sections may close the exec_ops target.
Clear exec_bfd before doing so to prevent recursion. */
@@ -232,6 +230,9 @@ exec_file_attach (char *filename, int from_tty)
&scratch_pathname);
}
#endif
+
+ cleanups = make_cleanup (xfree, scratch_pathname);
+
if (scratch_chan < 0)
perror_with_name (filename);
exec_bfd = gdb_bfd_ref (bfd_fopen (scratch_pathname, gnutarget,
@@ -245,13 +246,6 @@ exec_file_attach (char *filename, int from_tty)
scratch_pathname, bfd_errmsg (bfd_get_error ()));
}
- /* At this point, scratch_pathname and exec_bfd->name both point to the
- same malloc'd string. However exec_close() will attempt to free it
- via the exec_bfd->name pointer, so we need to make another copy and
- leave exec_bfd as the new owner of the original copy. */
- scratch_pathname = xstrdup (scratch_pathname);
- cleanups = make_cleanup (xfree, scratch_pathname);
-
if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
{
/* Make sure to close exec_bfd, or else "run" might try to use
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 0a59567..09578ee 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -56,6 +56,7 @@ create_gcore_bfd (char *filename)
if (!obfd)
error (_("Failed to open '%s' for output."), filename);
+ gdb_bfd_stash_filename (obfd);
bfd_set_format (obfd, bfd_core);
bfd_set_arch_mach (obfd, default_gcore_arch (), default_gcore_mach ());
return obfd;
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 160e9e6..8f40d74 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -21,6 +21,22 @@
#include "defs.h"
#include "gdb_bfd.h"
#include "gdb_assert.h"
+#include "gdb_string.h"
+
+/* See gdb_bfd.h. */
+
+void
+gdb_bfd_stash_filename (struct bfd *abfd)
+{
+ char *name = bfd_get_filename (abfd);
+ char *data;
+
+ data = bfd_alloc (abfd, strlen (name) + 1);
+ strcpy (data, name);
+
+ /* Unwarranted chumminess with BFD. */
+ abfd->filename = data;
+}
/* Close ABFD, and warn if that fails. */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 2708d01..d2d7e5d 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -21,6 +21,12 @@
#ifndef GDB_BFD_H
#define GDB_BFD_H
+/* Make a copy ABFD's filename using bfd_alloc, and reassign it to the
+ BFD. This ensures that the BFD's filename has the same lifetime as
+ the BFD itself. */
+
+void gdb_bfd_stash_filename (struct bfd *abfd);
+
/* Acquire a new reference to ABFD. Returns ABFD for convenience. */
struct bfd *gdb_bfd_ref (struct bfd *abfd);
diff --git a/gdb/jit.c b/gdb/jit.c
index a4899ce..24ab237 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -219,12 +219,11 @@ jit_reader_unload_command (char *args, int from_tty)
static struct bfd *
bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size, char *target)
{
- const char *filename = xstrdup ("<in-memory>");
struct target_buffer *buffer = xmalloc (sizeof (struct target_buffer));
buffer->base = addr;
buffer->size = size;
- return bfd_openr_iovec (filename, target,
+ return bfd_openr_iovec ("<in-memory>", target,
mem_bfd_iovec_open,
buffer,
mem_bfd_iovec_pread,
diff --git a/gdb/machoread.c b/gdb/machoread.c
index e32dead..a384940 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -629,10 +629,10 @@ macho_add_oso_symfile (oso_el *oso, bfd *abfd,
bfd_hash_table_free (&table);
- /* Make sure that the filename was malloc'ed. The current filename comes
- either from an OSO symbol name or from an archive name. Memory for both
- is not managed by gdb. */
- abfd->filename = xstrdup (abfd->filename);
+ /* Make sure that the filename has the correct lifetime. The
+ current filename comes either from an OSO symbol name or from an
+ archive name. Memory for both is not managed by gdb. */
+ gdb_bfd_stash_filename (abfd);
/* We need to clear SYMFILE_MAINLINE to avoid interractive question
from symfile.c:symbol_file_add_with_addrs_or_offsets. */
@@ -651,6 +651,7 @@ macho_symfile_read_all_oso (struct objfile *main_objfile, int symfile_flags)
int ix;
VEC (oso_el) *vec;
oso_el *oso;
+ struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
vec = oso_vector;
oso_vector = NULL;
@@ -677,6 +678,8 @@ macho_symfile_read_all_oso (struct objfile *main_objfile, int symfile_flags)
memcpy (archive_name, oso->name, pfx_len);
archive_name[pfx_len] = '\0';
+ make_cleanup (xfree, archive_name);
+
/* Compute number of oso for this archive. */
for (last_ix = ix;
VEC_iterate (oso_el, vec, last_ix, oso2); last_ix++)
@@ -773,6 +776,7 @@ macho_symfile_read_all_oso (struct objfile *main_objfile, int symfile_flags)
}
VEC_free (oso_el, vec);
+ do_cleanups (cleanup);
}
/* DSYM (debug symbols) files contain the debug info of an executable.
@@ -810,20 +814,18 @@ macho_check_dsym (struct objfile *objfile)
warning (_("can't find UUID in %s"), objfile->name);
return NULL;
}
- dsym_filename = xstrdup (dsym_filename);
dsym_bfd = gdb_bfd_ref (bfd_openr (dsym_filename, gnutarget));
if (dsym_bfd == NULL)
{
warning (_("can't open dsym file %s"), dsym_filename);
- xfree (dsym_filename);
return NULL;
}
+ gdb_bfd_stash_filename (dsym_filename);
if (!bfd_check_format (dsym_bfd, bfd_object))
{
gdb_bfd_unref (dsym_bfd);
warning (_("bad dsym file format: %s"), bfd_errmsg (bfd_get_error ()));
- xfree (dsym_filename);
return NULL;
}
@@ -832,7 +834,6 @@ macho_check_dsym (struct objfile *objfile)
{
warning (_("can't find UUID in %s"), dsym_filename);
gdb_bfd_unref (dsym_bfd);
- xfree (dsym_filename);
return NULL;
}
if (memcmp (dsym_uuid->command.uuid.uuid, main_uuid->command.uuid.uuid,
@@ -840,7 +841,6 @@ macho_check_dsym (struct objfile *objfile)
{
warning (_("dsym file UUID doesn't match the one in %s"), objfile->name);
gdb_bfd_unref (dsym_bfd);
- xfree (dsym_filename);
return NULL;
}
return dsym_bfd;
diff --git a/gdb/remote.c b/gdb/remote.c
index 9bfebd2..4a51890 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -44,6 +44,7 @@
#include "cli/cli-decode.h"
#include "cli/cli-setshow.h"
#include "target-descriptions.h"
+#include "gdb_bfd.h"
#include <ctype.h>
#include <sys/time.h>
@@ -9494,11 +9495,15 @@ remote_filename_p (const char *filename)
bfd *
remote_bfd_open (const char *remote_file, const char *target)
{
- return bfd_openr_iovec (remote_file, target,
- remote_bfd_iovec_open, NULL,
- remote_bfd_iovec_pread,
- remote_bfd_iovec_close,
- remote_bfd_iovec_stat);
+ bfd *abfd = bfd_openr_iovec (remote_file, target,
+ remote_bfd_iovec_open, NULL,
+ remote_bfd_iovec_pread,
+ remote_bfd_iovec_close,
+ remote_bfd_iovec_stat);
+
+ if (abfd != NULL)
+ gdb_bfd_stash_filename (abfd);
+ return abfd;
}
void
diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
index e471894..a92f8cb 100644
--- a/gdb/rs6000-nat.c
+++ b/gdb/rs6000-nat.c
@@ -730,7 +730,7 @@ static struct vmap *
add_vmap (LdInfo *ldi)
{
bfd *abfd, *last;
- char *mem, *objname, *filename;
+ char *mem, *filename;
struct objfile *obj;
struct vmap *vp;
int fd;
@@ -743,7 +743,6 @@ add_vmap (LdInfo *ldi)
filename = LDI_FILENAME (ldi, arch64);
mem = filename + strlen (filename) + 1;
mem = xstrdup (mem);
- objname = xstrdup (filename);
fd = LDI_FD (ldi, arch64);
if (fd < 0)
@@ -756,7 +755,7 @@ add_vmap (LdInfo *ldi)
if (!abfd)
{
warning (_("Could not open `%s' as an executable file: %s"),
- objname, bfd_errmsg (bfd_get_error ()));
+ filename, bfd_errmsg (bfd_get_error ()));
return NULL;
}
@@ -775,7 +774,7 @@ add_vmap (LdInfo *ldi)
if (!last)
{
- warning (_("\"%s\": member \"%s\" missing."), objname, mem);
+ warning (_("\"%s\": member \"%s\" missing."), filename, mem);
gdb_bfd_unref (abfd);
return NULL;
}
@@ -783,7 +782,7 @@ add_vmap (LdInfo *ldi)
if (!bfd_check_format (last, bfd_object))
{
warning (_("\"%s\": member \"%s\" not in executable format: %s."),
- objname, mem, bfd_errmsg (bfd_get_error ()));
+ filename, mem, bfd_errmsg (bfd_get_error ()));
gdb_bfd_unref (last);
gdb_bfd_unref (abfd);
return NULL;
@@ -794,7 +793,7 @@ add_vmap (LdInfo *ldi)
else
{
warning (_("\"%s\": not in executable format: %s."),
- objname, bfd_errmsg (bfd_get_error ()));
+ filename, bfd_errmsg (bfd_get_error ()));
gdb_bfd_unref (abfd);
return NULL;
}
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index d7b2128..cf92deb 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -457,6 +457,7 @@ darwin_bfd_open (char *pathname)
error (_("`%s': not a shared-library: %s"),
found_pathname, bfd_errmsg (bfd_get_error ()));
}
+ xfree (found_pathname);
return res;
}
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index 6163c41..3f21dda 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -325,7 +325,7 @@ spu_bfd_fopen (char *name, CORE_ADDR addr)
CORE_ADDR *open_closure = xmalloc (sizeof (CORE_ADDR));
*open_closure = addr;
- nbfd = gdb_bfd_ref (bfd_openr_iovec (xstrdup (name), "elf32-spu",
+ nbfd = gdb_bfd_ref (bfd_openr_iovec (name, "elf32-spu",
spu_bfd_iovec_open, open_closure,
spu_bfd_iovec_pread, spu_bfd_iovec_close,
spu_bfd_iovec_stat));
@@ -338,6 +338,7 @@ spu_bfd_fopen (char *name, CORE_ADDR addr)
return NULL;
}
+ gdb_bfd_stash_filename (nbfd);
return nbfd;
}
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index 2e5f3dc..67bfa2a 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -315,7 +315,7 @@ spu_bfd_open (ULONGEST addr)
ULONGEST *open_closure = xmalloc (sizeof (ULONGEST));
*open_closure = addr;
- nbfd = bfd_openr_iovec (xstrdup ("<in-memory>"), "elf32-spu",
+ nbfd = bfd_openr_iovec ("<in-memory>", "elf32-spu",
spu_bfd_iovec_open, open_closure,
spu_bfd_iovec_pread, spu_bfd_iovec_close,
spu_bfd_iovec_stat);
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 0ae35a7..da1592b 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -85,9 +85,13 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
gdb_bfd_ref (nbfd);
if (name == NULL)
- nbfd->filename = xstrdup ("shared object read from target memory");
+ nbfd->filename = "shared object read from target memory";
else
- nbfd->filename = name;
+ {
+ nbfd->filename = name;
+ gdb_bfd_stash_filename (nbfd);
+ xfree (name);
+ }
if (!bfd_check_format (nbfd, bfd_object))
{
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 314e204..74a0317 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1680,19 +1680,14 @@ symfile_bfd_open (char *name)
if (remote_filename_p (name))
{
- name = xstrdup (name);
sym_bfd = gdb_bfd_ref (remote_bfd_open (name, gnutarget));
if (!sym_bfd)
- {
- make_cleanup (xfree, name);
- error (_("`%s': can't open to read symbols: %s."), name,
- bfd_errmsg (bfd_get_error ()));
- }
+ error (_("`%s': can't open to read symbols: %s."), name,
+ bfd_errmsg (bfd_get_error ()));
if (!bfd_check_format (sym_bfd, bfd_object))
{
- gdb_bfd_unref (sym_bfd);
- make_cleanup (xfree, name);
+ make_cleanup_bfd_close (sym_bfd);
error (_("`%s': can't read symbols: %s."), name,
bfd_errmsg (bfd_get_error ()));
}
@@ -1721,16 +1716,14 @@ symfile_bfd_open (char *name)
perror_with_name (name);
}
- /* Free 1st new malloc'd copy, but keep the 2nd malloc'd copy in
- bfd. It'll be freed in free_objfile(). */
xfree (name);
name = absolute_name;
+ make_cleanup (xfree, name);
sym_bfd = gdb_bfd_ref (bfd_fopen (name, gnutarget, FOPEN_RB, desc));
if (!sym_bfd)
{
close (desc);
- make_cleanup (xfree, name);
error (_("`%s': can't open to read symbols: %s."), name,
bfd_errmsg (bfd_get_error ()));
}
@@ -1739,7 +1732,6 @@ symfile_bfd_open (char *name)
if (!bfd_check_format (sym_bfd, bfd_object))
{
make_cleanup_bfd_close (sym_bfd);
- make_cleanup (xfree, name);
error (_("`%s': can't read symbols: %s."), name,
bfd_errmsg (bfd_get_error ()));
}
@@ -2444,9 +2436,16 @@ reread_symbols (void)
/* Clean up any state BFD has sitting around. We don't need
to close the descriptor but BFD lacks a way of closing the
BFD without closing the descriptor. */
- obfd_filename = bfd_get_filename (objfile->obfd);
- gdb_bfd_unref (objfile->obfd);
- objfile->obfd = bfd_open_maybe_remote (obfd_filename);
+ {
+ struct bfd *obfd = objfile->obfd;
+
+ obfd_filename = bfd_get_filename (objfile->obfd);
+ /* Open the new BFD before freeing the old one, so that
+ the filename remains live. */
+ objfile->obfd = bfd_open_maybe_remote (obfd_filename);
+ gdb_bfd_unref (obfd);
+ }
+
if (objfile->obfd == NULL)
error (_("Can't open %s to read symbols."), objfile->name);
/* bfd_openr sets cacheable to true, which is what we want. */
--
1.7.6.4