[PATCH] Gracefully handle not being able to access a DWARF register number

[PATCH] Gracefully handle not being able to access a DWARF register number

[Matthew Gretton-Dann]

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

Hi,

There are cases when gcc outputs a valid DWARF register number in a
DWARF expression, but gdb can't access that register.  In theory this is
a generic problem, but the specific case I'm interested in is debugging
ARM VFP code on Linux with a kernel prior to 2.6.30. In this case ptrace
does not expose the contents of a processes' VFP registers although gcc
accurately describes the location of floating point values as being in
the VFP registers.

If you try to access such a DWARF register number gdb reports an
internal error.

The attached patch changes these internal errors into warnings, and
tries to return something sensible to the user.

Please could someone review and comment on the patch, and then if this
is approved can they please commit it as I don't have commit rights.

Suggested ChangeLog:

2010-04-26  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>

	* dwarf2loc.c (read_pieced_value, write_pieced_value,
	dwarf2_evaluate_loc_desc): Handle not being able to access DWARF
	registers gracefully.

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - Tools, PD Software
ARM Limited

[-- Attachment #2: 1004-gdb-dwarf2loc.patch --]
[-- Type: text/x-patch, Size: 2259 bytes --]

Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.75
diff -u -p -r1.75 dwarf2loc.c
--- gdb/dwarf2loc.c	20 Apr 2010 18:52:59 -0000	1.75
+++ gdb/dwarf2loc.c	26 Apr 2010 14:51:25 -0000
@@ -284,8 +284,17 @@ read_pieced_value (struct value *v)
 	      /* Big-endian, and we want less than full size.  */
 	      reg_offset = register_size (arch, gdb_regnum) - p->size;
 
-	    get_frame_register_bytes (frame, gdb_regnum, reg_offset, p->size,
-				      contents + offset);
+	    if (gdb_regnum != -1)
+	      {
+		get_frame_register_bytes (frame, gdb_regnum, reg_offset, 
+					  p->size, contents + offset);
+	      }
+	    else
+	      {
+		warning (_("Unable to access DWARF register number %" BFD_VMA_FMT "d"),
+			 p->v.expr.value);
+		memset (contents + offset, '\0', p->size);
+	      }
 	  }
 	  break;
 
@@ -356,8 +365,16 @@ write_pieced_value (struct value *to, st
 	      /* Big-endian, and we want less than full size.  */
 	      reg_offset = register_size (arch, gdb_regnum) - p->size;
 
-	    put_frame_register_bytes (frame, gdb_regnum, reg_offset, p->size,
-				      contents + offset);
+	    if (gdb_regnum != -1)
+	      {
+		put_frame_register_bytes (frame, gdb_regnum, reg_offset, 
+					  p->size, contents + offset);
+	      }
+	    else
+	      {
+		warning (_("Unable to write to DWARF Register %" BFD_VMA_FMT "d"),
+			 p->v.expr.value);
+	      }
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
@@ -454,7 +471,20 @@ dwarf2_evaluate_loc_desc (struct symbol 
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    CORE_ADDR dwarf_regnum = dwarf_expr_fetch (ctx, 0);
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, dwarf_regnum);
-	    retval = value_from_register (SYMBOL_TYPE (var), gdb_regnum, frame);
+	    if (gdb_regnum != -1)
+	      {
+		retval = value_from_register (SYMBOL_TYPE (var),
+					      gdb_regnum, frame);
+	      }
+	    else
+	      {
+		warning (_("Unable to access DWARF register number %" BFD_VMA_FMT "d"),
+			 dwarf_regnum);
+		retval = allocate_value (SYMBOL_TYPE (var));
+		VALUE_LVAL (retval) = not_lval;
+		set_value_optimized_out (retval, 1);
+		return retval;
+	      }
 	  }
 	  break;
 

Re: [PATCH] Gracefully handle not being able to access a DWARF register number

[Tom Tromey]

>>>>> "Matthew" == Matthew Gretton-Dann <matthew.gretton-dann@arm.com> writes:

Matthew> The attached patch changes these internal errors into warnings, and
Matthew> tries to return something sensible to the user.

Why a warning and not an error?

It seems to me that an error is preferable to returning incorrect
information.

Matthew> Please could someone review and comment on the patch, and then if this
Matthew> is approved can they please commit it as I don't have commit rights.

Do you have copyright assignment paperwork in place?

Matthew> +		warning (_("Unable to access DWARF register number %" BFD_VMA_FMT "d"),
Matthew> +			 p->v.expr.value);

I think there is some other way to print a CORE_ADDR, but I forget what
it is.  BFD_VMA_FMT seems suspect, just because it is not used anywhere
else in gdb.

Tom

Re: [PATCH] Gracefully handle not being able to access a DWARF register number

[Sergio Durigan Junior]

On Monday 26 April 2010 17:06:34, Tom Tromey wrote:
> >>>>> "Matthew" == Matthew Gretton-Dann <matthew.gretton-dann@arm.com> 
writes:
> Matthew> +		warning (_("Unable to access DWARF register number %"
> BFD_VMA_FMT "d"), Matthew> +			 p->v.expr.value);
> 
> I think there is some other way to print a CORE_ADDR, but I forget what
> it is.

