[2/5] arc: Set section to ".text" when disassembling

Message ID 20170214100130.29194-2-Anton.Kolesov@synopsys.com
State New, archived
Headers

Commit Message

Anton Kolesov Feb. 14, 2017, 10:01 a.m. UTC
  ARC disassembler requires that disassemble_info->section is valid - it will be
used to access program headers of ELF files that is needed to distinguish
between ARC EM and HS (ARC600, ARC700 and ARC v2 can be distinguished by BFD's
`mach` attribute).  Without this information disassembler will default to ARC
EM for ARC v2 targets, and it will not recognize instructions specific to ARC
HS, for example, double-word store and load.

gdb/ChangeLog:

yyyy-mm-dd  Anton Kolesov  <anton.kolesov@synopsys.com>

	* arc-tdep.c (arc_delayed_print_insn): Set info->section.
---
 gdb/arc-tdep.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Yao Qi Feb. 15, 2017, 10:27 p.m. UTC | #1
On Tue, Feb 14, 2017 at 10:01 AM, Anton Kolesov
<Anton.Kolesov@synopsys.com> wrote:
> +
> +  /* ARC disassembler requires that info->section is valid - it will be used to
> +     access program headers of ELF files that is needed to distinguish between
> +     ARC EM and HS (ARC600, ARC700 and ARC v2 can be distinguished by BFD's
> +     `mach` attribute).  Without this information disassembler will default to
> +     ARC EM for ARC v2 targets, and it will not recognize instructions specific
> +     to ARC HS, for example, double-word store and load.  */
> +  if (exec_bfd != NULL)
> +    info->section = bfd_get_section_by_name (exec_bfd, ".text");

info->section should be valid, but why must it be ".text"?  Can I disassembly
instructions in .plt section?  How about using find_pc_section (addr) to get
the right section?  like mep_gdb_print_insn.
  
Anton Kolesov Feb. 16, 2017, 4:35 p.m. UTC | #2
Hi Yao,

> info->section should be valid, but why must it be ".text"?  Can I

> info->disassembly

> instructions in .plt section?  How about using find_pc_section (addr) to get

> the right section?  like mep_gdb_print_insn.


Actual section doesn't matter here because opcodes really need elf header, which is
just obtained through a section. I've used .text simply because it is the section that is
very likely to exist. Using find_pc_section seems to be a much better solution, I wasn't
aware of it. Will change this in second version of the patch.

Anton

> 

> --

> Yao (齐尧)
  
Pedro Alves Feb. 17, 2017, 12:31 p.m. UTC | #3
On 02/16/2017 04:35 PM, Anton Kolesov wrote:
> Hi Yao,
> 
>> info->section should be valid, but why must it be ".text"?  Can I
>> info->disassembly
>> instructions in .plt section?  How about using find_pc_section (addr) to get
>> the right section?  like mep_gdb_print_insn.
> 
> Actual section doesn't matter here because opcodes really need elf header, which is
> just obtained through a section. I've used .text simply because it is the section that is
> very likely to exist. Using find_pc_section seems to be a much better solution, I wasn't
> aware of it. Will change this in second version of the patch.

And even better approach would be to include the right arch in
the xml target description, in the (existing) <architecture> element, and
then somehow pass that info down to opcodes here...

Thanks,
Pedro Alves
  
Anton Kolesov March 15, 2017, 3:16 p.m. UTC | #4
> 

> On 02/16/2017 04:35 PM, Anton Kolesov wrote:

> > Hi Yao,

> >

> >> info->section should be valid, but why must it be ".text"?  Can I

> >> info->disassembly

> >> instructions in .plt section?  How about using find_pc_section (addr)

> >> to get the right section?  like mep_gdb_print_insn.

> >

> > Actual section doesn't matter here because opcodes really need elf

> > header, which is just obtained through a section. I've used .text

> > simply because it is the section that is very likely to exist. Using

> > find_pc_section seems to be a much better solution, I wasn't aware of it.

> Will change this in second version of the patch.

> 

> And even better approach would be to include the right arch in the xml

> target description, in the (existing) <architecture> element, and then

> somehow pass that info down to opcodes here...


In our BFD ARC EM and ARC HS are treated as same processor (ARC v2) and
differ via flags in private ELF header, so changing BFD arch will not work.
I'd need to change opcodes, so that it will accept processor as an argument,
instead of always reading ELF header. That would take a while, so I will
remove this patch from the series and will resubmit it later, separately
from prologue analysis.  The only downside is that several architecture
specific test cases will not pass for ARC HS processor for the time being.

Anton

> 

> Thanks,

> Pedro Alves
  

Patch

diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index f78e3a9..e4b2e7a 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -705,6 +705,16 @@  arc_delayed_print_insn (bfd_vma addr, struct disassemble_info *info)
      will handle NULL value gracefully.  */
   print_insn = arc_get_disassembler (exec_bfd);
   gdb_assert (print_insn != NULL);
+
+  /* ARC disassembler requires that info->section is valid - it will be used to
+     access program headers of ELF files that is needed to distinguish between
+     ARC EM and HS (ARC600, ARC700 and ARC v2 can be distinguished by BFD's
+     `mach` attribute).  Without this information disassembler will default to
+     ARC EM for ARC v2 targets, and it will not recognize instructions specific
+     to ARC HS, for example, double-word store and load.  */
+  if (exec_bfd != NULL)
+    info->section = bfd_get_section_by_name (exec_bfd, ".text");
+
   return print_insn (addr, info);
 }