Re: [PATCH 4/5] ARM: read_pieced_value do big endian processing only in case of valid gdb_regnum

[Yao Qi <yao@codesourcery.com>]

Victor Kamensky <victor.kamensky@linaro.org> writes:

Hi Victor,
Could you please add more details in the commit message? for example....

> During armv7b testing gdb.base/store.exp test was failling with
> 'GDB internal error'. It turns out that compiler generated DWARF

What is the 'GDB internal error'?  Is it like this?

(gdb) PASS: gdb.base/store.exp: continue to wack_double
print l^M
gdb/regcache.c:178: internal-error: register_size: Assertion `regnum >= 0 && regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch))' failed.^M
A problem internal to GDB has been detected,

We've seen this internal error on (armv5te big-endian) for a while.

> with non-existent register numbers. The compiler issue is present
> in both little endian (armv7) and big endian (armv7b) (it is
> separate issue). In both case gdbarch_dwarf2_reg_to_regnum returns

Is there any PR opened for the compiler issue?  If there is, please
mention it in the commit message, otherwise, please describe the
mistakes in the compiler generated debug info, the snippet of
'readelf -wi' output, which shows the wrong register number, should be fine.

> -1 which is stored into gdb_regnum. But it cause 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, 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.

With your patch applied, the internal error is fixed.  How does GDB
behave now?  What is the output for 'print l'?  In my case, it becomes:

print l^M
Unable to access DWARF register number 80^M
(gdb) FAIL: gdb.base/store.exp: upvar float l; print old l, expecting -1

> ---
>  gdb/ChangeLog   |  6 ++++++
>  gdb/dwarf2loc.c | 30 +++++++++++++++---------------
>  2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index c32fb3f..6a735b8 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,11 @@
>  2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
>  
> +	* dwarf2loc.c (read_pieced_value): do BE processing only if
> +	gdb_regnum is not -1.

s/do/Do.  Looks you've fixed it in V2.
s/BE/big endian/ because BE isn't very clear here.

> +	(write_pieced_value): Ditto.
> +
> +2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
> +
>  	* arm-tdep.c: (extract_arm_insn): use dbarch_byte_order_for_code
>  	to read arm instruction.
>  
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index e347e59..fbe99bb 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -1686,20 +1686,20 @@ read_pieced_value (struct value *v)
>  	    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;
>  
> +		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;
> +		 }
> +

Nit: after the change, local variable 'reg_offset' is only used in the
"if (gdb_regnum != -1) {}" block, so we can move 'reg_offset' into that
block.

-- 
Yao (齐尧)