From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4722 invoked by alias); 5 Jul 2017 23:53:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 4698 invoked by uid 89); 5 Jul 2017 23:53:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:Thu X-Spam-User: qpsmtpd, 2 recipients X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Jul 2017 23:53:31 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id 778A3FE40358D; Thu, 6 Jul 2017 00:53:23 +0100 (IST) Received: from [10.20.78.94] (10.20.78.94) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Thu, 6 Jul 2017 00:53:27 +0100 Date: Wed, 05 Jul 2017 23:53:00 -0000 From: "Maciej W. Rozycki" To: Yao Qi CC: , Subject: Re: [PATCH 2/6] Delegate opcodes to select disassembler in GDB In-Reply-To: <86efu1diwp.fsf@gmail.com> Message-ID: References: <1494931698-15309-1-git-send-email-yao.qi@linaro.org> <1494931698-15309-3-git-send-email-yao.qi@linaro.org> <86efu1diwp.fsf@gmail.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2017-07/txt/msg00035.txt.bz2 Hi Yao, 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 ' 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 purpose? 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 place. Anyway, if you look at code in `gdb_print_insn_mips', then you'll find this comment: /* 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 work. */ 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 respectively. 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. Maciej