This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH 2/6] Delegate opcodes to select disassembler in GDB
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: <binutils at sourceware dot org>, <gdb-patches at sourceware dot org>
- Date: Thu, 6 Jul 2017 00:53:18 +0100
- Subject: Re: [PATCH 2/6] Delegate opcodes to select disassembler in GDB
- Authentication-results: sourceware.org; auth=none
- References: <email@example.com> <firstname.lastname@example.org> <alpine.DEB.email@example.com> <firstname.lastname@example.org>
On Fri, 30 Jun 2017, Yao Qi wrote:
> > This change causes an assertion failure when trying to disassemble a
> > MIPS16 function:
> > (gdb) disassemble main
> > Dump of assembler code for function main:
> > 0x00400209 <+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)
> Sorry about that. I did deliberately run
> testsuite/gdb.base/all-architectures-[0-7].exp tests to cover the case
> that using disassembler for each architecture. It covers mips, but it
> doesn't cover mips16 and micromips.
These are smoke tests really AFAICT, and for this to trigger they would
have to cover the usual case where the gdbarch's BFD is different from one
chosen for the disassembly, and for that `set architecture <foo>' is (or
at least may not be) enough. You'd really have to run full MIPS
regression testing with a MIPS16 or microMIPS multilib, and I realise you
may not have the necessary infrastructure available at hand.
> > This is because `info->mach' is 16 (the `bfd_mach_mips16' aka `mips:16'
> > BFD) whereas `bfd_get_mach (exec_bfd)' is 33 (the `bfd_mach_mipsisa32r2'
> > aka `mips:isa32r2' BFD). This is expected for MIPS16 code within a
> > program that has been built for the MIPS32r2 ISA; see
> > `gdb_print_insn_mips' for the origin.
> > So what's the purpose of this assertion?
> The assertion is based on assumption that "info->mach" is got from
> bfd_get_mach (exec_bfd), but it is not right for mips16. We can remove
> this line. If you agree, I'll post a patch to remove this line.
I can see the assumption from the assertion itself, but what is its
Normally you place an assertion in code to guard against a case that is
not handled (correctly) and if let through it would lead execution to go
astray, e.g. because it is a new complicated feature we have not yet got
to implementing or because it is a case we think that will never happen,
and code that follows has assumptions that will not be met.
So if you say that you can remove the assertion with no adverse effect on
processing, then I think it should not have been placed there in the first
Anyway, if you look at code in `gdb_print_insn_mips', then you'll find
/* FIXME: cagney/2003-06-26: Is this even necessary? The
disassembler needs to be able to locally determine the ISA, and
not rely on GDB. Otherwize the stand-alone 'objdump -d' will not
and it is indeed the case that `objdump' handles this correctly without
the need to switch away from the BFD selected for the binary being
handled. However in GDB we have this problem that we do not pass the
symbol table down to libopcodes for the disassembler, and in the case of
the MIPS target it is the symbol table that carries information about
which functions contain regular MIPS code, MIPS16 code and microMIPS code
Lacking symbol information we resort to this hack of overriding the BFD
for the purpose of disassembly only, and this has the adverse effect of
getting instruction subsetting wrong: the `bfd_mach_mips16' and
`bfd_mach_mips_micromips' BFDs always choose the maximum instruction set
supported possible whereas the binary handled may only support a subset
(or worse yet an alternative set), as indicated by the original BFD
selected. This in turn may confuse and mislead the person running a debug
session into thinking code disassembled is not the problem they are after.
Do you think it would be possible as a part of your disassembler rework
to make the symbol table available to libopcodes? Then the hack currently
present in `gdb_print_insn_mips' could go. I remember looking into it a
while ago and concluding it would be somewhat tricky because the symbol
table format expected by libopcodes is not the same that we use in GDB.