This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rfc] Record register names for OP_REGISTER


As I just wrote, I'm about to submit target-described register support
for MIPS.  Testing it turned up a lurking problem in expression
handling.  Suppose you write "display/i $pc", and then connect to a
target which informs GDB of its architecture.  GDB switches to the new
architecture, but the expression for "$pc" has already been parsed,
and OP_REGISTER takes a register number - which only has meaning for a
particular architecture.

You can reproduce this by hand.  I had to use "set architecture",
because "file" (my first try) deletes auto-display expressions for
some reason - I'm not sure why.  Like so:

(gdb) set architecture i386
The target architecture is assumed to be i386
(gdb) display/i $pc
(gdb) set architecture auto
The target architecture is set automatically (currently i386:x86-64)

(gdb) start
Breakpoint 1 at 0x44a9c0: file /space/fsf/commit/src/gdb/gdb.c, line 28.
Starting program: /space/fsf/x86-64/commit-gdb/gdb/gdb
[Thread debugging using libthread_db enabled]
[New Thread 0x2aaaab7ddd00 (LWP 1827)]
[Switching to Thread 0x2aaaab7ddd00 (LWP 1827)]
main (argc=1, argv=0x7fffffffe178) at /space/fsf/commit/src/gdb/gdb.c:28
28      {
Disabling display 1 to avoid infinite recursion.
1: x/i $xmm11  Value can't be converted to integer.

Whoops!  We aren't interested in $xmm11, we're interested in $pc.
xmm11 is register 51 in the x86-64 port; the i386 port only has
registers 0-49, and the "user register" allocated for $pc comes after
that.

I think it's reasonable to save the register name instead.  That fixes
the above, and several test failures for MIPS, where adding a new raw
register similarly bumps up the register number for $pc.

Any objections to this patch?  Otherwise, I'll check it in along with
the rest of MIPS target register support.

-- 
Daniel Jacobowitz
CodeSourcery

2007-05-21  Daniel Jacobowitz  <dan@codesourcery.com>

	* expression.h (enum exp_opcode): Document a register name for
	OP_REGISTER.
	* parse.c (write_dollar_variable): Write the register name for
	OP_REGISTER.
	(operator_length_standard): Expect the register name following
	OP_REGISTER.
	* ada-lang.c (resolve_subexp): Likewise.
	* ax-gdb.c (gen_expr): Likewise.
	* eval.c (evaluate_subexp_standard): Likewise.
	* expprint.c (print_subexp_standard, dump_subexp_body_standard):
	Likewise.
	* tracepoint.c (encode_actions): Likewise.

---
 ada-lang.c   |    5 ++++-
 ax-gdb.c     |   10 ++++++++--
 eval.c       |   13 +++++++++----
 expprint.c   |   11 ++++-------
 expression.h |    5 ++---
 parse.c      |    6 ++++--
 tracepoint.c |   19 ++++++++++++++-----
 7 files changed, 45 insertions(+), 24 deletions(-)

Index: gdb/ada-lang.c
===================================================================
--- gdb.orig/ada-lang.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/ada-lang.c	2007-05-21 09:05:33.000000000 -0400
@@ -2710,7 +2710,6 @@ resolve_subexp (struct expression **expp
     case OP_TYPE:
     case OP_BOOL:
     case OP_LAST:
-    case OP_REGISTER:
     case OP_INTERNALVAR:
       *pos += 3;
       break;
@@ -2720,6 +2719,10 @@ resolve_subexp (struct expression **expp
       nargs = 1;
       break;
 
+    case OP_REGISTER:
+      *pos += 4 + BYTES_TO_EXP_ELEM (exp->elts[pc + 1].longconst + 1);
+      break;
+
     case STRUCTOP_STRUCT:
       *pos += 4 + BYTES_TO_EXP_ELEM (exp->elts[pc + 1].longconst + 1);
       nargs = 1;
Index: gdb/ax-gdb.c
===================================================================
--- gdb.orig/ax-gdb.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/ax-gdb.c	2007-05-21 09:21:36.000000000 -0400
@@ -1597,8 +1597,14 @@ gen_expr (union exp_element **pc, struct
 
     case OP_REGISTER:
       {
-	int reg = (int) (*pc)[1].longconst;
-	(*pc) += 3;
+	const char *name = &(*pc)[2].string;
+	int reg;
+	(*pc) += 4 + BYTES_TO_EXP_ELEM ((*pc)[1].longconst + 1);
+	reg = frame_map_name_to_regnum (deprecated_safe_get_selected_frame (),
+					name, strlen (name));
+	if (reg == -1)
+	  internal_error (__FILE__, __LINE__,
+			  _("Register $%s not available"));
 	value->kind = axs_lvalue_register;
 	value->u.reg = reg;
 	value->type = register_type (current_gdbarch, reg);
Index: gdb/eval.c
===================================================================
--- gdb.orig/eval.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/eval.c	2007-05-21 09:21:31.000000000 -0400
@@ -500,16 +500,21 @@ evaluate_subexp_standard (struct type *e
 
     case OP_REGISTER:
       {
-	int regno = longest_to_int (exp->elts[pc + 1].longconst);
+	const char *name = &exp->elts[pc + 2].string;
+	int regno;
 	struct value *val;
-	(*pos) += 2;
+
+	(*pos) += 3 + BYTES_TO_EXP_ELEM (exp->elts[pc + 1].longconst + 1);
+	regno = frame_map_name_to_regnum (deprecated_safe_get_selected_frame (),
+					  name, strlen (name));
+	if (regno == -1)
+	  error (_("Register $%s not available."), name);
 	if (noside == EVAL_AVOID_SIDE_EFFECTS)
 	  val = value_zero (register_type (current_gdbarch, regno), not_lval);
 	else
 	  val = value_of_register (regno, get_selected_frame (NULL));
 	if (val == NULL)
-	  error (_("Value of register %s not available."),
-		 frame_map_regnum_to_name (get_selected_frame (NULL), regno));
+	  error (_("Value of register %s not available."), name);
 	else
 	  return val;
       }
Index: gdb/expprint.c
===================================================================
--- gdb.orig/expprint.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/expprint.c	2007-05-21 09:05:33.000000000 -0400
@@ -130,10 +130,8 @@ print_subexp_standard (struct expression
 
     case OP_REGISTER:
       {
-	int regnum = longest_to_int (exp->elts[pc + 1].longconst);
-	const char *name = user_reg_map_regnum_to_name (current_gdbarch,
-							regnum);
-	(*pos) += 2;
+	const char *name = &exp->elts[pc + 2].string;
+	(*pos) += 3 + BYTES_TO_EXP_ELEM (exp->elts[pc + 1].longconst + 1);
 	fprintf_filtered (stream, "$%s", name);
 	return;
       }
@@ -965,9 +963,8 @@ dump_subexp_body_standard (struct expres
       elt += 2;
       break;
     case OP_REGISTER:
-      fprintf_filtered (stream, "Register %ld",
-			(long) exp->elts[elt].longconst);
-      elt += 2;
+      fprintf_filtered (stream, "Register \"%s\"", &exp->elts[elt + 1].string);
+      elt += 3 + BYTES_TO_EXP_ELEM (exp->elts[elt].longconst + 1);
       break;
     case OP_INTERNALVAR:
       fprintf_filtered (stream, "Internal var @");
Index: gdb/expression.h
===================================================================
--- gdb.orig/expression.h	2007-05-21 09:00:03.000000000 -0400
+++ gdb/expression.h	2007-05-21 09:05:33.000000000 -0400
@@ -166,9 +166,8 @@ enum exp_opcode
        With another OP_LAST at the end, this makes three exp_elements.  */
     OP_LAST,
 
-    /* OP_REGISTER is followed by an integer in the next exp_element.
-       This is the number of a register to fetch (as an int).
-       With another OP_REGISTER at the end, this makes three exp_elements.  */
+    /* OP_REGISTER is followed by a string in the next exp_element.
+       This is the name of a register to fetch.  */
     OP_REGISTER,
 
     /* OP_INTERNALVAR is followed by an internalvar ptr in the next exp_element.
Index: gdb/parse.c
===================================================================
--- gdb.orig/parse.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/parse.c	2007-05-21 09:05:33.000000000 -0400
@@ -549,7 +549,9 @@ handle_last:
   return;
 handle_register:
   write_exp_elt_opcode (OP_REGISTER);
-  write_exp_elt_longcst (i);
+  str.length--;
+  str.ptr++;
+  write_exp_string (str);
   write_exp_elt_opcode (OP_REGISTER);
   return;
 }
@@ -899,7 +901,6 @@ operator_length_standard (struct express
     case OP_TYPE:
     case OP_BOOL:
     case OP_LAST:
-    case OP_REGISTER:
     case OP_INTERNALVAR:
       oplen = 3;
       break;
@@ -954,6 +955,7 @@ operator_length_standard (struct express
     case STRUCTOP_PTR:
       args = 1;
       /* fall through */
+    case OP_REGISTER:
     case OP_M2_STRING:
     case OP_STRING:
     case OP_OBJC_NSSTRING:	/* Objective C Foundation Class NSString constant */
Index: gdb/tracepoint.c
===================================================================
--- gdb.orig/tracepoint.c	2007-05-21 09:00:03.000000000 -0400
+++ gdb/tracepoint.c	2007-05-21 09:21:15.000000000 -0400
@@ -1601,11 +1601,20 @@ encode_actions (struct tracepoint *t, ch
 		  switch (exp->elts[0].opcode)
 		    {
 		    case OP_REGISTER:
-		      i = exp->elts[1].longconst;
-		      if (info_verbose)
-			printf_filtered ("OP_REGISTER: ");
-		      add_register (collect, i);
-		      break;
+		      {
+			const char *name = &exp->elts[2].string;
+
+			i = frame_map_name_to_regnum (deprecated_safe_get_selected_frame (),
+						      name, strlen (name));
+			if (i == -1)
+			  internal_error (__FILE__, __LINE__,
+					  _("Register $%s not available"),
+					  name);
+			if (info_verbose)
+			  printf_filtered ("OP_REGISTER: ");
+			add_register (collect, i);
+			break;
+		      }
 
 		    case UNOP_MEMVAL:
 		      /* safe because we know it's a simple expression */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]