[v3] arc: Select CPU model properly before disassembling
Commit Message
Changes in V3:
* Minor formatting fix.
* Do not assign disassemble_info->section if it is already set.
* Free arc_disassembler_options before assignment.
Changes in V2:
* Use xstrprintf instead of string_printf
* Reinstate arc_delayed_print_insn as a print instruction function for ARC.
* Change arc_delayed_print_insn to use default_print_insn.
* Change commit description accordingly.
---
Enforce CPU model for disassembler via its options, if it was specified in XML
target description, otherwise use default method of determining CPU implemented
in disassembler - scanning ELF private header. The latter requires
disassemble_info->section to be properly initialized. To make sure that
info->section is set in all cases this patch partially reverts [1] for ARC: it
reinstates arc_delayed_print_insn as a "print_insn" function for ARC, but
now this function only sets disassemble_info->section and then calls
default_print_insn to do the rest of the job.
Support for CPU in disassembler options for ARC has been added in [2].
[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=39503f82427e22ed8e04d986ccdc8562091ec62e
[2] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=10045478d984f9924cb945423388ba25b7dd3ffe
gdb/ChangeLog:
yyyy-mm-dd Anton Kolesov <anton.kolesov@synopsys.com>
* arc-tdep.c (arc_disassembler_options): New variable.
(arc_gdbarch_init): Set and use it. Use arc_delayed_print_insn instead
of default_print_insn.
(arc_delayed_print_insn): Set info->section when needed,
use default_print_insn to retrieve a disassembler.
---
gdb/arc-tdep.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 57 insertions(+), 6 deletions(-)
Comments
On 2017-06-15 15:20, Anton Kolesov wrote:
> @@ -2041,6 +2067,31 @@ arc_gdbarch_init (struct gdbarch_info info,
> struct gdbarch_list *arches)
> if (tdep->jb_pc >= 0)
> set_gdbarch_get_longjmp_target (gdbarch, arc_get_longjmp_target);
>
> + /* Disassembler options. Enforce CPU if it was specified in XML
> target
> + description, otherwise use default method of determining CPU (ELF
> private
> + header). */
> + if (info.target_desc != NULL)
> + {
> + const struct bfd_arch_info *tdesc_arch
> + = tdesc_architecture (info.target_desc);
> + if (tdesc_arch != NULL)
> + {
> + free (arc_disassembler_options);
Nit: Use xfree instead of free.
You can fix that and push directly.
Thanks!
Simon
> > + header). */
> > + if (info.target_desc != NULL)
> > + {
> > + const struct bfd_arch_info *tdesc_arch
> > + = tdesc_architecture (info.target_desc);
> > + if (tdesc_arch != NULL)
> > + {
> > + free (arc_disassembler_options);
>
> Nit: Use xfree instead of free.
>
> You can fix that and push directly.
Pushed, thanks for the review!
Anton
>
> Thanks!
>
> Simon
@@ -145,6 +145,8 @@ static const char *const core_arcompact_register_names[] = {
"lp_count", "reserved", "limm", "pcl",
};
+static char *arc_disassembler_options = NULL;
+
/* Functions are sorted in the order as they are used in the
_initialize_arc_tdep (), which uses the same order as gdbarch.h. Static
functions are defined before the first invocation. */
@@ -1405,12 +1407,34 @@ arc_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
int
arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
{
- int (*print_insn) (bfd_vma, struct disassemble_info *);
- /* exec_bfd may be null, if GDB is run without a target BFD file. Opcodes
- will handle NULL value gracefully. */
- print_insn = arc_get_disassembler (exec_bfd);
- gdb_assert (print_insn != NULL);
- return print_insn (addr, info);
+ /* Standard BFD "machine number" field allows libocodes disassembler to
+ distinguish ARC 600, 700 and v2 cores, however v2 encompasses both ARC EM
+ and HS, which have some difference between. There are two ways to specify
+ what is the target core:
+ 1) via the disassemble_info->disassembler_options;
+ 2) otherwise libopcodes will use private (architecture-specific) ELF
+ header.
+
+ Using disassembler_options is preferable, because it comes directly from
+ GDBserver which scanned an actual ARC core identification info. However,
+ not all GDBservers report core architecture, so as a fallback GDB still
+ should support analysis of ELF header. The libopcodes disassembly code
+ uses the section to find the BFD and the BFD to find the ELF header,
+ therefore this function should set disassemble_info->section properly.
+
+ disassembler_options was already set by non-target specific code with
+ proper options obtained via gdbarch_disassembler_options ().
+
+ This function might be called multiple times in a sequence, reusing same
+ disassemble_info. */
+ if ((info->disassembler_options == NULL) && (info->section == NULL))
+ {
+ struct obj_section *s = find_pc_section (addr);
+ if (s != NULL)
+ info->section = s->the_bfd_section;
+ }
+
+ return default_print_insn (addr, info);
}
/* Baremetal breakpoint instructions.
@@ -2013,6 +2037,8 @@ arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_frame_align (gdbarch, arc_frame_align);
+ set_gdbarch_print_insn (gdbarch, arc_delayed_print_insn);
+
set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
/* "nonsteppable" watchpoint means that watchpoint triggers before
@@ -2041,6 +2067,31 @@ arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdep->jb_pc >= 0)
set_gdbarch_get_longjmp_target (gdbarch, arc_get_longjmp_target);
+ /* Disassembler options. Enforce CPU if it was specified in XML target
+ description, otherwise use default method of determining CPU (ELF private
+ header). */
+ if (info.target_desc != NULL)
+ {
+ const struct bfd_arch_info *tdesc_arch
+ = tdesc_architecture (info.target_desc);
+ if (tdesc_arch != NULL)
+ {
+ free (arc_disassembler_options);
+ /* FIXME: It is not really good to change disassembler options
+ behind the scene, because that might override options
+ specified by the user. However as of now ARC doesn't support
+ `set disassembler-options' hence this code is the only place
+ where options are changed. It also changes options for all
+ existing gdbarches, which also can be problematic, if
+ arc_gdbarch_init will start reusing existing gdbarch
+ instances. */
+ arc_disassembler_options = xstrprintf ("cpu=%s",
+ tdesc_arch->printable_name);
+ set_gdbarch_disassembler_options (gdbarch,
+ &arc_disassembler_options);
+ }
+ }
+
tdesc_use_registers (gdbarch, tdesc, tdesc_data);
return gdbarch;