Re: [RFC] Alternate approach to keeping convenience variables

Daniel Jacobowitz
2006-01-22T21:31:26Z

This version of the patch addresses all the comments I've received,
except for:

- TYPE_ZALLOC. I think this is more appropriate as a macro,
personally, though I wouldn't argue too loudly if someone wanted to
functionify it at the same time as TYPE_ALLOC.

GDB developers seem to have developed an allergy to C macros that
I just don't understand. They're not _inherently_ obfuscating
or evil! They can be both useful and elegant.

- gdbint; I couldn't find a vaguely appropriate place to add a
description of this, and I don't think it's such a fascinating
approach that it's worth creating a new section for.

- $tpnum. Eli was in favor of the change I'd made, Jim preferred
the opposite. We don't set $bpnum until the first breakpoint
is set; Jim, is that a convincing argument for not setting
$tpnum until the first tracepoint is set?

I noticed in the course of updating gdb.texinfo that there are still a
whole lot of references to VxWorks. GDB no longer supports VxWorks,
and current versions of VxWorks don't speak a compatible protocol to
the one that GDB used to support, anyway. Ripping this out will be a
little more work than I have time for at the moment though.

--
Daniel Jacobowitz
CodeSourcery

2006-01-22 Daniel Jacobowitz <dan@codesourcery.com>

* Makefile.in (gdbtypes_h, gdbtypes.o, utils.o): Update.
* defs.h (hashtab_obstack_allocate, dummy_obstack_deallocate): Add
prototypes.
* dwarf2read.c (read_subroutine_type): Use TYPE_ZALLOC.
(hashtab_obstack_allocate, dummy_obstack_deallocate): Moved to...
* utils.c (hashtab_obstack_allocate, dummy_obstack_deallocate):
...here.
* gdbtypes.c: Include "hashtab.h".
(build_gdbtypes): Remove extra prototype.
(struct type_pair, type_pair_hash, type_pair_eq)
(create_copied_types_hash, copy_type_recursive): New.
* gdbtypes.h: Include "hashtab.h".
(TYPE_ZALLOC): New.
(create_copied_types_hash, copy_type_recursive): New prototypes.
* objfiles.c (free_objfile): Call preserve_values.
* symfile.c (reread_symbols): Likewise.
(clear_symtab_users): Remove calls to clear_value_history and
clear_internalvars.
* value.c (clear_value_history, clear_internalvars): Removed.
(preserve_one_value, preserve_values): New functions.
* value.h (clear_value_history, clear_internalvars): Removed.
(preserve_values): New prototype.

* tracepoint.c (_initialize_tracepoint): Do not initialize convenience
variables here.

