[RFA 1/2] mips: Switch inferior function calls to ON_STACK method.

[Maciej W. Rozycki]

On Fri, 4 May 2012, Joel Brobecker wrote:

> Now I know why I was told you are a MIPS expert :-). I never really
> had the chance or need to delve into the details of any specific
> architecture. Even for the ia64-hpux port, I could do with just
> a superficial knowledge of that CPU.

 It just started the other way round for me -- I got pulled into toolchain 
hacking as a result of my hopeless attempts to bring up the then current 
version of Linux kernel on my DECstation box (a MIPS R3000 class system).  
Of course you can't really use a kernel without userland and as it turned 
out that meant porting upcoming glibc 2.2 unless I wanted to downgrade 
back to long obsolete 2.0 (which I did not).

 Of course binutils at that point did not support symbol versioning in the 
MIPS port yet (it couldn't use the generic ELF versioning stuff all the 
other ports used because of some peculiarities of the IRIX ELF variation), 
so I had to fix that first and that's how it started rolling...  So I'm 
really a low-level kernel developer who just happens to do something else. 
;)

 I still have that DECstation and can check any original MIPS I ISA stuff 
at the run time should there be such a need.

> > * newly-added 48-bit instructions.
> 
> I am wondering if this addition is going to hurt in terms of our
> support...  From what I could tell from my mips64 manual, even
> on this CPU the instructions are still 32bit long... But I'm
> digressing, sorry.

 My GDB patch handles it just fine as the major opcode for any 48-bit 
instructions has been already allocated by the architecture (you can see 
some parts in my change that classify instructions according to their 
length for example), except there has been no hardware or simulator 
available to test it yet.  QEMU might be closest to getting there.  For 
the purpose of software breakpoints any 48-bit instruction is I reckon 
replaced with a 32-bit breakpoint.  This is safe because 48-bit 
instructions cannot be placed in any fixed-width branch delay slots, so 
the choice of the width of the breakpoint does not matter (not that we put 
any breakpoints in delay slots anyway, but it doesn't hurt to play safe 
anyway).

 Worse, however, that the lack of permission for C99 code in opcodes 
precludes the use of the long long type to hold instruction patterns and 
opcode masks there.  And redefining "struct mips_opcode" to split 
instructions/masks across multiple members (or worse yet, using some hack 
to split them acros multiple opcode table entries) means rewriting lots of 
stuff at least in opcodes itself and in GAS.

 That's sort of discouraging for a single instruction (and no 64-bit 
microMIPS hardware yet), especially given that I think we will eventually 
permit at least the type itself if not other C99 features sometime.  This 
is even more disappointing given these members are defined as the long 
type -- already wasting 32-bits twice per each entry throughout the table 
on 64-bit hosts...

 The end result is binutils do not support the LI32 instruction now, so 
while we do in GDB, there's actually no reasonable way to produce it (of 
course you can still handcode it as data with the .half directive -- note 
that just like MIPS16 extended instructions (and unlike standard MIPS 
ones) 32-bit and 48-bit microMIPS instructions are encoded as sequences of 
16-bit halfwords each of which is in the processor's endianness).

> >  Coincidentally all-zeroes is a 32-bit NOP instruction both in the 
> > standard MIPS and the microMIPS mode -- there's a 16-bit encoding of NOP 
> > in the microMIPS mode naturally as well.
> 
> I'm wondering if you'd like me to rename "null_insn" into "nop_insn"
> in my patch. I didn't do it, because I'd expect the instruction size
> to depend on the mode. As of today, we know that the breakpoint we
> are inserting is always going to be at an even address, so it's always
> going to be 4 bytes. So maybe it does make sense to rename it. Let
> me know.

 I think "nop_insn" might be better; we handle different encodings of the 
NOP intruction in GAS already (used for .align padding among others) but 
from my understanding of this piece this is never going to be necessary 
here as we'll only ever need a standard MIPS and a 32-bit microMIPS 
breakpoint here.  I think however that initialising it like this:

  static gdb_byte nop_insn[] = { 0, 0, 0, 0 };

may express the intent a bit better (this is what we do in 
mips_breakpoint_from_pc where we initialise all array elements explicitly 
even where we could let the compiler default to zeros).

> >  Understood, but I'd be happier if the comment you're removing or a 
> > similar stayed in place.  If by trap you mean SIGTRAP, then I think this 
> > is not going to be the case.
> 
> I think you refer to the comment from Andrew Cagney? I've put it back
> as is.

 Yes, however following Mark's note I think it'll be better to state that 
it's OK if we get SIGSEGV for a non-executable stack and refer to 
handle_inferior_event (look for SIGSEGV there for a detailed explanation).  
Furthermore, given that it is supported I think it should simply be 
documented in the function's description instead.

> OK to commit, modulo the possible rename above?

 OK with these changes; once you've checked your patch in I'll update the 
outstanding microMIPS change according to my earlier notes (and will 
probably come back with issues and haunt everybody here).

  Maciej