[PATCH] keeping convenience variables (take 2)

[Andrew STUBBS <andrew.stubbs@st.com>]

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

Hi all,

Attached is a new attempt at keeping convenience variables over 
symbol-file commands.

This time the 'keep-variable' command has been ditched in favour of 
always attempting to keep all variables.

Additionally, it no longer throws away the value if it cannot find a 
suitable type. Instead it keeps the value stored away and reinstates it 
if a suitable type reappears at some point in the future. Note that here 
'suitable' means 'has the same name' - it does not try to check if the 
types are in any way compatible.

While the type does not exist, the value is 'void'. 'show convenience' 
will annotate this value with '<type 'whatever' unknown>'. I would 
prefer that this were the case when the value is printed with 'print' 
also, but that would require changing all language val_print functions 
(???) and need some sort of scheme for passing the information to those 
functions (they only see the type and the raw contents, not the struct 
value).

If this sort of annotation is appropriate I would do something similar 
for endian-challenged variables.

When it does not find a type, parse_expression() prints a diagnostic 
message. If possible I would like to suppress this, but I am not sure 
what the best approach to this might be. Any suggestions?

Note that this patch is intended to be applied on top of my endian fixup 
patch posted recently.

Thanks

Andrew Stubbs

[-- Attachment #2: keepvariable-2.patch --]
[-- Type: text/plain, Size: 14757 bytes --]

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

	* value.c: Include exceptions.h.
	(fixup_internalvar_endian): Don't fix the endian if the type
	needs fixing.
	(fixup_internalvar_type_info): New function.
	(lookup_internalvar): Initialise new 'saved_value' and
	'type_needs_fixing' fields.
	(value_of_internalvar): Call fixup_internalvar_type_info() if needed.
	(set_internalvar_component): Likewise.
	(set_internalvar): Reset type_needs_fixing and saved_value.
	(get_name_from_type): New function.
	(clear_internalvars): Rename to ...
	(save_internalvars): ... this and rewrite to keep type information.
	(show_convenience): Call fixup_internalvar_type_info() if needed.
	* exec.c (exec_file_attach). Call save_internalvars().
	* symfile.c (symbol_file_command). Call save_internalvars().
	(clear_symtab_users): Change clear_internalvars to save_internalvars.
	* value.h (struct internalvar): Add 'type_needs_fixing', 'type_name',
	'enclosing_type_name', and 'saved_value' fields.
	(clear_internalvars): Rename to ...
	(save_internalvars): ... this.
	* Makefile.in (value.o): Add dependency on exceptions.h.

doc/
	* gdb.texinfo (Convenience variables): Mention preservation of types.

testsuite/
	* default.exp (test show convenience): Update.

Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c	2005-11-18 17:02:35.000000000 +0000
+++ src/gdb/value.c	2005-11-21 14:36:40.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,65 @@ fixup_all_internalvars_endian (void)
 }
 
 
+/* If the symbol table is reloaded then many variables will lose
+   their type info. save_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 && !error; i++)
+    {
+      char *type_name = i==0 ? var->type_name
+			     : var->enclosing_type_name;
+      struct type **var_type = i==0 ? &var->saved_value->type
+				    : &var->saved_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.
+       Leave it unfixed - the type may come back later.  */
+    return;
+
+  /* Return the internal variable to its normal state.  */
+  xfree (var->type_name);
+  xfree (var->enclosing_type_name);
+  var->type_needs_fixing = 0;
+  xfree (var->value);
+  var->value = var->saved_value;
+  var->saved_value = NULL;
+
+  /* Fix up the endianess.  */
+  fixup_internalvar_endian (var);
+}
+
+
 /* Look up an internal variable with name NAME.  NAME should not
    normally include a dollar sign.
 
@@ -830,6 +892,8 @@ lookup_internalvar (char *name)
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
   var->endian = TARGET_BYTE_ORDER;
+  var->type_needs_fixing = 0;
+  var->saved_value = NULL;
   release_value (var->value);
   var->next = internalvars;
   internalvars = var;
@@ -841,6 +905,9 @@ value_of_internalvar (struct internalvar
 {
   struct value *val;
 
+  if (var->type_needs_fixing)
+    fixup_internalvar_type_info (var);
+
   val = value_copy (var->value);
   if (value_lazy (val))
     value_fetch_lazy (val);
@@ -853,7 +920,12 @@ void
 set_internalvar_component (struct internalvar *var, int offset, int bitpos,
 			   int bitsize, struct value *newval)
 {
-  gdb_byte *addr = value_contents_writeable (var->value) + offset;
+  gdb_byte *addr;
+
+  if (var->type_needs_fixing)
+    fixup_internalvar_type_info (var);
+
+  addr = value_contents_writeable (var->value) + offset;
 
   if (bitsize)
     modify_field (addr, value_as_long (newval),
@@ -885,6 +957,12 @@ set_internalvar (struct internalvar *var
   xfree (var->value);
   var->value = newval;
   var->endian = TARGET_BYTE_ORDER;
+  var->type_needs_fixing = 0;
+
+  /* If there was a saved value then it is no longer valid.  */
+  xfree (var->saved_value);
+  var->saved_value = NULL;
+
   release_value (newval);
   /* End code which must not call error().  */
 }
@@ -895,21 +973,114 @@ 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 and save the types for later.
+
+   All variables whose types do not belong to an object file are left alone.
+
+   Otherwise steps must be taken to survive the loss of the type information.
+
+   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().
+
+   This function must be called BEFORE the type information is deleted.  */
 
 void
