[PATCH] Allow mips64 code compiled with -mgp32 to be debuggable

[Andrew Cagney]

Jonathan Larmour wrote:
> 
> Andrew Cagney wrote:
> >
> > Is GDB able to determine this specific setting of ``saved regsize'' from
> > just the executable?  Some ELF flag?  If GDB can't then things should be
> > put into play so that GDB can.  Perhaphs it already is?
> 
> Ah, that's an interesting question. There *is* a flag EF_MIPS_32BIT_MODE.
> But gas/config/tc-mips.c contains the comment:
> 
>       /* We deliberately don't allow "-gp32" to set the MIPS_32BITMODE
>          flag in object files because to do so would make it
>          impossible to link with libraries compiled without "-gp32".
>          This is unnecessarily restrictive.
> 
>          We could solve this problem by adding "-gp32" multilibs to
>          gcc, but to set this flag before gcc is built with such
>          multilibs will break too many systems. */
> 
> But even more unhelpfully, the default linker output target for mips64-elf
> is elf32-bigmips, and has EF_MIPS_32BIT_MODE set, even for 64-bit programs
> - you need to change the ABI to change this, and not all gcc targets
> support that currently. Therefore using this flag to detect 32-bit mode
> would do the wrong thing given the current gcc - it would make GDB treat
> these 64-bit programs as 32-bit programs by default.
> 
> I should set in motion changes to gcc to resolve this (although adding a
> -mgp32 multilib has been resisted in the past, so it may not happen). But
> even so, we will have to deal with binaries from current compilers which
> don't really do the right thing.
> 
> So, what I'm getting at is that there *is* a flag, but it would be a
> mistake to use it right now. Maybe later.
> 
> > I think the command is to specific.  It should instead allow the user to
> > specify any of:
> >
> >         auto    GDB determine value from
> >                 executable and other information
> >
> >         32      GDB push 32 bit integer registers
> >
> >         64      GDB push 64 bit integer registers
> >
> > That way, we don't end up with a random collection of commands that all
> > twiddle the same knob.  Eventually other knobs to control other aspects
> > of the MIPS abi will be needed this is just the first one.
> > (Check how enums are handled by GDB - remote.c and the protocol packet).
> 
> Okay, done.
> 
> > Regarding the specific code:
> >
> > > +#define MIPS_SAVED_REGSIZE (mips_force_32bit_saved_regsize != 0 ? 4 : \
> > > +                           MIPS_DEFAULT_SAVED_REGSIZE)
> >
> > once the above is changed, this would be much better implemented as a
> > function.
> 
> Done that too.
> 
> > Regarding the name, I'm eventually going to have to deprecate the
> > command as it currently stands and replace it with one with the form:
> >
> >         (gdb) set mips ....
> >
> > (This bit isn't your problem, however :-).
> 
> Ah, well, there's no point me adding a command I know will be deprecated
> :-). So I've decided to be helpful and I've added the "set mips" prefix
> command. So the new command is actually "set mips saved-gpreg-size
> {auto,32,64}". This way at least if people add any more MIPS-specific
> commands, it won't be increasing the problem and the pain later. Hope this
> helps :-).
> 
> > What are the testsuite results like with your change applied and
> > enabled?
> 
> Enabled? By hand, yes, but not the testsuite surely - it depends on how you
> compile programs you pass to it, so I don't quite see what you're getting
> at. But in case this is what you want, I have run the GDB testsuite with
> the vr4300-sim target, and there were no changes in the results. I have of
> course tested this by hand, including invoking all of the new commands with
> help, and so on.
> 
> One thing I'm not happy about is that all of mips-tdep.c is in K&R. I don't
> feel up to ANSIfication of it all, so I've followed the existing style
> rather than have pockets of ANSI C dotted in the middle of it.
> 
> Let me know what you think. The ChangeLog may be more verbose than
> necessary - I'm never sure where to draw the line.

It's fine.  You've managed to mention every variable/function that was
added they will be easy to track down.

One thing,

> -#ifndef MIPS_SAVED_REGSIZE
> -#define MIPS_SAVED_REGSIZE MIPS_REGSIZE
> +#ifndef MIPS_DEFAULT_SAVED_REGSIZE
> +#define MIPS_DEFAULT_SAVED_REGSIZE MIPS_REGSIZE

>  #if GDB_MULTI_ARCH
> -#undef MIPS_SAVED_REGSIZE
> -#define MIPS_SAVED_REGSIZE (gdbarch_tdep (current_gdbarch)->mips_saved_regsize)
> +#undef MIPS_DEFAULT_SAVED_REGSIZE
> +#define MIPS_DEFAULT_SAVED_REGSIZE (gdbarch_tdep (current_gdbarch)->mips_saved_regsize)
>  #endif

>      default:
>        ef_mips_abi = "default";
>        tdep->mips_eabi = 0;
> -      tdep->mips_saved_regsize = MIPS_REGSIZE;
> +      tdep->mips_saved_regsize = MIPS_DEFAULT_SAVED_REGSIZE;
>        tdep->mips_fp_register_double = (REGISTER_VIRTUAL_SIZE (FP0_REGNUM) == 8);
>        set_gdbarch_long_bit (gdbarch, 32);
>        set_gdbarch_ptr_bit (gdbarch, 32);
>        set_gdbarch_long_long_bit (gdbarch, 64);
>        break;

Given the above, I think that line is wrong.  It should continue to be
MIPS_REGSIZE.  I've committed the patch with two additions:

	o	fixed the above line

	o	renamed tdep->mips_saved_regsize to tdep->mips_default_saved_regsize

	o	added an extract of your MIPS32 notes to the file

Andrew