Re: [RFA] MIPS16 FP manual call/return fixes

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

On Tue, 1 May 2012, Jan Kratochvil wrote:

> > +  find_pc_partial_function_gnu_ifunc (b->loc->address, NULL,
> > +                                     &func_func_addr, NULL, &is_gnu_ifunc);
> > +  gdb_assert (is_gnu_ifunc);
> 
> Please remove that gdb_assert and fall back somehow, IMO just pass 
> FUNCTION as NULL in such case.

 It may not matter for most platforms, but if that ever happens for one of 
the MIPS ABIs affected, then this will fail an assertion in the backend 
instead.  It is invalid to call any of these backends with NULL FUNCTION 
and non-NULL READBUF both at a time, because the handler needs to retrieve 
the return value from the correct registers and it can only do that when 
it knows the address of the function called.

>  Any resolving ADDRESS->SYMBOL can be ambiguous with weird symbol files 
> and it may find some overlapping non-IFUNC symbol instead. Also user may 
> have unloaded symbol files in the middle of the debugging etc.

 OK, so let me put this another way.  Here we have just returned from a 
function that we called.  Obviously that function does exist somewhere no 
matter if symbol tables have been unloaded or whatever.  How can we 
determine the address of that function? -- in a different way, presumably.  
We must have somehow established its address previously or we couldn't 
have called it.  Is it possible to cache it somehow for example?

 Thanks for raising the issue of unloading symbol tables, that's an 
important point as to how MIPS16 and microMIPS symbols should be treated 
in general -- here I think it will only matter in the asynchronous mode. 
This is because in the synchronous if a function is called manually and it 
traps, then my observation is that any return value previously requested 
is ignored, the evaluation of any expression requested is abandoned and 
execution just stops when the function returns, without retrieving the 
return value.  And obviously there is no way for the user to unload a 
symbol table while the target is running in the synchronous mode.

> FYI getting with your original patch:
> 
> bfin-tdep.c: In function 'bfin_gdbarch_init':
> bfin-tdep.c:841:3: error: passing argument 2 of 'set_gdbarch_return_value' from incompatible pointer type [-Werror]
> In file included from defs.h:945:0,
>                  from bfin-tdep.c:22:
> gdbarch.h:452:13: note: expected 'enum return_value_convention (*)(struct gdbarch *, struct value *, struct type *, struct regcache *, gdb_byte *, const gdb_byte *)' but argument is of type 'enum return_value_convention (*)(struct gdbarch *, struct type *, struct type *, struct regcache *, gdb_byte *, const gdb_byte *)'
> 
> So maybe your patch I used is already obsolete or be sure you run with
> "--enable-64-bit-bfd --enable-targets=all".

 I thought I've caught all the newly-added targets, it looks like I missed 
this one.  Sorry about that, and thanks for taking time to check it.  And 
for the hint -- I'll check that all the targets at least build this way.

  Maciej