-clear_internalvars (void)
+save_internalvars (void)
 {
   struct internalvar *var;
 
-  while (internalvars)
+  for (var = internalvars; var; var = var->next)
     {
-      var = internalvars;
-      internalvars = var->next;
-      xfree (var->name);
-      xfree (var->value);
-      xfree (var);
+      /* Check if this symbol's type is from an object file.
+	 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
+	  && TYPE_OBJFILE (value_enclosing_type (var->value)))
+	{
+	  /* Note down the values of the types.
+	     This ensures that they can be fixed up after the symbol
+	     tables are rebuilt.  */
+	  var->type_needs_fixing = 1;
+	  var->type_name = get_name_from_type (value_type (var->value));
+	  var->enclosing_type_name =
+		get_name_from_type (value_enclosing_type (var->value));
+	  var->saved_value = var->value;
+
+	  /* Replace the value with void until it is fixed.  */
+	  var->value = allocate_value (builtin_type_void);
+	  release_value (var->value);
+	}
     }
 }
 
@@ -921,12 +1092,17 @@ 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;
 	}
       printf_filtered (("$%s = "), var->name);
       value_print (var->value, gdb_stdout, 0, Val_pretty_default);
+      if (var->type_needs_fixing)
+	printf_filtered (_(" <type '%s' unknown>"), var->enclosing_type_name);
       printf_filtered (("\n"));
     }
   if (!varseen)
Index: src/gdb/exec.c
===================================================================
--- src.orig/gdb/exec.c	2005-11-18 16:59:35.000000000 +0000
+++ src/gdb/exec.c	2005-11-18 17:33:47.000000000 +0000
@@ -266,6 +266,11 @@ exec_file_attach (char *filename, int fr
 
       validate_files ();
 
+      /* Save the convenience variables BEFORE their types become invalid.
+	 A note can be made of what the types of the variables were and
+	 they can be recovered later.  */
+      save_internalvars ();
+
       set_gdbarch_from_file (exec_bfd);
 
       push_target (&exec_ops);
Index: src/gdb/symfile.c
===================================================================
--- src.orig/gdb/symfile.c	2005-11-18 16:59:35.000000000 +0000
+++ src/gdb/symfile.c	2005-11-18 17:47:07.000000000 +0000
@@ -1268,6 +1268,11 @@ symbol_file_command (char *args, int fro
 {
   dont_repeat ();
 
+  /* Save the convenience variables BEFORE their types become invalid.
+     A note can be made of what the types of the variables were and
+     they can be recovered later.  */
+  save_internalvars ();
+
   if (args == NULL)
     {
       symbol_file_clear (from_tty);
@@ -2484,7 +2489,7 @@ clear_symtab_users (void)
 
   clear_value_history ();
   clear_displays ();
-  clear_internalvars ();
+  save_internalvars ();
   breakpoint_re_set ();
   set_default_breakpoint (0, 0, 0, 0);
   clear_pc_function_cache ();
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h	2005-11-18 17:02:35.000000000 +0000
+++ src/gdb/value.h	2005-11-18 18:18:48.000000000 +0000
@@ -246,6 +246,13 @@ struct internalvar
   char *name;
   struct value *value;
   int endian;
+
+  /* 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;
+  struct value *saved_value;
 };
 
 \f
@@ -507,7 +514,7 @@ extern char *internalvar_name (struct in
 
 extern void clear_value_history (void);
 
-extern void clear_internalvars (void);
+extern void save_internalvars (void);
 
 /* From values.c */
 
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2005-11-18 17:02:35.000000000 +0000
+++ src/gdb/Makefile.in	2005-11-18 17:02:36.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-18 17:02:34.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2005-11-21 14:39:00.000000000 +0000
@@ -6125,6 +6125,15 @@ variable any type of value, including st
 that variable already has a value of a different type.  The convenience
 variable, when used as an expression, has the type of its current value.
 
+@value{GDBN} commands that wipe the symbol table, such as @samp{file} and
+@samp{symbol-file}, cause problems for convenience variables---their types
+may be lost so their values may become meaningless.  @value{GDBN} tries to
+avoid this by selecting a type from the new symbol table (if any).  If a
+suitable type does not exist (at the time the variable is accessed) then
+@value{GDBN} will show the value as @code{void} until the type becomes
+available once more.  The @samp{show convenience} command will tag these
+variables with @samp{<type '...' unknown>}.
+
 @table @code
 @kindex show convenience
 @cindex show all user variables
Index: src/gdb/testsuite/gdb.base/default.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/default.exp	2005-07-14 15:49:23.000000000 +0100
+++ src/gdb/testsuite/gdb.base/default.exp	2005-11-21 14:00:50.000000000 +0000
@@ -602,7 +602,7 @@ gdb_test "show complaints" "Max number o
 #test show confirm
 gdb_test "show confirm" "Whether to confirm potentially dangerous operations is o\[a-z\]*." "show confirm"
 #test show convenience
-gdb_test "show convenience" "No debugger convenience variables now defined.(\[^\r\n\]*\[\r\n\])+Convenience variables have names starting with \".\";(\[^\r\n\]*\[\r\n\])+use \"set\" as in \"set .foo = 5\" to define them." "show convenience"
+gdb_test "show convenience" ".trace_frame = -1(\[^\r\n\]*\[\r\n\])+.tpnum = 0(\[^\r\n\]*\[\r\n\])+" "show convenience"
 #test show directories
 gdb_test "show directories" "Source directories searched: .cdir:.cwd" "show directories"
 #test show editing