This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] mips-tdep.c: Fix mips16 bit rot
- From: Kevin Buettner <kevinb at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 14 Dec 2011 16:21:17 -0700
- Subject: Re: [RFC] mips-tdep.c: Fix mips16 bit rot
- References: <20101213170635.6f0c4356@mesquite.lan> <alpine.DEB.1.10.1111231201520.4191@tp.orcam.me.uk>
Hi Maciej,
Sorry it's taken me so long to get back to you this. I will try to
answer your questions to the best of my ability, but it's been a
while since I've looked at or thought about this code. I fear
that I might not be of much help...
On Wed, 23 Nov 2011 12:40:37 +0000
"Maciej W. Rozycki" <macro@codesourcery.com> wrote:
> First of all this construct used in a few places:
>
> + if (is_mips16_addr (pc))
> + pc = unmake_mips16_addr (pc);
>
> makes me a bit nervous you might be removing the ISA bit for code
> references where it is the only means of signalling GDB the address is a
> MIPS16 code address -- that would happen if pc pointed to a location that
> has no associated symbol information. While in this case the
> functionality GDB provides is limited, it still has to work correctly up
> to expectations, e.g. instruction-level single-stepping has to work and
> where software stepping is used the MIPS16 BREAK instruction encoding has
> to be used rather than the MIPS32 one. Have you verified this
> functionality has not regressed? Similarly the MIPS16 heuristic frame
> unwinder may be affected.
>
> Otherwise I'd just be tempted to change all these cases into something
> functionally equivalent to:
>
> + if (msymbol_is_special (lookup_minimal_symbol_by_pc (pc)))
> + pc = unmake_mips16_addr (pc);
>
> where the ISA bit is only stripped if there's symbol information
> indicating this is a piece of MIPS16 code so there's no information lost.
I see your point. You proposed change looks reasonable to me.
> Second, you only made corresponding adjustments to
> mips_eabi_push_dummy_call and mips_o64_push_dummy_call which are the least
> standard and probably the least used MIPS ABIs. Any particular reason you
> did not make similar changes to mips_o32_push_dummy_call or
> mips_n32n64_push_dummy_call?
>
> Also I see you only adjust function pointers that are arguments on their
> own -- isn't a similar adjustment required for such pointers that are
> parts of aggregate types as well?
I don't remember enough about what I did roughly a year ago to be able
to answer this. I think it's likely that changes should be made to the
areas that you've identified.
> Overall, what was the rationale behing your change? -- as it's unclear to
> me from your e-mail. Did you just want to fix test results you discovered
> that were quite poor or does this change address problems you stumbled
> across during actual MIPS16 debugging?
The former - I was attempting to fix some very bad test results with respect
to mips16.
Kevin