This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] New gdbarch method "dwarf_cfa_op" and migrate SPARC to it
On 04/21/2017 03:56 PM, Jiong Wang wrote:
> Hi Ivo,
>
> Thanks very much for testing this on SPARC platform.
>
> What's really reused is the DWARF CFA number 0x2d behind
> DW_CFA_GNU_window_save. It is in vendor extension space (
> DW_CFA_lo_user.. DW_CFA_hi_user) so the semantics depends on vendor
> interpreation.
Maybe the commit log could/should be simplified, because
I was confused too.
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.
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. 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. 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.
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.
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.
Maybe rename to hook from dwarf_cfa_op to something like:
handle_dwarf_cfa_op
handle_dwarf_cfa_vendor_op
execute_dwarf_cfa_op
execute_dwarf_cfa_vendor_op
too, while at it?
Thanks,
Pedro Alves