Re: [RFA] MIPS/GDB: Fix the handling of MIPS16 thunks

["Maciej W. Rozycki" <macro at codesourcery dot com>]

Hi Pedro,

[Cc-ing Daniel for the question below.]

> > +  /* macro/2005-03-31: This hack skips over MIPS16 call thunks as
> 
> 
> Eh, that long?  Might want to update that date to the check in date,
> or drop it.  Otherwise it's just useless, IMO.

 Yes, sigh...  And fair enough :) -- I'll tweak the date.  The comment 
might need updating once new GCC's unwind information Richard mentioned 
has been tested.

 There's another note, that is even older, by Daniel -- Daniel, would you 
mind if I updated the date on your note to the check in date too -- IOW, 
do you continue to stand by it?

> > +
> > +static int mips_is_stub_suffix (const char *suffix, int zero)
> 
> Function name at column 0.
> 
> > +
> > +static int mips_is_stub_mode (const char *mode)
> > +{
> 
> Ditto.

 Fixed, thanks.

> > Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.arch/mips16-thunks-sin.c
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.arch/mips16-thunks-sin.c	2012-04-05 21:42:24.995424172 +0100
> > @@ -0,0 +1,38 @@
> > +#include <math.h>
> > +
> 
> I've noticed the tests miss copyright headers.

 Fixed.  I wasn't sure, some existing tests don't have either.  Not that 
this program is any useful code ;) -- although it does certainly compute 
"something".

> > +gdb_breakpoint inmain
> > +gdb_run_cmd
> > +gdb_expect 30 {
> > +    -re "Breakpoint 1.*inmain .*$gdb_prompt $" {
> > +	send_gdb "finish\n"
> > +	gdb_expect {
> > +	    -re "Value returned is \\\$\[0-9\]+ = 0\[^0-9\].*$gdb_prompt $" {
> 
> Can we use gdb_test_multiple (catching internal errors etc., and dropping
> the default cases) ?  (other instances)

 I'm not sure -- I looked at gdb_test_multiple and it appeared to me you 
can't avoid scoring failures for cases the test considers to be just 
symptoms of an unsupported feature that we want to silently ignore; I'll 
recheck.

> > +# Single-step through the function that is at the head of function list
> > +# FUNCS until a different function (frame) is reached.  Before each step
> > +# check the backtrace against FUNCS.  ID is used for reporting, to tell
> > +# apart different calls to this procedure for the same function.  If
> > +# successful, then return the name of the function we have stopped in.
> > +proc step_through { id funcs } {
> 
> Not sure it'd be useful here, but note the new with_test_prefix routine,
> which we now use as a convenient way to make sure there are no
> duplicate messages in gdb.sum (, as in
> <http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique>).

 ISTR looking at it; I decided to go for a monotonically increasing ID 
instead that notes the stage of the test as there are too many repeating 
test messages IMO.  You can refer to the place in the .exp script easily 
by the ID as I noted these IDs thoroughout (which I consider valuable as 
chasing failures I get frequently lost in the maze of some tests that are 
not simply a linear flow of gdb_test or suchlike calls).

 I think getting prefixes right would be difficult and error-prone; note 
for example that some messages may appear more or fewer times depending on 
whether your system library has debugging enabled or not.  And this is 
actually intentional -- to check if PLT or MIPS SVR4 stubs are handled 
correctly in the context of MIPS16 thunks; I decided not to build a shared 
library as a part of this test at this stage not to overcomplicate what is 
already quite complex -- I might revisit it in the future, but I think it 
better was a separate test case then.  As a side-effect this arrangement 
triggered the MIPS32 JALX decoding bug I fixed a couple days ago, so it 
seems to be doing its work quite well.

 Thanks for your feedback.  I'll wait for a while for any further notes 
from other people on the infrun.c change.

  Maciej