From bef155c3e8a995fcdb1c2ba5aba012eb653d9f30 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Mon, 21 Nov 2016 16:26:20 -0700 Subject: [PATCH] Introduce and use gdb::unlinker This introduces a new class, gdb::unlinker, that unlinks a file in the destructor. The user of this class has the option to preserve the file instead, by calling the "keep" method. This patch then changes the spots in gdb that use unlink in a cleanup to use this class instead. In one spot I went ahead and removed all the cleanups from the function. This fixes one latent bug -- do_bfd_delete_cleanup could refer to freed memory, by decref'ing the BFD before using its filename. 2017-01-10 Tom Tromey * record-full.c (record_full_save_cleanups): Remove. (record_full_save): Use gdb::unlinker. * gcore.c (do_bfd_delete_cleanup): Remove. (gcore_command): Use gdb::unlinker, unique_xmalloc_ptr. Remove cleanups. * dwarf2read.c (unlink_if_set): Remove. (write_psymtabs_to_index): Use gdb::unlinker. * common/gdb_unlinker.h: New file. --- gdb/ChangeLog | 11 +++++++++ gdb/common/gdb_unlinker.h | 60 +++++++++++++++++++++++++++++++++++++++++++++++ gdb/dwarf2read.c | 21 ++++------------- gdb/gcore.c | 38 ++++++++++-------------------- gdb/record-full.c | 54 +++++++++++++++++------------------------- 5 files changed, 110 insertions(+), 74 deletions(-) create mode 100644 gdb/common/gdb_unlinker.h diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 57f21c2..f1e6458 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,16 @@ 2017-01-10 Tom Tromey + * record-full.c (record_full_save_cleanups): Remove. + (record_full_save): Use gdb::unlinker. + * gcore.c (do_bfd_delete_cleanup): Remove. + (gcore_command): Use gdb::unlinker, unique_xmalloc_ptr. Remove + cleanups. + * dwarf2read.c (unlink_if_set): Remove. + (write_psymtabs_to_index): Use gdb::unlinker. + * common/gdb_unlinker.h: New file. + +2017-01-10 Tom Tromey + * windows-tdep.c (windows_xfer_shared_library): Update. * windows-nat.c (windows_make_so): Update. * utils.h (make_cleanup_bfd_unref): Remove. diff --git a/gdb/common/gdb_unlinker.h b/gdb/common/gdb_unlinker.h new file mode 100644 index 0000000..0f3b98a --- /dev/null +++ b/gdb/common/gdb_unlinker.h @@ -0,0 +1,60 @@ +/* Unlinking class + + Copyright (C) 2016 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef GDB_UNLINKER_H +#define GDB_UNLINKER_H + +namespace gdb +{ + +/* An object of this class holds a filename and, when the object goes + of scope, the file is removed using unlink. + + A user of this class can request that the file be preserved using + the "keep" method. */ +class unlinker +{ + public: + + unlinker (const char *filename) ATTRIBUTE_NONNULL (1) + : m_filename (filename) + { + gdb_assert (filename != NULL); + } + + ~unlinker () + { + if (m_filename != NULL) + unlink (m_filename); + } + + /* Keep the file, rather than unlink it. */ + void keep () + { + m_filename = NULL; + } + + private: + + const char *m_filename; +}; + +} + +#endif /* GDB_UNLINKER_H */ diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 8a66848..dfb79c9 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -69,6 +69,7 @@ #include "filestuff.h" #include "build-id.h" #include "namespace.h" +#include "common/gdb_unlinker.h" #include #include @@ -23165,16 +23166,6 @@ write_obstack (FILE *file, struct obstack *obstack) error (_("couldn't data write to file")); } -/* Unlink a file if the argument is not NULL. */ - -static void -unlink_if_set (void *p) -{ - char **filename = (char **) p; - if (*filename) - unlink (*filename); -} - /* A helper struct used when iterating over debug_types. */ struct signatured_type_index_data { @@ -23259,7 +23250,7 @@ static void write_psymtabs_to_index (struct objfile *objfile, const char *dir) { struct cleanup *cleanup; - char *filename, *cleanup_filename; + char *filename; struct obstack contents, addr_obstack, constant_pool, symtab_obstack; struct obstack cu_list, types_cu_list; int i; @@ -23289,8 +23280,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir) if (!out_file) error (_("Can't open `%s' for writing"), filename); - cleanup_filename = filename; - make_cleanup (unlink_if_set, &cleanup_filename); + gdb::unlinker unlink_file (filename); symtab = create_mapped_symtab (); make_cleanup (cleanup_mapped_symtab, symtab); @@ -23429,9 +23419,8 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir) fclose (out_file); - /* We want to keep the file, so we set cleanup_filename to NULL - here. See unlink_if_set. */ - cleanup_filename = NULL; + /* We want to keep the file. */ + unlink_file.keep (); do_cleanups (cleanup); } diff --git a/gdb/gcore.c b/gdb/gcore.c index 1d8a63f..c32d2ff 100644 --- a/gdb/gcore.c +++ b/gdb/gcore.c @@ -35,6 +35,7 @@ #include "gdb_bfd.h" #include "readline/tilde.h" #include +#include "common/gdb_unlinker.h" /* The largest amount of memory to read from the target at once. We must throttle it to limit the amount of memory used by GDB during @@ -135,59 +136,44 @@ write_gcore_file (bfd *obfd) throw_exception (except); } -static void -do_bfd_delete_cleanup (void *arg) -{ - bfd *obfd = (bfd *) arg; - const char *filename = obfd->filename; - - gdb_bfd_unref ((bfd *) arg); - unlink (filename); -} - /* gcore_command -- implements the 'gcore' command. Generate a core file from the inferior process. */ static void gcore_command (char *args, int from_tty) { - struct cleanup *filename_chain; - struct cleanup *bfd_chain; - char *corefilename; - bfd *obfd; + gdb::unique_xmalloc_ptr corefilename; /* No use generating a corefile without a target process. */ if (!target_has_execution) noprocess (); if (args && *args) - corefilename = tilde_expand (args); + corefilename.reset (tilde_expand (args)); else { /* Default corefile name is "core.PID". */ - corefilename = xstrprintf ("core.%d", ptid_get_pid (inferior_ptid)); + corefilename.reset (xstrprintf ("core.%d", ptid_get_pid (inferior_ptid))); } - filename_chain = make_cleanup (xfree, corefilename); if (info_verbose) fprintf_filtered (gdb_stdout, - "Opening corefile '%s' for output.\n", corefilename); + "Opening corefile '%s' for output.\n", + corefilename.get ()); /* Open the output file. */ - obfd = create_gcore_bfd (corefilename).release (); + gdb_bfd_ref_ptr obfd (create_gcore_bfd (corefilename.get ())); - /* Need a cleanup that will close and delete the file. */ - bfd_chain = make_cleanup (do_bfd_delete_cleanup, obfd); + /* Arrange to unlink the file on failure. */ + gdb::unlinker unlink_file (corefilename.get ()); /* Call worker function. */ - write_gcore_file (obfd); + write_gcore_file (obfd.get ()); /* Succeeded. */ - discard_cleanups (bfd_chain); - gdb_bfd_unref (obfd); + unlink_file.keep (); - fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename); - do_cleanups (filename_chain); + fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename.get ()); } static unsigned long diff --git a/gdb/record-full.c b/gdb/record-full.c index e4ff6a4..fdd613b 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -35,6 +35,7 @@ #include "gdb_bfd.h" #include "observer.h" #include "infrun.h" +#include "common/gdb_unlinker.h" #include @@ -2537,17 +2538,6 @@ cmd_record_full_restore (char *args, int from_tty) record_full_open (args, from_tty); } -static void -record_full_save_cleanups (void *data) -{ - bfd *obfd = (bfd *) data; - char *pathname = xstrdup (bfd_get_filename (obfd)); - - gdb_bfd_unref (obfd); - unlink (pathname); - xfree (pathname); -} - /* Save the execution log to a file. We use a modified elf corefile format, with an extra section for our data. */ @@ -2558,9 +2548,7 @@ record_full_save (struct target_ops *self, const char *recfilename) uint32_t magic; struct regcache *regcache; struct gdbarch *gdbarch; - struct cleanup *old_cleanups; struct cleanup *set_cleanups; - bfd *obfd; int save_size = 0; asection *osec = NULL; int bfd_offset = 0; @@ -2571,8 +2559,10 @@ record_full_save (struct target_ops *self, const char *recfilename) recfilename); /* Open the output file. */ - obfd = create_gcore_bfd (recfilename).release (); - old_cleanups = make_cleanup (record_full_save_cleanups, obfd); + gdb_bfd_ref_ptr obfd (create_gcore_bfd (recfilename)); + + /* Arrange to remove the output file on failure. */ + gdb::unlinker unlink_file (recfilename); /* Save the current record entry to "cur_record_full_list". */ cur_record_full_list = record_full_list; @@ -2615,20 +2605,20 @@ record_full_save (struct target_ops *self, const char *recfilename) } /* Make the new bfd section. */ - osec = bfd_make_section_anyway_with_flags (obfd, "precord", + osec = bfd_make_section_anyway_with_flags (obfd.get (), "precord", SEC_HAS_CONTENTS | SEC_READONLY); if (osec == NULL) error (_("Failed to create 'precord' section for corefile %s: %s"), recfilename, bfd_errmsg (bfd_get_error ())); - bfd_set_section_size (obfd, osec, save_size); - bfd_set_section_vma (obfd, osec, 0); - bfd_set_section_alignment (obfd, osec, 0); - bfd_section_lma (obfd, osec) = 0; + bfd_set_section_size (obfd.get (), osec, save_size); + bfd_set_section_vma (obfd.get (), osec, 0); + bfd_set_section_alignment (obfd.get (), osec, 0); + bfd_section_lma (obfd.get (), osec) = 0; /* Save corefile state. */ - write_gcore_file (obfd); + write_gcore_file (obfd.get ()); /* Write out the record log. */ /* Write the magic code. */ @@ -2638,7 +2628,7 @@ record_full_save (struct target_ops *self, const char *recfilename) " Writing 4-byte magic cookie " "RECORD_FULL_FILE_MAGIC (0x%s)\n", phex_nz (magic, 4)); - bfdcore_write (obfd, osec, &magic, sizeof (magic), &bfd_offset); + bfdcore_write (obfd.get (), osec, &magic, sizeof (magic), &bfd_offset); /* Save the entries to recfd and forward execute to the end of record list. */ @@ -2653,7 +2643,7 @@ record_full_save (struct target_ops *self, const char *recfilename) uint64_t addr; type = record_full_list->type; - bfdcore_write (obfd, osec, &type, sizeof (type), &bfd_offset); + bfdcore_write (obfd.get (), osec, &type, sizeof (type), &bfd_offset); switch (record_full_list->type) { @@ -2668,11 +2658,11 @@ record_full_save (struct target_ops *self, const char *recfilename) /* Write regnum. */ regnum = netorder32 (record_full_list->u.reg.num); - bfdcore_write (obfd, osec, ®num, + bfdcore_write (obfd.get (), osec, ®num, sizeof (regnum), &bfd_offset); /* Write regval. */ - bfdcore_write (obfd, osec, + bfdcore_write (obfd.get (), osec, record_full_get_loc (record_full_list), record_full_list->u.reg.len, &bfd_offset); break; @@ -2690,15 +2680,16 @@ record_full_save (struct target_ops *self, const char *recfilename) /* Write memlen. */ len = netorder32 (record_full_list->u.mem.len); - bfdcore_write (obfd, osec, &len, sizeof (len), &bfd_offset); + bfdcore_write (obfd.get (), osec, &len, sizeof (len), + &bfd_offset); /* Write memaddr. */ addr = netorder64 (record_full_list->u.mem.addr); - bfdcore_write (obfd, osec, &addr, + bfdcore_write (obfd.get (), osec, &addr, sizeof (addr), &bfd_offset); /* Write memval. */ - bfdcore_write (obfd, osec, + bfdcore_write (obfd.get (), osec, record_full_get_loc (record_full_list), record_full_list->u.mem.len, &bfd_offset); break; @@ -2712,12 +2703,12 @@ record_full_save (struct target_ops *self, const char *recfilename) (unsigned long) sizeof (count)); /* Write signal value. */ signal = netorder32 (record_full_list->u.end.sigval); - bfdcore_write (obfd, osec, &signal, + bfdcore_write (obfd.get (), osec, &signal, sizeof (signal), &bfd_offset); /* Write insn count. */ count = netorder32 (record_full_list->u.end.insn_num); - bfdcore_write (obfd, osec, &count, + bfdcore_write (obfd.get (), osec, &count, sizeof (count), &bfd_offset); break; } @@ -2746,8 +2737,7 @@ record_full_save (struct target_ops *self, const char *recfilename) } do_cleanups (set_cleanups); - gdb_bfd_unref (obfd); - discard_cleanups (old_cleanups); + unlink_file.keep (); /* Succeeded. */ printf_filtered (_("Saved core file %s with execution log.\n"), -- 2.7.4