Re: [PATCH] Clear upper bits during sign extension

[Pedro Alves <palves@redhat.com>]

On 12/29/2014 01:12 AM, Yao Qi wrote:
> I see the error message "access outside bounds of object referenced
> via synthetic pointer" in the two fails below of mips gdb testing
> 
> print d[-2]^M
> access outside bounds of object referenced via synthetic pointer^M
> (gdb) FAIL: gdb.dwarf2/implptrconst.exp: print d[-2]
> (gdb) print/d p[-1]^M
> access outside bounds of object referenced via synthetic pointer^M
> (gdb) FAIL: gdb.dwarf2/implptrpiece.exp: print/d p[-1]
> 
> in the first test, 'd[-2]' is processed by GDB as '* (&d[-2])'.  'd'
> is a synthetic pointer, so its value is zero, the address of 'd[-2]'
> is -2.  In dwarf2loc.c:indirect_pieced_value,
> 
>   /* This is an offset requested by GDB, such as value subscripts.
>      However, due to how synthetic pointers are implemented, this is
>      always presented to us as a pointer type.  This means we have to
>      sign-extend it manually as appropriate.  */
>   byte_offset = value_as_address (value);                  <---- [1]
>   if (TYPE_LENGTH (value_type (value)) < sizeof (LONGEST))
>     byte_offset = gdb_sign_extend (byte_offset,            <---- [2]
> 				   8 * TYPE_LENGTH (value_type (value)));
>   byte_offset += piece->v.ptr.offset;
> 
> on MIPS target, after [1], byte_offset is -2 (0xfffffffffffffffe),
> because 32-bit -2 (as an address) is sign extended to 64-bit.  After
> [2], we manually sign extend byte_offset too, and then it becomes
> 0xfffffffefffffffe, which is wrong.  Function gdb_sign_extend
> sign-extends VALUE on bit BIT, and assumes upper bits from bit BIT are
> all zero.  That is why the code works well on targets on which address
> is zero extended, such as x86.  On these targets, byte_offset is
> 0xfffffffe (zero extended from 32-bit address -2).
> 
> The patch is to clear upper bits of VALUE in gdb_sign_extend first.
> Regression tested on mips-linux-gnu, and fixes two fails above.

This seems to me to paper over an issue elsewhere, and is likely
to paper over issues as gdb_sign_extend is used more throughout.

I'm not immediately familiar with all the conditions indirect_pieced_value
is called, but going by the comment quoted, I think the root issue
might be that we shouldn't use value_as_address in the first place,
but something like unpack_long directly.

E.g., I don't see how it makes sense to interpret -2 as an address
on spu, which ends up calling:

 static CORE_ADDR
 spu_integer_to_address (struct gdbarch *gdbarch,
 			struct type *type, const gdb_byte *buf)
 {
   int id = spu_gdbarch_id (gdbarch);
   ULONGEST addr = unpack_long (type, buf);

   return SPUADDR (id, addr);
 }

or on avr, which ends up calling:

static CORE_ADDR
avr_integer_to_address (struct gdbarch *gdbarch,
			struct type *type, const gdb_byte *buf)
{
  ULONGEST addr = unpack_long (type, buf);

  return avr_make_saddr (addr);
}

...

/* SRAM address checks and convertions.  */

static CORE_ADDR
avr_make_saddr (CORE_ADDR x)
{
  /* Return 0 for NULL.  */
  if (x == 0)
    return 0;

  return ((x) | AVR_SMEM_START);
}

...
  AVR_SMEM_START = 0x00800000,	/* SRAM memory */
...

Thanks,
Pedro Alves