Thanks for the review.

On 21/04/17 20:37, Pedro Alves wrote:
> On 04/21/2017 03:56 PM, Jiong Wang wrote:
> Doesn't the Aarch64 version of the opcode have its own
> name, like DW_CFA_GNU_Aarch64_whatever, even if it reuses the
> opcode number?  I think that would help a lot going forward
> if it had one.  E.g., it'd avoid confusion, allow for easier
> searching, etc.

AArch64 do have a name for it, "DW_CFA_AARCH64_negate_ra_state",
I will add it to gcc/include/dwarf2.def later once it's used and will request
a sync from GCC to Bintuils.

> On the patch, I think it would be better if in execute_cfa_program:
>
>>          case DW_CFA_GNU_window_save:
>> -          /* This is SPARC-specific code, and contains hard-coded
>> -             constants for the register numbering scheme used by
>
> you removed "case CFA_GNU_window_save:" too, and moved the
> gdbarch_dwarf_cfa_op call to the default case.

Done.

> Then make the hook return a boolean indication about whether it /recognized the
> opcode, and make the caller throw an error if the hook returns
> false.

Done.

> And make that error call be a regular "error()" call instead
> of the current internal_error call.  The internal error is bogus
> here because we reach that with an unrecognized opcode that comes
> from a binary, i.e., input, not a gdb bug.

Done.

> Replace the gdb_assert in sparc_dwarf_cfa_op by returning false for
> unrecognized opcodes, and likewise change default_dwarf_cfa_op
> to return false instead of calling error itself.

Done.

> Add a comment at the hook call site about letting the backend
> handle vendor-specific opcodes, and generalize a bit the comment
> describing the hook in gdbarch.sh in that direction as well.

Done.

> Maybe rename to hook from dwarf_cfa_op to something like:
>   execute_dwarf_cfa_vendor_op

Done with above name.

Ok for master? thanks


gdb/
2017-04-25  Jiong Wang  <jiong.wang@arm.com>

         * gdbarch.sh: New gdbarch method execute_dwarf_cfa_vendor_op.
         * gdbarch.c: Regenerated.
         * gdbarch.h: Regenerated.
         * dwarf2-frame.c (dwarf2_frame_state_alloc_regs): Made the
         visibility external.
         (execute_cfa_program): Call execute_dwarf_cfa_vendor_op for CFI
         between DW_CFA_lo_user and DW_CFA_high_user inclusive.
         (enum cfa_how_kind): Move to ...
         (struct dwarf2_frame_state_reg_info): Likewise.
         (struct dwarf2_frame_state): Likewise.
         * dwarf2-frame.h: ... here.
         (dwarf2_frame_state_alloc_regs): New declaration.
         * sparc-tdep.c (sparc_execute_dwarf_cfa_vendor_op): New function.
         (sparc32_gdbarch_init): Register execute_dwarf_cfa_vendor_op hook.