* doc/gdb.texinfo (Files): Remove obsolete bits from the description
of "symbol-file".

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/Makefile.in 2006-01-22 16:08:23.000000000 -0500
@@ -696,7 +696,7 @@ gdb_stat_h = gdb_stat.h
gdb_string_h = gdb_string.h
gdb_thread_db_h = gdb_thread_db.h
gdbthread_h = gdbthread.h $(breakpoint_h) $(frame_h)
-gdbtypes_h = gdbtypes.h
+gdbtypes_h = gdbtypes.h $(hashtab_h)
gdb_vfork_h = gdb_vfork.h
gdb_wait_h = gdb_wait.h
glibc_tdep_h = glibc-tdep.h
@@ -1979,7 +1979,7 @@ gdb-events.o: gdb-events.c $(defs_h) $(g
gdbtypes.o: gdbtypes.c $(defs_h) $(gdb_string_h) $(bfd_h) $(symtab_h) \
$(symfile_h) $(objfiles_h) $(gdbtypes_h) $(expression_h) \
$(language_h) $(target_h) $(value_h) $(demangle_h) $(complaints_h) \
- $(gdbcmd_h) $(wrapper_h) $(cp_abi_h) $(gdb_assert_h)
+ $(gdbcmd_h) $(wrapper_h) $(cp_abi_h) $(gdb_assert_h) $(hashtab_h)
glibc-tdep.o: glibc-tdep.c $(defs_h) $(frame_h) $(symtab_h) $(symfile_h) \
$(objfiles_h) $(glibc_tdep_h)
gnu-nat.o: gnu-nat.c $(gdb_string_h) $(defs_h) $(inferior_h) $(symtab_h) \
@@ -2714,7 +2714,7 @@ utils.o: utils.c $(defs_h) $(gdb_assert_
$(exceptions_h) $(tui_h) $(gdbcmd_h) $(serial_h) $(bfd_h) \
$(target_h) $(demangle_h) $(expression_h) $(language_h) $(charset_h) \
$(annotate_h) $(filenames_h) $(symfile_h) $(inferior_h) \
- $(gdb_curses_h) $(readline_h)
+ $(gdb_curses_h) $(readline_h) $(gdb_obstack_h)
uw-thread.o: uw-thread.c $(defs_h) $(gdbthread_h) $(target_h) $(inferior_h) \
$(regcache_h) $(gregset_h)
v850-tdep.o: v850-tdep.c $(defs_h) $(frame_h) $(frame_base_h) $(trad_frame_h) \
Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/defs.h 2006-01-22 16:08:23.000000000 -0500
@@ -1217,4 +1217,9 @@ extern int use_windows;
extern ULONGEST align_up (ULONGEST v, int n);
extern ULONGEST align_down (ULONGEST v, int n);

+/* Allocation and deallocation functions for the libiberty hash table
+ which use obstacks. */
+void *hashtab_obstack_allocate (void *data, size_t size, size_t count);
+void dummy_obstack_deallocate (void *object, void *data);
+
#endif /* #ifndef DEFS_H */
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/dwarf2read.c 2006-01-22 16:08:23.000000000 -0500
@@ -1026,10 +1026,6 @@ static char *skip_one_die (char *info_pt

static void free_stack_comp_unit (void *);

-static void *hashtab_obstack_allocate (void *data, size_t size, size_t count);
-
-static void dummy_obstack_deallocate (void *object, void *data);
-
static hashval_t partial_die_hash (const void *item);

static int partial_die_eq (const void *item_lhs, const void *item_rhs);
@@ -4622,7 +4618,7 @@ read_subroutine_type (struct die_info *d
/* Allocate storage for parameters and fill them in. */
TYPE_NFIELDS (ftype) = nparams;
TYPE_FIELDS (ftype) = (struct field *)
- TYPE_ALLOC (ftype, nparams * sizeof (struct field));
+ TYPE_ZALLOC (ftype, nparams * sizeof (struct field));

child_die = die->child;
while (child_die && child_die->tag)
@@ -9617,28 +9613,6 @@ dwarf2_clear_marks (struct dwarf2_per_cu
}
}

-/* Allocation function for the libiberty hash table which uses an
- obstack. */
-
-static void *
-hashtab_obstack_allocate (void *data, size_t size, size_t count)
-{
- unsigned int total = size * count;
- void *ptr = obstack_alloc ((struct obstack *) data, total);
- memset (ptr, 0, total);
- return ptr;
-}
-
-/* Trivial deallocation function for the libiberty splay tree and hash
- table - don't deallocate anything. Rely on later deletion of the
- obstack. */
-
-static void
-dummy_obstack_deallocate (void *object, void *data)
-{
- return;
-}
-
/* Trivial hash function for partial_die_info: the hash value of a DIE
is its offset in .debug_info for this objfile. */

Index: src/gdb/gdbtypes.c
===================================================================
--- src.orig/gdb/gdbtypes.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/gdbtypes.c 2006-01-22 16:08:40.000000000 -0500
@@ -1,6 +1,6 @@
/* Support routines for manipulating internal types for GDB.
Copyright 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000, 2001, 2002, 2003,
- 2004 Free Software Foundation, Inc.
+ 2004, 2005 Free Software Foundation, Inc.
Contributed by Cygnus Support, using pieces from other GDB modules.

This file is part of GDB.
@@ -37,6 +37,7 @@
#include "wrapper.h"
#include "cp-abi.h"
#include "gdb_assert.h"
+#include "hashtab.h"

/* These variables point to the objects
representing the predefined C data types. */
@@ -3111,7 +3112,147 @@ recursive_dump_type (struct type *type,
obstack_free (&dont_print_type_obstack, NULL);
}

-static void build_gdbtypes (void);
+/* Trivial helpers for the libiberty hash table, for mapping one
+ type to another. */
+
+struct type_pair
+{
+ struct type *old, *new;
+};
+
+static hashval_t
+type_pair_hash (const void *item)
+{
+ const struct type_pair *pair = item;
+ return htab_hash_pointer (pair->old);
+}
+
+static int
+type_pair_eq (const void *item_lhs, const void *item_rhs)
+{
+ const struct type_pair *lhs = item_lhs, *rhs = item_rhs;
+ return lhs->old == rhs->old;
+}
+
+/* Allocate the hash table used by copy_type_recursive to walk
+ types without duplicates. We use OBJFILE's obstack, because
+ OBJFILE is about to be deleted. */
+
+htab_t
+create_copied_types_hash (struct objfile *objfile)
+{
+ return htab_create_alloc_ex (1, type_pair_hash, type_pair_eq,
+ NULL, &objfile->objfile_obstack,
+ hashtab_obstack_allocate,
+ dummy_obstack_deallocate);
+}
+
+/* Recursively copy (deep copy) TYPE, if it is associated with OBJFILE.
+ Return a new type allocated using malloc, a saved type if we have already
+ visited TYPE (using COPIED_TYPES), or TYPE if it is not associated with
+ OBJFILE. */
+
+struct type *
+copy_type_recursive (struct objfile *objfile, struct type *type,
+ htab_t copied_types)
+{
+ struct type_pair *stored, pair;
+ void **slot;
+ struct type *new_type;
+
+ if (TYPE_OBJFILE (type) == NULL)
+ return type;
+
+ /* This type shouldn't be pointing to any types in other objfiles; if
+ it did, the type might disappear unexpectedly. */
+ gdb_assert (TYPE_OBJFILE (type) == objfile);
+
+ pair.old = type;
+ slot = htab_find_slot (copied_types, &pair, INSERT);
+ if (*slot != NULL)
+ return ((struct type_pair *) *slot)->new;
+
+ new_type = alloc_type (NULL);
+
+ /* We must add the new type to the hash table immediately, in case
+ we encounter this type again during a recursive call below. */
+ stored = xmalloc (sizeof (struct type_pair));
+ stored->old = type;
+ stored->new = new_type;
+ *slot = stored;
+
+ /* Copy the common fields of types. */
+ TYPE_CODE (new_type) = TYPE_CODE (type);
+ TYPE_ARRAY_UPPER_BOUND_TYPE (new_type) = TYPE_ARRAY_UPPER_BOUND_TYPE (type);
+ TYPE_ARRAY_LOWER_BOUND_TYPE (new_type) = TYPE_ARRAY_LOWER_BOUND_TYPE (type);
+ if (TYPE_NAME (type))
+ TYPE_NAME (new_type) = xstrdup (TYPE_NAME (type));
+ if (TYPE_TAG_NAME (type))
+ TYPE_TAG_NAME (new_type) = xstrdup (TYPE_TAG_NAME (type));
+ TYPE_FLAGS (new_type) = TYPE_FLAGS (type);
+ TYPE_VPTR_FIELDNO (new_type) = TYPE_VPTR_FIELDNO (type);
+
+ TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);
+ TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
+
+ /* Copy the fields. */
+ TYPE_NFIELDS (new_type) = TYPE_NFIELDS (type);
+ if (TYPE_NFIELDS (type))
+ {
+ int i, nfields;
+
+ nfields = TYPE_NFIELDS (type);
+ TYPE_FIELDS (new_type) = xmalloc (sizeof (struct field) * nfields);
+ for (i = 0; i < nfields; i++)
+ {
+ TYPE_FIELD_ARTIFICIAL (new_type, i) = TYPE_FIELD_ARTIFICIAL (type, i);
+ TYPE_FIELD_BITSIZE (new_type, i) = TYPE_FIELD_BITSIZE (type, i);
+ if (TYPE_FIELD_TYPE (type, i))
+ TYPE_FIELD_TYPE (new_type, i)
+ = copy_type_recursive (objfile, TYPE_FIELD_TYPE (type, i),
+ copied_types);
+ if (TYPE_FIELD_NAME (type, i))
+ TYPE_FIELD_NAME (new_type, i) = xstrdup (TYPE_FIELD_NAME (type, i));
+ if (TYPE_FIELD_STATIC_HAS_ADDR (type, i))
+ SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i),
+ TYPE_FIELD_STATIC_PHYSADDR (type, i));
+ else if (TYPE_FIELD_STATIC (type, i))
+ SET_FIELD_PHYSNAME (TYPE_FIELD (new_type, i),
+ xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, i)));
+ else
+ {
+ TYPE_FIELD_BITPOS (new_type, i) = TYPE_FIELD_BITPOS (type, i);
+ TYPE_FIELD_STATIC_KIND (new_type, i) = 0;
+ }
+ }
+ }
+
+ /* Copy pointers to other types. */
+ if (TYPE_TARGET_TYPE (type))
+ TYPE_TARGET_TYPE (new_type) = copy_type_recursive (objfile,
+ TYPE_TARGET_TYPE (type),
+ copied_types);
+ if (TYPE_VPTR_BASETYPE (type))
+ TYPE_VPTR_BASETYPE (new_type) = copy_type_recursive (objfile,
+ TYPE_VPTR_BASETYPE (type),
+ copied_types);
+ /* Maybe copy the type_specific bits.
+
+ NOTE drow/2005-12-09: We do not copy the C++-specific bits like
+ base classes and methods. There's no fundamental reason why we
+ can't, but at the moment it is not needed. */
+
+ if (TYPE_CODE (type) == TYPE_CODE_FLT)
+ TYPE_FLOATFORMAT (new_type) == TYPE_FLOATFORMAT (type);
+ else if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (type) == TYPE_CODE_UNION
+ || TYPE_CODE (type) == TYPE_CODE_TEMPLATE
+ || TYPE_CODE (type) == TYPE_CODE_NAMESPACE)
+ INIT_CPLUS_SPECIFIC (new_type);
+
+ return new_type;
+}
+
static void
build_gdbtypes (void)
{
Index: src/gdb/gdbtypes.h
===================================================================
--- src.orig/gdb/gdbtypes.h 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/gdbtypes.h 2006-01-22 16:08:23.000000000 -0500
@@ -25,6 +25,8 @@
#if !defined (GDBTYPES_H)
#define GDBTYPES_H 1

+#include "hashtab.h"
+
/* Forward declarations for prototypes. */
struct field;
struct block;
@@ -1175,6 +1177,12 @@ extern struct type *builtin_type_f_void;
? obstack_alloc (&TYPE_OBJFILE (t) -> objfile_obstack, size) \
: xmalloc (size))

