rfc/wip $gdbframe

[Andrew Cagney <ac131313 at cygnus dot com>]

Hello,

Ref:
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=327
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=251

The attached patch is work in progress on ``The $fp problem''.

It creates a new register $gdbframe that sits above the range of target 
registers [0 .. NUM_REGS+NUM_PSEUDO_REGS).  This register than contains 
the frame.base.  While the basic mechanism appears to be working, this 
change does raise a few interesting questions.

--

The name: $fp, $frame, $gdbframe, $gdb.frame, $gdb_frame

Some targets have a real $fp.  Since the real $fp takes precidence, such 
targets wouldn't have access to the builtin register.  $frame is likely 
similar (true?).  Hence a $gdb prefix is included.

I have a preference for $gdbframe over $gdb_frame as it uses less shift 
keys (I can be convinced of otherwize :-).

$gdb.frame would be good.  That way everything is in a single variable. 
  It has other issues (below) though.

--

Target access?

At present $gdbframe makes use of information found in 
``selected_frame''.  Since that information is local to GDB, a reference 
to $frame doesn't involve additional querying of the target (it is cheap).

$gdbframe could, however, be extended to display things like the values 
of saved registers and the like.  Since that would require accessing 
target info it would be more expensive.

My feeling is that, as a policy, $gdbframe should not introduce 
additional target accesses keeping it light weight.

--

$gdb.frame, $gdb.locals, $gdb.... VS $gdbframe, $gdblocals, ...

An idea is to have $gdb have multiple fields: $gdb.frame $gdb.locals 
$gdb... While it looks good, I suspect it might suffer from the same 
problems as ``target access'' above.

The $gdb variable would need to be completly initialized in a single 
hit.  By having separate variables, they can each have their own policy 
- $gdblocals might be expensive but $gdbframe is cheap.

If someone knows how to do this without the need to initialize all 
fields, I'm all ears!

--

Thoughts?

Andrew






The gdb prefix is to, hopefully, stop any conflicts with existing variables.
I have a dislike of the shift key so prefer $gdbframe over $gdb_frame 
(but I can be convinced otherwise :-).

--

2002-02-09  Andrew Cagney  <ac131313@redhat.com>

	* findvar.c: Include "builtin-regs.h".
	(value_of_register): Call value_of_builtin_reg when applicable.

	* parse.c: Include "builtin-regs.h" and "gdb_assert.h".
	(target_map_name_to_register): Call
	builtin_reg_map_name_to_regnum.
	
	* Makefile.in (SFILES): Add builtin-regs.c.
	(COMMON_OBS): Add builtin-regs.o.
	(builtin_regs_h): Define.
	(builtin-regs.o): New target.
	(findvar.o): Add $(builtin_regs_h).
	
	* builtin-regs.c, builtin-regs.h: New files.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.156
diff -p -r1.156 Makefile.in
*** Makefile.in	2002/02/09 18:45:06	1.156
--- Makefile.in	2002/02/09 22:30:07
*************** SFILES = ax-general.c ax-gdb.c bcache.c 
*** 548,553 ****
--- 548,554 ----
  	tui/tui-file.h tui/tui-file.c tui/tui-out.c tui/tui-hooks.c \
  	ui-file.h ui-file.c \
  	frame.c doublest.c \
+ 	builtin-regs.c \
  	gnu-v2-abi.c gnu-v3-abi.c hpacc-abi.c cp-abi.c
  
  LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
