This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] microMIPS support
On Wed, 25 Apr 2012, Joel Brobecker wrote:
> I could only scan the patch briefly for today, but noticed something
> that caught my attention:
>
> > +/* For backwards compatibility we default to MIPS16. This flag is
> > + overridden as soon as unambiguous ELF file flags tell us the
> > + compressed ISA encoding used. */
> > +static const char mips_compression_mips16[] = "mips16";
> > +static const char mips_compression_micromips[] = "micromips";
> > +static const char *mips_compression_strings[] = {
> > + mips_compression_mips16,
> > + mips_compression_micromips,
> > + NULL
> > +};
>
> We usually provide this sort of feature a little differently: Instead
> of 2 values that are adjusted automatically by the debugger, we provide
> 3 values: auto, mips16, and micromips. If auto, then the debugger has
> to guess, possibly defaulting to mips16 if guessing did not work.
> But if the user sets the setting to either of the non-auto values,
> then the setting should be honored, even if the user is wrong.
>
> This is usually implemented using two variables: One representing
> the setting, and one representing the actual value.
I have been aware of that and decided the model does not fit. Please
note that this setting is a fall-back that's never used when you have an
executable available. If you do have an executable, then the ISA mode of
compressed code is always known, because it's recorded in two places:
individual symbol's ELF st_other flags, and globally, in the ELF file
header.
The latter is actually optional, in the sense that originally the
compressed ISA was not recorded at all. However all microMIPS objects
must have the microMIPS flag set and therefore any compressed code in an
executable that does not have any compressed flag set must be MIPS16 code
in a legacy object.
It's the st_other flags that's normally used by GDB to determine the
compressed ISA kind, the ELF header flags are only checked for pieces of
code that have no symbol information available for some reason. And there
is no point in overriding the ISA mode mandated by the executable, it must
be clearly wrong, and there is really nothing to "guess" for GDB here --
all the required information is always available.
Therefore the "auto" setting is meaningless -- you cannot infer any mode
in the scope this setting has any use for, that is debugging with no
executable (e.g. connecting to a live remote target), which is the very
point of providing this setting at all. Also note that this setting is
"sticky", that is it's set from any executable selected. If the
executable is later on discarded, then the last setting is preserved
assuming that if the user worked with one compressed ISA before it's more
likely that they still use that ISA and not the other for non-debug code.
Note this is a corner case too, but I decided it has to be handled
somehow for robustness of GDB -- e.g. you may want to step-through
power-on firmware you have no matching executable for via a JTAG probe
for some reason.
> This brings me to the next question: Could this be an objfile-specific
> setting? In other words, is it possible to that the same executable
> might have one objfile using micromips while another might still be
> using mips16? (this might be the stupidest question of the week...).
> If not, I still believe that this is at least an inferior-specific
> property. With multi-inferior debugging being possible, I can see
> how one debugs two programs using a different setting. In that case,
> you need to store that information in the inferior private data
> (I do that in ada-tasks, IIRC, for storing the layout of some data
> structures).
The question is of course fine and actually asked before in the process
of adding microMIPS support to binutils. There is no way for MIPS16 and
microMIPS code to be present in a single executable. We mandated this in
binutils deciding that it would be a corner case not worth handling.
Background information: the architecture does not permit the MIPS16 and
the microMIPS ASE to be implemented in a single processor, one precludes
the other (note that the architecture does permit an implementation that
only supports the microMIPS mode and not the "traditional" standard MIPS
mode, known since ~1985). Therefore the only case where an executable
could contain both MIPS16 and microMIPS code would be a piece of software
that dynamically determines which of the two ASEs is present on a
processor and switch to the right set of procedures on the fly (this is
similar to what some power-on firmware does to handle both endiannesses
with a single image on systems where you can switch the endianness on the
fly, either with a physical switch or via some board logic). We decided
this is too much complication for little possibility of this case to ever
matter for anyone.
And personally I think the case of mixing MIPS16- and microMIPS-capable
processors in a multi-processor systems (for multi-inferior debugging) is
very unlikely too. I don't think anyone has plans to support such a
system, that would sort of be missing the point (the microMIPS ASE is
intended to replace the MIPS16 ASE in deployments where code density
matters), not even mentioning this would most likely require different
base processor types too as I don't expect any single MIPS processor to
support the choice between the MIPS16 ASE and the microMIPS ASE as RTL
options (currently only the MIPS M14K and M14Kc processors support the
microMIPS ASE and neither base architecture supports the MIPS16 ASE as an
option).
So until (or really unless) somebody comes with the requirement to
support both the microMIPS and the MIPS16 ASE at a time in a single debug
session let's keep this simple.
> (oops, style issue as well, the opening curly bracket should be
> at the start of the next line - we've seen a lot of your style too,
> but I think it should be fixed)
Yes, I've changed that, thanks. Actually the very preceding array uses
the very style I did. ;)
Actually I keep getting confused about the style expected for aggregate
types, especially in the context of initialisers. So for example is this
correct:
static int
foo (void)
{
struct
{
int i;
void *p;
}
s[] =
{
{ 1, &foo },
{ 2, NULL }
};
}
or should that be written yet differently? What if that's defined at the
file scope:
struct bar
{
int i;
void *p;
}
s[] =
{
{ 1, &foo },
{ 2, NULL }
};
or:
struct bar
{
int i;
void *p;
}
s[] =
{
{ 1, &foo },
{ 2, NULL }
};
? I recall seeing both styles (plus some variations), but my inclination
is not to give indentation to curly brackets at the file level in any
cases -- compare global functions vs nested functions (a GCC extension).
Then this gets even weirder with C99's compound literals, e.g. this piece
gets tricky:
void
baz(struct bar b[])
{
}
void
bax(void)
{
baz ((struct bar[]) { { .i = 3, .p = &bax }, { .i = 4, .p = NULL } });
}
when you need to wrap the line (e.g. add third element), sigh...
Maciej