GDB/opcodes: Remove arch/mach/endian disassembler assertions

Message ID 86pocc3oam.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Aug. 3, 2017, 11:04 a.m. UTC
  "Maciej W. Rozycki" <macro@imgtec.com> writes:

Hi Maciej,

> @@ -971,13 +971,6 @@ default_print_insn (bfd_vma memaddr, dis
>  {
>    disassembler_ftype disassemble_fn;
>  
> -  if (exec_bfd != NULL)
> -    {
> -      gdb_assert (info->arch == bfd_get_arch (exec_bfd));
> -      gdb_assert (info->endian == (bfd_big_endian (exec_bfd)
> -				   ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE));
> -      gdb_assert (info->mach == bfd_get_mach (exec_bfd));
> -    }
>    disassemble_fn = disassembler (info->arch, info->endian == BFD_ENDIAN_BIG,
>  				 info->mach, exec_bfd);

I prefer to keep these asserts, as they could check the inconsistent
uses of disassemble_info between gdb and opcodes.  There is another
assert for target armv5te, PR 21818, but the assert exposes an improper
or unexpected usage of disassemble_info.mach in opcodes/arm-dis.c.  I am
fixing it.

If these asserts make few sense to mips, gdb_print_insn_mips doesn't
have to call default_print_insn.  You can just do this at the end of
gdb_print_insn_mips,
  

Comments

Maciej W. Rozycki Aug. 3, 2017, 11:36 a.m. UTC | #1
On Thu, 3 Aug 2017, Yao Qi wrote:

> I prefer to keep these asserts, as they could check the inconsistent
> uses of disassemble_info between gdb and opcodes.  There is another
> assert for target armv5te, PR 21818, but the assert exposes an improper
> or unexpected usage of disassemble_info.mach in opcodes/arm-dis.c.  I am
> fixing it.

 How does the retaining of these assertions interact with `set
architecture' and `set endian' then?

  Maciej
  
Maciej W. Rozycki Aug. 3, 2017, 12:24 p.m. UTC | #2
On Thu, 3 Aug 2017, Maciej W. Rozycki wrote:

> > I prefer to keep these asserts, as they could check the inconsistent
> > uses of disassemble_info between gdb and opcodes.  There is another
> > assert for target armv5te, PR 21818, but the assert exposes an improper
> > or unexpected usage of disassemble_info.mach in opcodes/arm-dis.c.  I am
> > fixing it.
> 
>  How does the retaining of these assertions interact with `set
> architecture' and `set endian' then?

 E.g.:

$ x86_64-linux-gnu-gdb
GNU gdb (GDB) 8.0.50.20170729-git
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) file main86
Reading symbols from main86...(no debugging symbols found)...done.
(gdb) show architecture
The target architecture is set automatically (currently i386:x86-64)
(gdb) disassemble main
Dump of assembler code for function main:
   0x0000000000400450 <+0>:	xor    %eax,%eax
   0x0000000000400452 <+2>:	retq
End of assembler dump.
(gdb) set architecture i386
The target architecture is assumed to be i386
(gdb) disassemble main
Dump of assembler code for function main:
   0x00400450 <+0>:
.../gdb/arch-utils.c:979: internal-error: int default_print_insn(bfd_vma, disassemble_info*): Assertion `info->mach == bfd_get_mach (exec_bfd)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) n

This is a bug, please report it.  For instructions, see:
<http://www.gnu.org/software/gdb/bugs/>.

.../gdb/arch-utils.c:979: internal-error: int default_print_insn(bfd_vma, disassemble_info*): Assertion `info->mach == bfd_get_mach (exec_bfd)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n) n
Command aborted.
(gdb)

A change applied to the MIPS backend obviously won't do anything here.

  Maciej
  
Yao Qi Aug. 4, 2017, 10:49 a.m. UTC | #3
"Maciej W. Rozycki" <macro@imgtec.com> writes:

> (gdb) set architecture i386
> The target architecture is assumed to be i386
> (gdb) disassemble main
> Dump of assembler code for function main:
>    0x00400450 <+0>:
> .../gdb/arch-utils.c:979: internal-error: int default_print_insn(bfd_vma, disassemble_info*): Assertion `info->mach == bfd_get_mach (exec_bfd)' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) n

I didn't notice this example in your email.  The gdb patch is good to me.
  

Patch

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index c1800e4..8ab7886 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -7025,7 +7025,13 @@  gdb_print_insn_mips (bfd_vma memaddr, struct disassemble_info *info)
        register naming conventions specified by the user.  */
     info->disassembler_options = "gpr-names=32";
 
-  return default_print_insn (memaddr, info);
+  disassembler_ftype disassemble_fn;
+
+  disassemble_fn = disassembler (info->arch, info->endian == BFD_ENDIAN_BIG,
+				 info->mach, exec_bfd);
+
+  gdb_assert (disassemble_fn != NULL);
+  return (*disassemble_fn) (memaddr, info);
 }
 
 static int