Re: [PATCH] Abi selection based on set arch command in case of remote debugging for MIPS

["Maciej W. Rozycki" <macro at linux-mips dot org>]

Simon,

 Apologies for the long RTT, I've come across your e-mail only now, while 
routinely reviewing general mailing list traffic received.  I think my 
mips.com address does not work anymore (did you get a bounce?), and even 
if it does, I have no access to whatever system collects incoming messages 
there.

> Just wondering, would this patch still fall under your maintainership, and do
> you plan on reviewing it?  I think it's a followup this, which should give more
> context:
> 
>   https://sourceware.org/ml/gdb-patches/2018-10/msg00342.html

 Thanks for the pointer.  I suppose I could look into it as a general 
issue, even though Octeon specifically used here is not a legacy MIPS 
implementation, but the change proposed makes no sense to me I am afraid.  
You can't infer the ABI just from the architecture and whatever you choose 
may work with one remote debug stub and then break with another (e.g. o32 
Linux `gdbserver' will exchange register data in 32-bit quantities and use 
32-bit addressing even with a 64-bit processor).

 Also AFAIK the embedded MIPS ABIs have been obsolete for years now; I 
haven't seen any use of them for well over a decade now, so I recommend 
against making any new use of them.

 The correct solution to get the address and register size right in GDB is 
to communicate it with an XML target description, and this is what I'd 
suggest implementing in the debug stub, as it is the stub that knows what 
it communicates to GDB.  Then GDB can present that information according 
to the ABI selected, either manually or inferred from the ELF file 
selected for debugging, however in the absence of an ELF file GDB still 
won't be able to guess the ABI (we might consider having a different 
default selected somehow at build time, although o32 has the benefit of 
being compatible with all MIPS hardware).

 It would be good to know in detail though what the original problem was 
that the change proposed was intended to solve, as my proposal for the 
solution has been based on a guess and may not actually address what has 
been causing troubles.

> > +      {
> > +        elf_flags = E_MIPS_ABI_EABI64;
> > +      }
> > +      else if (info.bfd_arch_info->bits_per_address == 32)
> > +      {
> > +        elf_flags = MIPS_ABI_EABI32;
> > +      }
> 
> When there is a single statement, drop the braces:
> 
>   if (...)
>     elf_flags = E_MIPS_ABI_EABI64;
>   else
>     elf_flags = MIPS_ABI_EABI32;

 This clearly hasn't been even barely tested :( as `E_MIPS_ABI_*' and 
`MIPS_ABI_*' groups of constants have distinct uses each.

  Maciej