Re: [RFC] mips-tdep.c: Fix mips16 bit rot

[Kevin Buettner <kevinb@redhat.com>]

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