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  <tom@tromey.com>

	* 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.
This commit is contained in:
Tom Tromey 2016-11-21 16:26:20 -07:00
parent 192b62ce0b
commit bef155c3e8
5 changed files with 110 additions and 74 deletions

View File

@ -1,3 +1,14 @@
2017-01-10 Tom Tromey <tom@tromey.com>
* 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 <tom@tromey.com>
* windows-tdep.c (windows_xfer_shared_library): Update.

60
gdb/common/gdb_unlinker.h Normal file
View File

@ -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 <http://www.gnu.org/licenses/>. */
#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 */

View File

@ -69,6 +69,7 @@
#include "filestuff.h"
#include "build-id.h"
#include "namespace.h"
#include "common/gdb_unlinker.h"
#include <fcntl.h>
#include <sys/types.h>
@ -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);
}

View File

@ -35,6 +35,7 @@
#include "gdb_bfd.h"
#include "readline/tilde.h"
#include <algorithm>
#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<char> 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

View File

@ -35,6 +35,7 @@
#include "gdb_bfd.h"
#include "observer.h"
#include "infrun.h"
#include "common/gdb_unlinker.h"
#include <signal.h>
@ -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, &regnum,
bfdcore_write (obfd.get (), osec, &regnum,
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"),