[RFA] fix "ptype $pc" internal-error

[RFA] fix "ptype $pc" internal-error

[Joel Brobecker]

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

Hello,

"ptype $pc" doesn't work anymore. With any program, try the following:

    (gdb) ptype $pc
    ../../src-public/gdb/regcache.c:164: internal-error: register_type: Assertion `regnum >= 0 && regnum < descr->nr_cooked_registers' failed.
    [...]

The problem is inside evaluate_subexp_standard, case OP_REGISTER:

        regno = frame_map_name_to_regnum (deprecated_safe_get_selected_frame (),
                                          name, strlen (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));

In our case, the regno points to a "user" register, and as a result
calling "register_type" with that register number triggers the failed
assertion.

The type of a user register doesn't seem to be conveniently accessible
like with the other registers. So my approach was to return a value_zero
only when the register was not a user register. For user registers,
I get the actual register value, regardless of EVAL_AVOID_SIDE_EFFECTS.
I hope the performance will not be noticeably affected.

Otherwise, a grander way of fixing this would be to extend the user-regs
interface to provide access to these register types. Either through
a new callback function, or by updating the "add_reg" function with
an extra parameter being the type.

I don't remember seeing this need elsewhere, so I think the simple
approach is sufficient for now.

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * eval.c (evaluate_subexp_standard): Add handling of user
        registers when in EVAL_AVOID_SIDE_EFFECTS mode.

I also added a new test in gdb.base/ptype.exp:

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * gdb.base/ptype.exp: Add testing of "ptype $pc". 

All tested on x86-linux. No regression.
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: ptype_pc.diff --]
[-- Type: text/plain, Size: 952 bytes --]

Index: eval.c
===================================================================
--- eval.c	(revision 65)
+++ eval.c	(revision 66)
@@ -512,7 +512,15 @@ evaluate_subexp_standard (struct type *e
 					  name, strlen (name));
 	if (regno == -1)
 	  error (_("Register $%s not available."), name);
-	if (noside == EVAL_AVOID_SIDE_EFFECTS)
+
+        /* In EVAL_AVOID_SIDE_EFFECTS mode, we only need to return
+           a value with the appropriate register type.  Unfortunately,
+           we don't have easy access to the type of user registers.
+           So for these registers, we fetch the register value regardless
+           of the evaluation mode.  */
+	if (noside == EVAL_AVOID_SIDE_EFFECTS
+	    && regno < gdbarch_num_regs (current_gdbarch)
+	       + gdbarch_num_pseudo_regs (current_gdbarch))
 	  val = value_zero (register_type (current_gdbarch, regno), not_lval);
 	else
 	  val = value_of_register (regno, get_selected_frame (NULL));

[-- Attachment #3: ptype_pc-tc.diff --]
[-- Type: text/plain, Size: 474 bytes --]

Index: gdb.base/ptype.exp
===================================================================
--- gdb.base/ptype.exp	(revision 66)
+++ gdb.base/ptype.exp	(revision 67)
@@ -639,4 +639,7 @@ if [runto_main] then {
   gdb_test "ptype {{0,1,2},{3,4,5}}"	"type = int \\\[2\\\]\\\[3\\\]"
   gdb_test "ptype {4,5,6}\[2\]"	"type = int"
   gdb_test "ptype *&{4,5,6}\[1\]"	"type = int"
+
+  # Test ptype of user register
+  gdb_test "ptype \$pc" "void \\(\\*\\)\\(\\)" "ptype \$pc"
 }

Re: [RFA] fix "ptype $pc" internal-error

[Andreas Schwab]

Joel Brobecker <brobecker@adacore.com> writes:

> Index: eval.c
> ===================================================================
> --- eval.c	(revision 65)
> +++ eval.c	(revision 66)
> @@ -512,7 +512,15 @@ evaluate_subexp_standard (struct type *e
>  					  name, strlen (name));
>  	if (regno == -1)
>  	  error (_("Register $%s not available."), name);
> -	if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +
> +        /* In EVAL_AVOID_SIDE_EFFECTS mode, we only need to return
> +           a value with the appropriate register type.  Unfortunately,
> +           we don't have easy access to the type of user registers.
> +           So for these registers, we fetch the register value regardless
> +           of the evaluation mode.  */
> +	if (noside == EVAL_AVOID_SIDE_EFFECTS
> +	    && regno < gdbarch_num_regs (current_gdbarch)
> +	       + gdbarch_num_pseudo_regs (current_gdbarch))

Indentation is off here.

	if (noside == EVAL_AVOID_SIDE_EFFECTS
	    && regno < (gdbarch_num_regs (current_gdbarch)
		        + gdbarch_num_pseudo_regs (current_gdbarch)))

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFA] fix "ptype $pc" internal-error

[Joel Brobecker]

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

> Indentation is off here.
> 
> 	if (noside == EVAL_AVOID_SIDE_EFFECTS
> 	    && regno < (gdbarch_num_regs (current_gdbarch)
> 		        + gdbarch_num_pseudo_regs (current_gdbarch)))

Oops, you are right, of course. Thanks!
Fixed in the attached patch.

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * eval.c (evaluate_subexp_standard): Add handling of user
        registers when in EVAL_AVOID_SIDE_EFFECTS mode.

2008-01-04  Joel Brobecker  <brobecker@adacore.com>

        * gdb.base/ptype.exp: Add testing of "ptype $pc".

-- 
Joel

[-- Attachment #2: ptype.diff --]
[-- Type: text/plain, Size: 1418 bytes --]

Index: eval.c
===================================================================
--- eval.c	(revision 65)
+++ eval.c	(revision 68)
@@ -512,7 +512,15 @@ evaluate_subexp_standard (struct type *e
 					  name, strlen (name));
 	if (regno == -1)
 	  error (_("Register $%s not available."), name);
-	if (noside == EVAL_AVOID_SIDE_EFFECTS)
+
+	/* In EVAL_AVOID_SIDE_EFFECTS mode, we only need to return
+	   a value with the appropriate register type.  Unfortunately,
+	   we don't have easy access to the type of user registers.
+	   So for these registers, we fetch the register value regardless
+	   of the evaluation mode.  */
+	if (noside == EVAL_AVOID_SIDE_EFFECTS
+	    && regno < (gdbarch_num_regs (current_gdbarch)
+			+ gdbarch_num_pseudo_regs (current_gdbarch)))
 	  val = value_zero (register_type (current_gdbarch, regno), not_lval);
 	else
 	  val = value_of_register (regno, get_selected_frame (NULL));
Index: testsuite/gdb.base/ptype.exp
===================================================================
--- testsuite/gdb.base/ptype.exp	(revision 65)
+++ testsuite/gdb.base/ptype.exp	(revision 68)
@@ -639,4 +639,7 @@ if [runto_main] then {
   gdb_test "ptype {{0,1,2},{3,4,5}}"	"type = int \\\[2\\\]\\\[3\\\]"
   gdb_test "ptype {4,5,6}\[2\]"	"type = int"
   gdb_test "ptype *&{4,5,6}\[1\]"	"type = int"
+
+  # Test ptype of user register
+  gdb_test "ptype \$pc" "void \\(\\*\\)\\(\\)" "ptype \$pc"
 }

Re: [RFA] fix "ptype $pc" internal-error

[Daniel Jacobowitz]

On Fri, Jan 04, 2008 at 02:49:43AM -0800, Joel Brobecker wrote:
> 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> 
>         * eval.c (evaluate_subexp_standard): Add handling of user
>         registers when in EVAL_AVOID_SIDE_EFFECTS mode.

OK.

> 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdb.base/ptype.exp: Add testing of "ptype $pc".

Might want to loosen the expected type; I don't think every platform
returns a function type for PC.

Then again, might not want to - everyone should do so :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] fix "ptype $pc" internal-error

[Mark Kettenis]

> Date: Tue, 29 Jan 2008 12:45:36 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Fri, Jan 04, 2008 at 02:49:43AM -0800, Joel Brobecker wrote:
> > 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * eval.c (evaluate_subexp_standard): Add handling of user
> >         registers when in EVAL_AVOID_SIDE_EFFECTS mode.
> 
> OK.
> 
> > 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gdb.base/ptype.exp: Add testing of "ptype $pc".
> 
> Might want to loosen the expected type; I don't think every platform
> returns a function type for PC.
> 
> Then again, might not want to - everyone should do so :-)

Indeed!  So I'm fine with leaving it as is.

Re: [RFA] fix "ptype $pc" internal-error

[Joel Brobecker]

> > 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * eval.c (evaluate_subexp_standard): Add handling of user
> >         registers when in EVAL_AVOID_SIDE_EFFECTS mode.
> 
> OK.

Thanks. I checked it in.

> > 2008-01-04  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gdb.base/ptype.exp: Add testing of "ptype $pc".
> 
> Might want to loosen the expected type; I don't think every platform
> returns a function type for PC.
> 
> Then again, might not want to - everyone should do so :-)

:). I checked the test as-is, Mark agreed that it was better this way.

-- 
Joel