Re: [PATCH] aarch64-tdep basic port.

[Pedro Alves <palves@redhat.com>]

On 01/22/2013 05:38 PM, Marcus Shawcroft wrote:
> Hi,
> 
> Attached is a re-spin of the AArch64 basic port addressing comments received here:
> 
> http://sourceware.org/ml/gdb-patches/2013-01/msg00449.html
> http://sourceware.org/ml/gdb-patches/2013-01/msg00254.html
> http://sourceware.org/ml/gdb-patches/2013-01/msg00255.html

Thanks.  Some more nits below, but otherwise I have no further
comments.

> 
>   >>  +  if (tdep->jb_pc>= 0)
>   >>  +    set_gdbarch_get_longjmp_target (gdbarch,
> aarch64_get_longjmp_target);
>   >
>   >  Have you answered the comment I made in my first review? I can't
>   >  find anything...
> 
> Between the initial definition of jb_pc and this conditional is a call to gdbarch_init_osabi() both the aarch64-linux-tdep.c and aarch64-newlib-tdep.c contain an initializer that re-defines jb_pc.

So the default, non-default-osabi implementation uses which libc?

> A separate patch for documentation will be sent shortly.

Thanks.



> +
> +/* Implement the "addr_bits_remove" gdbarch method.  */
> +
> +static CORE_ADDR
> +aarch64_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val)
> +{
> +  /* All instructions are 4-byte aligned.  */
> +  return val & ~(CORE_ADDR) 0x3;
> +}

Excuse the ignorance, but why do you need this?  Does
Aarch64 do any magic low address encoding, like arm/thumb?

On 01/22/2013 05:38 PM, Marcus Shawcroft wrote:
> +aarch64_stub_unwind_sniffer (const struct frame_unwind *self,
> +			     struct frame_info *this_frame,
> +			     void **this_prologue_cache)
> +{
> +  CORE_ADDR addr_in_block;
> +  char dummy[4];

gdb_byte.

> +
> +  addr_in_block = get_frame_address_in_block (this_frame);
> +  if (in_plt_section (addr_in_block, NULL)
> +      || target_read_memory (get_frame_pc (this_frame), dummy, 4) != 0)
> +    return 1;

Please import the comment arm-tdep.c has gained since (I guess)
you forked this:

http://sourceware.org/ml/gdb-patches/2010-03/msg00189.html

(and check it makes sense for Aarch64 too).

> +     If the language code decides to pass in memory we want to move
> +     the pointer inserted as the intial argument from the argument
> +     list and into X8, the conventional AArch64 struct return pointer
> +     register.

Typo "initial".

> +  while (! VEC_empty (stack_item_t, info.si))

GDB's coding standard says no space after "!":

http://sourceware.org/gdb/current/onlinedocs/gdbint/Coding-Standards.html

> +  gdb_assert_not_reached ("regnum out of bound");

This throws, so the below is dead unreachable code.

> +
> +  mark_value_bytes_unavailable (result_value, 0,
> +				TYPE_LENGTH (value_type (result_value)));
> +  return result_value;
> +}

-- 
Pedro Alves