[PATCH] keep-variable command

[Andrew STUBBS <andrew dot stubbs at st dot com>]

Hi,

This patch adds a command 'keep-variable'. The command takes one single 
parameter - a convenience variable.

This variable is then protected from being deleted by the symbol-file 
command. In order to achieve this it must patch up the type information 
after the symbol table has changed. If the type cannot be fixed up then 
a warning is given and the value is lost. Lazy fixup means that this 
only happens if the variable is actually accessed.

If the keep-variable is never used then variables work exactly as before.

I feel certain that there is something somebody will not like about this 
implementation, but I hope we can sort it out.

The patch requires that my endian fixup patch has been applied first. 
The context in the patch assumes the init-if-undefined has been applied 
first, although the patch is not actually dependent.

Thanks

Andrew Stubbs
2005-11-16  Andrew Stubbs  <andrew.stubbs@st.com>

	* value.c: Include exceptions.h.
	(fixup_internalvar_endian): Don't fix the endian if the type
	needs fixing due to keep-variable.
	(keep_variable_command): New function.
	(fixup_internalvar_type_info): New function.
	(lookup_internalvar): Call fixup_internalvar_type_info() if needed.
	Initialise new 'temp' and 'type_needs_fixing' fields.
	(set_internalvar): Reset type_needs_fixing.
	(get_name_from_type): New function.
	(clear_internalvars): Rewrite to support keep-variable command.
	(show_convenience): Call fixup_internalvar_type_info() if needed.
	(_initialize_values): Add keep-variable command.
	* exec.c (exec_file_attach). Call clear_internalvars().
	* symfile.c (symbol_file_command). Call clear_internalvars().
	* value.h (struct internalvar): Add 'temp', 'type_needs_fixing',
	'type_name', and 'enclosing_type_name' fields.
	* Makefile.in (value.o): Add dependency on exceptions.h.

doc/
	* gdb.texinfo (Convenience variables): Add keep-variable command.

Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c	2005-11-07 18:27:03.000000000 +0000
+++ src/gdb/value.c	2005-11-07 18:27:04.000000000 +0000
@@ -37,6 +37,7 @@
 #include "gdb_assert.h"
 #include "regcache.h"
 #include "block.h"
+#include "exceptions.h"
 
 /* Prototypes for exported functions. */
 