I think that's what you're looking for:

/* Convert CORE_ADDR to string in platform-specific manner.
   This is usually formatted similar to 0x%lx.  */
extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr);

Defined in gdb/defs.h

Regards,

-- 
Sergio

Re: [PATCH] Gracefully handle not being able to access a DWARF register number

[Matthew Gretton-Dann]

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

On Mon, 2010-04-26 at 14:06 -0600, Tom Tromey wrote:
> >>>>> "Matthew" == Matthew Gretton-Dann <matthew.gretton-dann@arm.com> writes:
> 
> Matthew> The attached patch changes these internal errors into warnings, and
> Matthew> tries to return something sensible to the user.
> 
> Why a warning and not an error?
> 
> It seems to me that an error is preferable to returning incorrect
> information.

Agreed.

> Matthew> Please could someone review and comment on the patch, and then if this
> Matthew> is approved can they please commit it as I don't have commit rights.
> 
> Do you have copyright assignment paperwork in place?

Yes - through my employer (ARM).

> Matthew> +		warning (_("Unable to access DWARF register number %" BFD_VMA_FMT "d"),
> Matthew> +			 p->v.expr.value);
> 
> I think there is some other way to print a CORE_ADDR, but I forget what
> it is.  BFD_VMA_FMT seems suspect, just because it is not used anywhere
> else in gdb.

Okay.

Please find attached an updated patch that addresses the above two
issues.

Proposed ChangeLog:

2010-04-27  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>

        * dwarf2loc.c (read_pieced_value, write_pieced_value,
        dwarf2_evaluate_loc_desc): Handle not being able to access DWARF
        registers gracefully.

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - Tools, PD Software
ARM Limited

[-- Attachment #2: 1004-gdb-dwarf2loc.patch --]
[-- Type: text/x-patch, Size: 2100 bytes --]

Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 dwarf2loc.c
--- gdb/dwarf2loc.c	17 Mar 2010 22:04:43 -0000	1.74
+++ gdb/dwarf2loc.c	27 Apr 2010 09:44:34 -0000
@@ -284,8 +284,16 @@ read_pieced_value (struct value *v)
 	      /* Big-endian, and we want less than full size.  */
 	      reg_offset = register_size (arch, gdb_regnum) - p->size;
 
-	    get_frame_register_bytes (frame, gdb_regnum, reg_offset, p->size,
-				      contents + offset);
+	    if (gdb_regnum != -1)
+	      {
+		get_frame_register_bytes (frame, gdb_regnum, reg_offset, 
+					  p->size, contents + offset);
+	      }
+	    else
+	      {
+		error (_("Unable to access DWARF register number %s"),
+		       paddress (arch, p->v.expr.value));
+	      }
 	  }
 	  break;
 
@@ -356,8 +364,16 @@ write_pieced_value (struct value *to, st
 	      /* Big-endian, and we want less than full size.  */
 	      reg_offset = register_size (arch, gdb_regnum) - p->size;
 
-	    put_frame_register_bytes (frame, gdb_regnum, reg_offset, p->size,
-				      contents + offset);
+	    if (gdb_regnum != -1)
+	      {
+		put_frame_register_bytes (frame, gdb_regnum, reg_offset, 
+					  p->size, contents + offset);
+	      }
+	    else
+	      {
+		error (_("Unable to write to DWARF register number %s"),
+		       paddress (arch, p->v.expr.value));
+	      }
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
@@ -454,7 +470,16 @@ dwarf2_evaluate_loc_desc (struct symbol 
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    CORE_ADDR dwarf_regnum = dwarf_expr_fetch (ctx, 0);
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, dwarf_regnum);
-	    retval = value_from_register (SYMBOL_TYPE (var), gdb_regnum, frame);
+	    if (gdb_regnum != -1)
+	      {
+		retval = value_from_register (SYMBOL_TYPE (var),
+					      gdb_regnum, frame);
+	      }
+	    else
+	      {
+		error (_("Unable to access DWARF register number %s"),
+		       paddress (arch, dwarf_regnum));
+	      }
 	  }
 	  break;
 

Re: [PATCH] Gracefully handle not being able to access a DWARF register number

[Tom Tromey]

>>>>> "Matthew" == Matthew Gretton-Dann <matthew.gretton-dann@arm.com> writes:

Matthew> 2010-04-27  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
Matthew>         * dwarf2loc.c (read_pieced_value, write_pieced_value,
Matthew>         dwarf2_evaluate_loc_desc): Handle not being able to access DWARF
Matthew>         registers gracefully.

This is ok.  Thanks.

It seems that some targets already emit a warning in their
dwarf2_reg_to_regnum implementation.  E.g., amd64-tdep.c:

  if (regnum == -1)
    warning (_("Unmapped DWARF Register #%d encountered."), reg);

This is inconsistently done, though, and IMO doesn't impact the
usefulness of your patch.

Tom