+#define TYPE_ZALLOC(t,size) \
+ (TYPE_OBJFILE (t) != NULL \
+ ? memset (obstack_alloc (&TYPE_OBJFILE (t)->objfile_obstack, size), \
+ 0, size) \
+ : xzalloc (size))
+
extern struct type *alloc_type (struct objfile *);

extern struct type *init_type (enum type_code, int, int, char *,
@@ -1364,4 +1372,10 @@ extern int is_integral_type (struct type

extern void maintenance_print_type (char *, int);

+extern htab_t create_copied_types_hash (struct objfile *objfile);
+
+extern struct type *copy_type_recursive (struct objfile *objfile,
+ struct type *type,
+ htab_t copied_types);
+
#endif /* GDBTYPES_H */
Index: src/gdb/objfiles.c
===================================================================
--- src.orig/gdb/objfiles.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/objfiles.c 2006-01-22 16:08:23.000000000 -0500
@@ -392,6 +392,10 @@ free_objfile (struct objfile *objfile)
objfile->separate_debug_objfile_backlink->separate_debug_objfile = NULL;
}

+ /* Remove any references to this objfile in the global value
+ lists. */
+ preserve_values (objfile);
+
/* First do any symbol file specific actions required when we are
finished with a particular symbol file. Note that if the objfile
is using reusable symbol information (via mmalloc) then each of
Index: src/gdb/symfile.c
===================================================================
--- src.orig/gdb/symfile.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/symfile.c 2006-01-22 16:08:23.000000000 -0500
@@ -2012,6 +2012,10 @@ reread_symbols (void)
memcpy (offsets, objfile->section_offsets,
SIZEOF_N_SECTION_OFFSETS (num_offsets));

+ /* Remove any references to this objfile in the global
+ value lists. */
+ preserve_values (objfile);
+
/* Nuke all the state that we will re-read. Much of the following
code which sets things to NULL really is necessary to tell
other parts of GDB that there is nothing currently there. */
@@ -2484,9 +2488,7 @@ clear_symtab_users (void)
breakpoint_re_set may try to access the current symtab. */
clear_current_source_symtab_and_line ();

- clear_value_history ();
clear_displays ();
- clear_internalvars ();
breakpoint_re_set ();
set_default_breakpoint (0, 0, 0, 0);
clear_pc_function_cache ();
Index: src/gdb/tracepoint.c
===================================================================
--- src.orig/gdb/tracepoint.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/tracepoint.c 2006-01-22 16:08:23.000000000 -0500
@@ -2694,11 +2694,6 @@ _initialize_tracepoint (void)
traceframe_number = -1;
tracepoint_number = -1;

- set_internalvar (lookup_internalvar ("tpnum"),
- value_from_longest (builtin_type_int, (LONGEST) 0));
- set_internalvar (lookup_internalvar ("trace_frame"),
- value_from_longest (builtin_type_int, (LONGEST) - 1));
-
if (tracepoint_list.list == NULL)
{
tracepoint_list.listsize = 128;
Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/utils.c 2006-01-22 16:08:23.000000000 -0500
@@ -53,6 +53,7 @@
#include "annotate.h"
#include "filenames.h"
#include "symfile.h"
+#include "gdb_obstack.h"

#include "inferior.h" /* for signed_pointer_to_address */

@@ -3124,3 +3125,26 @@ align_down (ULONGEST v, int n)
gdb_assert (n && (n & (n-1)) == 0);
return (v & -n);
}
+
+/* Allocation function for the libiberty hash table which uses an
+ obstack. The obstack is passed as DATA. */
+
+void *
+hashtab_obstack_allocate (void *data, size_t size, size_t count)
+{
+ unsigned int total = size * count;
+ void *ptr = obstack_alloc ((struct obstack *) data, total);
+ memset (ptr, 0, total);
+ return ptr;
+}
+
+/* Trivial deallocation function for the libiberty splay tree and hash
+ table - don't deallocate anything. Rely on later deletion of the
+ obstack. DATA will be the obstack, although it is not needed
+ here. */
+
+void
+dummy_obstack_deallocate (void *object, void *data)
+{
+ return;
+}
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/value.c 2006-01-22 16:11:55.000000000 -0500
@@ -653,29 +653,6 @@ access_value_history (int num)
return value_copy (chunk->values[absnum % VALUE_HISTORY_CHUNK]);
}

