diff mbox

[V3] read_pieced_value do big endian processing only in case of valid gdb_regnum

Message ID 1414042597-30986-1-git-send-email-victor.kamensky@linaro.org
State New
Headers show

Commit Message

vkamensky Oct. 23, 2014, 5:36 a.m. UTC 
During armv7b testing gdb.base/store.exp test was failling with
'GDB internal error' with the following message:

Temporary breakpoint 1, wack_double (u=
../../binutils-gdb/gdb/regcache.c:177: internal-error: register_size: Assertion `regnum >= 0 && regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch))' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.

It turns out that compiler generated DWARF with non-existent
register numbers. The compiler issue is present in both little endian
(armv7) and big endian (armv7b) (it is separate issue). Here is
example for one of formal parameters of wack_double function:

 <2><792>: Abbrev Number: 10 (DW_TAG_formal_parameter)
    <793>   DW_AT_name        : u
    <795>   DW_AT_decl_file   : 1
    <796>   DW_AT_decl_line   : 115
    <797>   DW_AT_type        : <0x57c>
    <79b>   DW_AT_location    : 6 byte block: 6d 93 4 6c 93 4   (DW_OP_reg29 (r29); DW_OP_piece: 4; DW_OP_reg28 (r28); DW_OP_piece: 4)

In both big and little endian cases gdbarch_dwarf2_reg_to_regnum
returns -1 which is stored into gdb_regnum. But it causes severe
problem only in big endian case because in read_pieced_value and
write_pieced_value functions BFD_ENDIAN_BIG related processing
happen regardless of gdb_regnum value, for example register_size
function is called and in case of gdb_regnum=-1, it cause
'GDB internal error' and crash.

Solution is to move BFD_ENDIAN_BIG related processing under
(gdb_regnum != -1) branch of processing.

gdb/ChangeLog:

2014-10-21  Victor Kamensky  <victor.kamensky@linaro.org>

	* dwarf2loc.c (read_pieced_value): Do BE processing only if
	gdb_regnum is not -1.
	(write_pieced_value): Ditto.
---
 gdb/dwarf2loc.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Yao Qi Oct. 23, 2014, 6:32 a.m. UTC | #1 
Victor Kamensky <victor.kamensky@linaro.org> writes:

> gdb/ChangeLog:
>
> 2014-10-21  Victor Kamensky  <victor.kamensky@linaro.org>
>
> 	* dwarf2loc.c (read_pieced_value): Do BE processing only if

s/BE/big endian/

> @@ -1876,15 +1876,15 @@ write_pieced_value (struct value *to, struct value *from)
>  	  {
>  	    struct gdbarch *arch = get_frame_arch (frame);
>  	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.regno);
> -	    int reg_offset = dest_offset;
> -
> -	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
> -		&& this_size <= register_size (arch, gdb_regnum))
> -	      /* Big-endian, and we want less than full size.  */
> -	      reg_offset = register_size (arch, gdb_regnum) - this_size;
>  
>  	    if (gdb_regnum != -1)
>  	      {
> +		int reg_offset = dest_offset;

Add an empty line between local declaration and first statement.  See
https://sourceware.org/gdb/wiki/JoelsCodingStyleCheatSheet

> +		if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
> +		    && this_size <= register_size (arch, gdb_regnum))
> +		  /* Big-endian, and we want less than full size.  */
> +		  reg_offset = register_size (arch, gdb_regnum) - this_size;
> +

Could you please add braces to the if block, which has more than one
lines? (braces are not needed if the block only has one line.)

		if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
		    && this_size <= register_size (arch, gdb_regnum))
                    {
		      /* Big-endian, and we want less than full size.  */
                      reg_offset = register_size (arch, gdb_regnum) - this_size;
                    }

This patch is OK to me.  If there is no comments from other people in
three days, you can push it in.  I'd like this patch stay here for a
while, and another pair of eyes may have a chance to take a look.
diff mbox

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index e347e59..d0ca35e 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1684,21 +1684,21 @@  read_pieced_value (struct value *v)
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.regno);
-	    int reg_offset = source_offset;
-
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size < register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-		/* We want the lower-order THIS_SIZE_BITS of the bytes
-		   we extract from the register.  */
-		source_offset_bits += 8 * this_size - this_size_bits;
-	      }
 
 	    if (gdb_regnum != -1)
 	      {
 		int optim, unavail;
+		int reg_offset = source_offset;
+
+		if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
+		    && this_size < register_size (arch, gdb_regnum))
+		  {
+		    /* Big-endian, and we want less than full size.  */
+		    reg_offset = register_size (arch, gdb_regnum) - this_size;
+		    /* We want the lower-order THIS_SIZE_BITS of the bytes
+		       we extract from the register.  */
+		    source_offset_bits += 8 * this_size - this_size_bits;
+		 }
 
 		if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
 					       this_size, buffer,
@@ -1876,15 +1876,15 @@  write_pieced_value (struct value *to, struct value *from)
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.regno);
-	    int reg_offset = dest_offset;
-
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size <= register_size (arch, gdb_regnum))
-	      /* Big-endian, and we want less than full size.  */
-	      reg_offset = register_size (arch, gdb_regnum) - this_size;
 
 	    if (gdb_regnum != -1)
 	      {
+		int reg_offset = dest_offset;
+		if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
+		    && this_size <= register_size (arch, gdb_regnum))
+		  /* Big-endian, and we want less than full size.  */
+		  reg_offset = register_size (arch, gdb_regnum) - this_size;
+
 		if (need_bitwise)
 		  {
 		    int optim, unavail;