*************** annotate_h =	annotate.h $(symtab_h) $(gd
*** 587,592 ****
--- 588,594 ----
  arch_utils_h =	arch-utils.h
  ax_h = 		ax.h $(doublest_h)
  bcache_h =	bcache.h
+ builtin_regs_h = builtin-regs.h
  breakpoint_h =	breakpoint.h $(frame_h) $(value_h)
  buildsym_h =	buildsym.h
  c_lang_h =	c-lang.h $(value_h)
*************** COMMON_OBS = version.o blockframe.o brea
*** 705,710 ****
--- 707,713 ----
  	event-loop.o event-top.o inf-loop.o completer.o \
  	gdbarch.o arch-utils.o gdbtypes.o copying.o $(DEPFILES) \
  	memattr.o mem-break.o target.o parse.o language.o $(YYOBJ) buildsym.o \
+ 	builtin-regs.o \
  	signals.o \
  	kod.o kod-cisco.o \
  	gdb-events.o \
*************** breakpoint.o: breakpoint.c $(defs_h) $(g
*** 1262,1267 ****
--- 1265,1273 ----
  buildsym.o: buildsym.c $(bfd_h) $(buildsym_h) $(complaints_h) $(defs_h) \
  	$(objfiles_h) $(symfile_h) $(symtab_h) $(gdb_string_h)
  
+ builtin-regs.o: builtin-regs.c $(defs.h) $(builtin_regs_h) $(gdbtypes_h) \
+ 	$(gdb_string_h) $(value_h) $(frame_h)
+ 
  c-lang.o: c-lang.c $(c_lang_h) $(defs_h) $(expression_h) $(gdbtypes_h) \
  	$(language_h) $(parser_defs_h) $(symtab_h)
  
*************** expprint.o: expprint.c $(defs_h) $(expre
*** 1396,1402 ****
  	$(language_h) $(parser_defs_h) $(symtab_h) $(value_h)
  
  findvar.o: findvar.c $(defs_h) $(gdbcore_h) $(inferior_h) $(target_h) \
! 	$(gdb_string_h) $(regcache_h)
  
  frame.o: frame.c $(defs_h) $(frame_h) $(target_h) $(value_h) $(inferior_h) \
  	$(regcache_h)
--- 1402,1408 ----
  	$(language_h) $(parser_defs_h) $(symtab_h) $(value_h)
  
  findvar.o: findvar.c $(defs_h) $(gdbcore_h) $(inferior_h) $(target_h) \
! 	$(gdb_string_h) $(regcache_h) $(builtin_regs_h)
  
  frame.o: frame.c $(defs_h) $(frame_h) $(target_h) $(value_h) $(inferior_h) \
  	$(regcache_h)
Index: builtin-regs.c
===================================================================
RCS file: builtin-regs.c
diff -N builtin-regs.c
*** /dev/null	Tue May  5 13:32:27 1998
--- builtin-regs.c	Sat Feb  9 14:30:07 2002
***************
*** 0 ****
--- 1,142 ----
+ /* Builtin registers, for GDB, the GNU debugger.
+ 
+    Copyright 2002 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 2 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, write to the Free Software
+    Foundation, Inc., 59 Temple Place - Suite 330,
+    Boston, MA 02111-1307, USA.  */
+ 
+ #include "defs.h"
+ #include "builtin-regs.h"
+ #include "gdbtypes.h"
+ #include "gdb_string.h"
+ #include "value.h"
+ #include "frame.h"
+ 
+ /* Helper function.  Create an empty struct type.  */
+ 
+ static struct type *
+ init_struct_type (char *name)
+ {
+   struct type *t = init_type (TYPE_CODE_STRUCT, 0, 0, NULL, NULL);
+   TYPE_TAG_NAME (t) = name;
+   return t;
+ }
+ 
+ /* Helper function.  Append a field to a structure assuming it the
+    structure is packed.  */
+ 
+ static void
+ append_struct_type_field (struct type *t, char *name, struct type *field)
+ {
+   struct field *f;
+   TYPE_NFIELDS (t) = TYPE_NFIELDS (t) + 1;
+   TYPE_LENGTH (t) = TYPE_LENGTH (t) + TYPE_LENGTH (field);
+   TYPE_FIELDS (t) = xrealloc (TYPE_FIELDS (t),
+ 			      sizeof (struct field) * TYPE_NFIELDS (t));
+   f = &(TYPE_FIELDS (t)[TYPE_NFIELDS (t) - 1]);
+   memset (f, 0, sizeof (*f));
+   FIELD_TYPE (f[0]) = field;
+   FIELD_NAME (f[0]) = name;
+   if (TYPE_NFIELDS (t) > 1)
+     FIELD_BITPOS (f[0]) = (FIELD_BITPOS (f[-1])
+ 			   + (TYPE_LENGTH (FIELD_TYPE (f[-1]))
+ 			      * TARGET_CHAR_BIT));
+ }
+ 
+ /* Types that describe the various builtin registers.  */
+ 
+ static struct type *builtin_type_gdbframe;
+ 
+ /* Constructors for those types.  */
+ 
+ static void
+ build_builtin_reg_types (void)
+ {
+   /* $gdbframe.  */
+   if (builtin_type_gdbframe == NULL)
+     {
+ #if 0
+       struct gdbframe
+       {
+ 	void *base;
+       };
+ #endif
+       builtin_type_gdbframe = init_struct_type ("gdbframe");
+       append_struct_type_field (builtin_type_gdbframe,
+ 				"base", builtin_type_void_data_ptr);
+     }
+ }
+ 
+ void
+ _initialize_builtin_reg_types (void)
+ {
+   /* FIXME: cagney/2002-02-08: At present the local builtin types
+      can't be initialized using _initialize*() or gdbarch.  Due mainly
+      to non-multi-arch targets, GDB initializes things piece meal and,
+      as a consequence can leave these types NULL.  */
+   REGISTER_GDBARCH_SWAP (builtin_type_gdbframe);
+ }
+ 
+ 
+ /* Implement builtin register types.  Builtin registers have regnum's
+    that live above of the range [0 .. NUM_REGS+NUM_PSEUDO_REGS) which
+    is controlled by the target.  The target should never see those
+    regnums.  */
+ 
+ enum builtin_reg
+ {
+   BUILTIN_REG_GDBFRAME
+ };
+ 
+ int
+ builtin_reg_map_name_to_regnum (const char *name, int len)
+ {
+   int i;
+   /* FIXME: This is a little generous.  $gdb matches.  */
+   if (strncmp (name, "gdbframe", len) == 0)
+     i = BUILTIN_REG_GDBFRAME;
+   else
+     return -1;
+   return i + NUM_REGS + NUM_PSEUDO_REGS;
+ }
+ 
+ struct value *
+ value_of_builtin_reg (int regnum,
+ 		      struct frame_info *frame)
+ {
+   struct value *val;
+   build_builtin_reg_types ();
+   switch ((enum builtin_reg) (regnum - NUM_REGS - NUM_PSEUDO_REGS))
+     {
+     case BUILTIN_REG_GDBFRAME:
+       {
+ 	char *buf;
+ 	val = allocate_value (builtin_type_gdbframe);
+ 	VALUE_LVAL (val) = not_lval;
+ 	buf = VALUE_CONTENTS_RAW (val);
+ 	memset (buf, TYPE_LENGTH (VALUE_TYPE (val)), 0);
+ 	/* gdb.frame.base.  */
+ 	if (frame != NULL)
+ 	  ADDRESS_TO_POINTER (builtin_type_void_data_ptr, buf, frame->frame);
+ 	buf += TYPE_LENGTH (builtin_type_void_data_ptr);
+ 	break;
+       }
+     default:
+       internal_error (__FILE__, __LINE__, "bad switch");
+     }
+   return val;
+ }
Index: builtin-regs.h
===================================================================
RCS file: builtin-regs.h
diff -N builtin-regs.h
*** /dev/null	Tue May  5 13:32:27 1998
--- builtin-regs.h	Sat Feb  9 14:30:07 2002
***************
*** 0 ****
--- 1,29 ----
+ /* Builtin registers, for GDB, the GNU debugger.
+ 
+    Copyright 2002 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 2 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, write to the Free Software
+    Foundation, Inc., 59 Temple Place - Suite 330,
+    Boston, MA 02111-1307, USA.  */
+ 
+ #ifndef BUILTIN_REGS_H
+ #define BUILTIN_REGS_H
+ 
+ int builtin_reg_map_name_to_regnum (const char *str, int len);
+ 
+ struct value *value_of_builtin_reg (int regnum, struct frame_info *frame);
+ 
+ #endif
Index: findvar.c
===================================================================
RCS file: /cvs/src/src/gdb/findvar.c,v
retrieving revision 1.27
diff -p -r1.27 findvar.c
*** findvar.c	2002/01/05 04:30:17	1.27
--- findvar.c	2002/02/09 22:30:07
***************
*** 33,38 ****
--- 33,39 ----
  #include "floatformat.h"
  #include "symfile.h"		/* for overlay functions */
  #include "regcache.h"
+ #include "builtin-regs.h"
  
  /* This is used to indicate that we don't know the format of the floating point
     number.  Typically, this is useful for native ports, where the actual format
*************** value_of_register (int regnum)
*** 304,309 ****
--- 305,315 ----
    struct value *reg_val;
    char *raw_buffer = (char*) alloca (MAX_REGISTER_RAW_SIZE);
    enum lval_type lval;
+ 
+   /* Builtin registers lie completly outside of the range of normal
+      registers.  Catch them early so that the target never sees them.  */
+   if (regnum >= NUM_REGS + NUM_PSEUDO_REGS)
+     return value_of_builtin_reg (regnum, selected_frame);
  
    get_saved_register (raw_buffer, &optim, &addr,
  		      selected_frame, regnum, &lval);
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.19
diff -p -r1.19 parse.c
*** parse.c	2002/01/31 02:13:56	1.19
--- parse.c	2002/02/09 22:30:07
***************
*** 47,52 ****
--- 47,54 ----
  #include "inferior.h"		/* for NUM_PSEUDO_REGS.  NOTE: replace 
  				   with "gdbarch.h" when appropriate.  */
  #include "doublest.h"
+ #include "builtin-regs.h"
+ #include "gdb_assert.h"
  
  
  /* Symbols which architectures can redefine.  */
*************** target_map_name_to_register (char *str, 
*** 132,137 ****
--- 134,147 ----
        {
  	return std_regs[i].regnum;
        }
+ 
+   /* Try builtin registers.  */
+   i = builtin_reg_map_name_to_regnum (str, len);
+   if (i >= 0)
+     {
+       gdb_assert (i >= NUM_REGS + NUM_PSEUDO_REGS);
+       return i;
+     }
  
    return -1;
  }