-/* Clear the value history entirely.
- Must be done when new symbol tables are loaded,
- because the type pointers become invalid. */
-
-void
-clear_value_history (void)
-{
- struct value_history_chunk *next;
- int i;
- struct value *val;
-
- while (value_history_chain)
- {
- for (i = 0; i < VALUE_HISTORY_CHUNK; i++)
- if ((val = value_history_chain->values[i]) != NULL)
- xfree (val);
- next = value_history_chain->next;
- xfree (value_history_chain);
- value_history_chain = next;
- }
- value_history_count = 0;
-}
-
static void
show_values (char *num_exp, int from_tty)
{
@@ -842,22 +819,49 @@ internalvar_name (struct internalvar *va
return var->name;
}

-/* Free all internalvars. Done when new symtabs are loaded,
- because that makes the values invalid. */
+/* Update VALUE before discarding OBJFILE. COPIED_TYPES is used to
+ prevent cycles / duplicates. */
+
+static void
+preserve_one_value (struct value *value, struct objfile *objfile,
+ htab_t copied_types)
+{
+ if (TYPE_OBJFILE (value->type) == objfile)
+ value->type = copy_type_recursive (objfile, value->type, copied_types);
+
+ if (TYPE_OBJFILE (value->enclosing_type) == objfile)
+ value->enclosing_type = copy_type_recursive (objfile,
+ value->enclosing_type,
+ copied_types);
+}
+
+/* Update the internal variables and value history when OBJFILE is
+ discarded; we must copy the types out of the objfile. New global types
+ will be created for every convenience variable which currently points to
+ this objfile's types, and the convenience variables will be adjusted to
+ use the new global types. */

void
-clear_internalvars (void)
+preserve_values (struct objfile *objfile)
{
+ htab_t copied_types;
+ struct value_history_chunk *cur;
struct internalvar *var;
+ int i;

- while (internalvars)
- {
- var = internalvars;
- internalvars = var->next;
- xfree (var->name);
- xfree (var->value);
- xfree (var);
- }
+ /* Create the hash table. We allocate on the objfile's obstack, since
+ it is soon to be deleted. */
+ copied_types = create_copied_types_hash (objfile);
+
+ for (cur = value_history_chain; cur; cur = cur->next)
+ for (i = 0; i < VALUE_HISTORY_CHUNK; i++)
+ if (cur->values[i])
+ preserve_one_value (cur->values[i], objfile, copied_types);
+
+ for (var = internalvars; var; var = var->next)
+ preserve_one_value (var->value, objfile, copied_types);
+
+ htab_delete (copied_types);
}

static void
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h 2006-01-22 16:06:58.000000000 -0500
+++ src/gdb/value.h 2006-01-22 16:08:23.000000000 -0500
@@ -502,9 +502,7 @@ extern void typedef_print (struct type *

extern char *internalvar_name (struct internalvar *var);

-extern void clear_value_history (void);
-
-extern void clear_internalvars (void);
+extern void preserve_values (struct objfile *);

/* From values.c */

Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo 2005-12-03 07:44:31.000000000 -0500
+++ src/gdb/doc/gdb.texinfo 2006-01-22 16:20:46.000000000 -0500
@@ -10821,11 +10821,11 @@ table and program to run from the same f
@code{symbol-file} with no argument clears out @value{GDBN} information on your
program's symbol table.

-The @code{symbol-file} command causes @value{GDBN} to forget the contents
-of its convenience variables, the value history, and all breakpoints and
-auto-display expressions. This is because they may contain pointers to
-the internal data recording symbols and data types, which are part of
-the old symbol table data being discarded inside @value{GDBN}.
+The @code{symbol-file} command causes @value{GDBN} to forget the contents of
+some breakpoints and auto-display expressions. This is because they may
+contain pointers to the internal data recording symbols and data types,
+which are part of the old symbol table data being discarded inside
+@value{GDBN}.

@code{symbol-file} does not repeat if you press @key{RET} again after
executing it once.