@@ -779,10 +780,12 @@ fixup_internalvar_endian (struct interna
   gdb_byte temp;
   gdb_byte *array = value_contents_raw (var->value);
 
-  /* Don't do anything if the endian has not changed.
+  /* Don't do anything if the endian has not changed or if the type needs
+     fixing - the endian will be fixed with the type later.
      Also disregard FP types because they don't seem to vary with
      endian - at least, not on i686 Linux or sparc Solaris.  */
   if (var->endian != TARGET_BYTE_ORDER
+      && var->type_needs_fixing == 0
       && TYPE_CODE (value_type (var->value)) != TYPE_CODE_FLT)
     {
       /* Reverse the bytes.
@@ -811,6 +814,92 @@ fixup_all_internalvars_endian (void)
 }
 
 
+/* Ensure that the specified internal variable will not be destroyed
+   when new symtabs are loaded. The argument 'name', must
+   be an internal variable name, prefixed with a dollar character.  */
+static void
+keep_variable_command (char* name, int from_tty)
+{
+  struct internalvar* intvar;
+  char* str;
+
+  if (name == NULL || name[0] != '$')
+    {
+      printf_unfiltered( "usage: keep-variable $<var>\n" );
+      return;
+    }
+
+  str = ++name;            /* Remove $ character, i.e. "$var" ==> "var"  */
+  intvar = lookup_internalvar(str);
+  intvar->temp = 0;        /* Preserve variable by clearing 'temp' attribute */
+}
+
+/* If the symbol table is reloaded then permanent variables will lose
+   their type info. clear_internalvars() makes a note of the name of
+   any such types in the variable.
+   This function is called whenever the variable is next required to
+   get the new type info. It also fixes the endianess if required.  */
+
+static void
+fixup_internalvar_type_info (struct internalvar *var)
+{
+  int i, error=0;
+
+  /* For each in type and enclosing type ...  */
+  for (i=0; i<2; i++)
+    {
+      char *type_name = i==0 ? var->type_name
+			     : var->enclosing_type_name;
+      struct type **var_type = i==0 ? &var->value->type
+				    : &var->value->enclosing_type;
+
+      if (type_name != NULL)
+	{
+	  /* Get the typename using parse expression - it can cope
+	     with builtin types even when there is no symbol table.  */
+	  struct expression *expr = (struct expression *)
+	    catch_errors ((catch_errors_ftype *)parse_expression,
+			  type_name, NULL, RETURN_MASK_ALL);
+	  register struct cleanup *old_chain =
+	    make_cleanup (free_current_contents, &expr);
+
+	  if (expr != NULL && expr->nelts >= 2
+	      && expr->elts[0].opcode == OP_TYPE)
+	    /* Overwite the old (now broken) type with the shiny new one.  */
+	    *var_type = expr->elts[1].type;
+	  else
+	    error=1;
+
+	  do_cleanups (old_chain);
+	}
+    }
+
+
+  if (error)
+    {
+      /* Types no longer exist.
+	 The only thing to do is wipe the value.  */
+      xfree (var->value);
+      var->value = allocate_value (builtin_type_void);
+      release_value (var->value);
+
+      /* Tell the user we have had to do this.
+	 The message will occur when they are accessing the variable.  */
+      printf_unfiltered (
+	"The type of '$%s' was lost when the symbol table was updated.\n",
+	var->name);
+    }
+
+  /* Return the internal variable to its normal state.  */
+  xfree (var->type_name);
+  xfree (var->enclosing_type_name);
+  var->type_needs_fixing = 0;
+
+  /* Fix up the endianess.  */
+  fixup_internalvar_endian (var);
+}
+
+
 /* Look up an internal variable with name NAME.  NAME should not
    normally include a dollar sign.
 
@@ -824,12 +913,18 @@ lookup_internalvar (char *name)
 
   for (var = internalvars; var; var = var->next)
     if (strcmp (var->name, name) == 0)
-      return var;
+      {
+	if (var->type_needs_fixing)
+	  fixup_internalvar_type_info (var);
+	return var;
+      }
 
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
   var->endian = TARGET_BYTE_ORDER;
+  var->temp = 1;
+  var->type_needs_fixing = 0;
   release_value (var->value);
   var->next = internalvars;
   internalvars = var;
@@ -885,6 +980,7 @@ set_internalvar (struct internalvar *var
   xfree (var->value);
   var->value = newval;
   var->endian = TARGET_BYTE_ORDER;
+  var->type_needs_fixing = 0;
   release_value (newval);
   /* End code which must not call error().  */
 }
@@ -895,21 +991,136 @@ internalvar_name (struct internalvar *va
   return var->name;
 }
 
-/* Free all internalvars.  Done when new symtabs are loaded,
-   because that makes the values invalid.  */
+
+/* Get the typename from a struct type.
+   Returns the name of type as parse_expresion will understand.
+   Returns NULL if the type is unsupported.  */
+
+static char *
+get_name_from_type (const struct type *type)
+{
+  char *result;
+  char *prefix = NULL, pointers=0;
+
+  /* Follow the pointer types until the base type is discovered.  */
+  while (TYPE_CODE (type) == TYPE_CODE_PTR)
+    {
+      pointers++;
+      type = TYPE_TARGET_TYPE (type);
+    }
+
+  /* Find the appropriate string for the type.
+     If we can't find one then return NULL and fixup_internalvar_type_info()
+     will replace the variable with void.  If this is a problem then this
+     table will have to be extended to support the relavent type.  */
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_STRUCT:
+      prefix = "struct";
+      /* Fall through.  */
+    case TYPE_CODE_UNION:
+      if (prefix == NULL)
+	prefix = "union";
+      /* Fall through.  */
+    case TYPE_CODE_ENUM:
+      if (prefix == NULL)
+	prefix = "enum";
+
+      /* Note that these types do not have names, but do have tag names.  */
+
+      if (TYPE_TAG_NAME(type) == NULL)
+	return NULL;
+
+      result = xmalloc (strlen (prefix)
+			+ 1 /* space */
+			+ strlen (TYPE_TAG_NAME(type))
+			+ 1 /* space */
+			+ pointers
+			+ 1 /* terminator */);
+      sprintf( result, "%s %s ", prefix, TYPE_TAG_NAME(type));
+
+      break;
+    default:
+      /* Other types all (?) have names.  */
+
+      if ( TYPE_NAME(type) == NULL )
+	return NULL;
+
+      result = xmalloc (strlen (TYPE_NAME(type))
+			+ 1 /* space */
+			+ pointers
+			+ 1 /* terminator */);
+      sprintf (result, "%s ", TYPE_NAME(type));
+    }
+
+  /* Add stars for pointer types.  */
+  while ( pointers-- > 0 )
+    strcat (result, "*");
+
+  return result;
+}
+
+
+/* Traverse list of intervalvars.  If an internalvar's 'temp'
+   attribute is set, then free it - otherwise ignore it and check
+   the next member of the list.  Ensure that 'internalvars' points
+   to the head of the list.
+
+   Although the variables are kept, the type information is not.
+   Therefore take a note of the textual names.  The types will then
+   be patched up later when they are referenced.  The function that
+   does this is fixup_internalvars_type_info().
+
+   In order to take the notes BEFORE the types vanish a few extra
+   calls to this function have been inserted elsewhere.
+
+   The majority of internalvar's will be cleared when new symtabs are
+   loaded, because at this point they become invalid.  */
 
 void
 clear_internalvars (void)
 {
   struct internalvar *var;
+  struct internalvar *next, **prev;
 
-  while (internalvars)
+  var = internalvars;
+  prev = &internalvars;
+
+  while (var)
     {
-      var = internalvars;
-      internalvars = var->next;
-      xfree (var->name);
-      xfree (var->value);
-      xfree (var);
+      next = var->next;
+
+      /* Is this a temporary variable? If so - free it.  */
+      if (var->temp)
+	{
+	  /* Move the next variable up the list - we may want to keep it.  */
+	  *prev = next;
+
+	  xfree (var->name);
+	  xfree (var->value);
+	  xfree (var);
+	}
+      else
+	{
+	  /* This is a permanent variable so it will not be removed from
+	     the list.  Mark it as the current tail of the list.  */
+	  prev = &var->next;
+
+	  /* Note down the values of the types.
+	     This ensures that they can be fixed up after the symbol
+	     tables are rebuilt.
+	     Don't do it twice - the lazy fixup means it may not have been
+	     fixed since the last time we were here.  */
+	  if ( var->type_needs_fixing == 0 )
+	    {
+	      var->type_needs_fixing = 1;
+	      var->type_name = get_name_from_type (var->value->type);
+	      var->enclosing_type_name =
+		get_name_from_type (var->value->enclosing_type);
+	    }
+	}
+
+      var = next;
     }
 }
 
@@ -921,6 +1132,9 @@ show_convenience (char *ignore, int from
 
   for (var = internalvars; var; var = var->next)
     {
+      if (var->type_needs_fixing)
+	fixup_internalvar_type_info (var);
+
       if (!varseen)
 	{
 	  varseen = 1;
@@ -1730,4 +1944,7 @@ A few convenience variables are given va
 init-if-undefined <var> = <expr>\n\
 Ensure that an internal variable exists and set it to\n\
 a given value if it does not."));
+
+  add_com("keep-variable", class_vars, keep_variable_command,
+	  _("Ensures that specified internal variable will not be removed"));
 }
Index: src/gdb/exec.c
===================================================================
--- src.orig/gdb/exec.c	2005-11-07 17:22:56.000000000 +0000
+++ src/gdb/exec.c	2005-11-07 18:27:04.000000000 +0000
@@ -266,6 +266,11 @@ exec_file_attach (char *filename, int fr
 
       validate_files ();
 
+      /* Delete convenience variables BEFORE their types become invalid.
+	 This way a note can be made of what the types of the permanent
+	 variables were and they can recover later. */
+      clear_internalvars ();
+
       set_gdbarch_from_file (exec_bfd);
 
       push_target (&exec_ops);
Index: src/gdb/symfile.c
===================================================================
--- src.orig/gdb/symfile.c	2005-11-07 17:22:56.000000000 +0000
+++ src/gdb/symfile.c	2005-11-07 18:27:04.000000000 +0000
@@ -1268,6 +1268,11 @@ symbol_file_command (char *args, int fro
 {
   dont_repeat ();
 
+  /* Delete convenience variables BEFORE their types become invalid.
+     This way a note can be made of what the types of the permanent
+     variables were and they can recover later. */
+  clear_internalvars ();
+
   if (args == NULL)
     {
       symbol_file_clear (from_tty);
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h	2005-11-07 18:27:03.000000000 +0000
+++ src/gdb/value.h	2005-11-07 18:27:04.000000000 +0000
@@ -246,6 +246,15 @@ struct internalvar
   char *name;
   struct value *value;
   int endian;
+
+  /* 'temp' determines whether the variable will be destroyed when
+     new symtabs are loaded.  */
+  int temp;
+  /* These are used to fixup the type pointers after the symbol tables
+     are reloaded.  */
+  char type_needs_fixing;
+  char *type_name;
+  char *enclosing_type_name;
 };
 
 
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2005-11-07 18:27:03.000000000 +0000
+++ src/gdb/Makefile.in	2005-11-07 18:27:04.000000000 +0000
@@ -2735,7 +2735,7 @@ valprint.o: valprint.c $(defs_h) $(gdb_s
 value.o: value.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(value_h) $(gdbcore_h) $(command_h) $(gdbcmd_h) $(target_h) \
 	$(language_h) $(scm_lang_h) $(demangle_h) $(doublest_h) \
-	$(gdb_assert_h) $(regcache_h) $(block_h)
+	$(gdb_assert_h) $(regcache_h) $(block_h) $(exceptions_h)
 varobj.o: varobj.c $(defs_h) $(value_h) $(expression_h) $(frame_h) \
 	$(language_h) $(wrapper_h) $(gdbcmd_h) $(gdb_string_h) $(varobj_h)
 vaxbsd-nat.o: vaxbsd-nat.c $(defs_h) $(inferior_h) $(regcache_h) $(target_h) \
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2005-11-07 18:27:03.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2005-11-07 19:01:11.000000000 +0000
@@ -6120,6 +6120,11 @@ Using a convenience variable for the fir
 value is @code{void} until you assign a new value.  You can alter the
 value with another assignment at any time.
 
+@value{GDBN} commands that wipe the symbol table, such as @samp{file} and
+@samp{symbol-file}, cause all convenience variables to be deleted - their
+types are lost so their value would become meaningless.  This can be avoided
+using the @samp{keep-variable} command below.
+
 Convenience variables have no fixed types.  You can assign a convenience
 variable any type of value, including structures and arrays, even if
 that variable already has a value of a different type.  The convenience
@@ -6143,6 +6148,24 @@ override default values used in a comman
 
 If the variable is already defined then the expression is not evaluated so
 any side-effects do not occur.
+
+@kindex keep-variable
+@cindex convenience variables, keeping
+@item keep-variable $@var{variable}
+Mark the variable to be preserved across commands which destroy the symbol
+table, such as @samp{symbol-file}.
+
+The next time the variable is accessed (by whatever means) @value{GDBN} will
+attempt to find a type, in the current symbol table, with the same name as
+the type used previously.  It does not check whether the new type is in any
+way compatible with the old type.
+
+If a suitable type does not exist (at the time the variable is accessed) then
+@value{GDBN} will print a message and reset the value to @code{void}.
+
+If the symbol table is destroyed and recreated multiple times, the content
+of the intermediate tables has no effect, @emph{provided that the variable
+is not accessed while they are loaded}.
 @end table
 
 One of the ways to use a convenience variable is as a counter to be