Re: [PATCH] MIPS: Fix unwinding through signal frames on -EB o32.

[David Daney <ddaney at avtrex dot com>]

Richard Sandiford wrote:
> David Daney <ddaney@avtrex.com> writes:
> > Richard Sandiford wrote:
> > > David Daney <ddaney@avtrex.com> writes:
> > > > When I previously changed the mips_fallback_frame_state to return an 
> > > > adjusted $pc value, I overlooked the big-endian o32 case.  For this case 
> > > > (and only this case), we were adding 4 to $pc and also adding 4 to the 
> > > > cfa resulting in a total adjustment of 8.  This was causing the libjava 
> > > > Array_3 test to fail as the reported $pc had been moved outside of the 
> > > > catch block resulting in the exception being caught at the wrong place.
> > > Ah, thanks, is that why that test was failing?  I hadn't got around
> > > to it yet...
> > >
> > > > The fix is to base the $pc calculation on the unadjusted cfa value and 
> > > > use a new (possibly adjusted) variable 'reg_base' as the base for the 
> > > > rest of the registers.
> > > Well, going back to the review of the original endian-correction patch:
> > >
> > > http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00230.html
> > >
> > > your experience seems to suggest even more strongly that it would be
> > > best not to adjust "new_cfa" at all, and simply add a separate offset
> > > to the register accesses.  I.e.: set reg_offset to 0 or 4, and add
> > > "reg_offset" to:
> > >
> > >    (_Unwind_Ptr)&(sc->sc_regs[i]) - new_cfa
> > >
> > > I think that's equivalent to what you're doing, so the patch is
> > > preapproved with that change, if it works.
> > It is equivalent, and should work.  Unfortunately the O2 is quite slow, 
> > so testing will take some time...
> >
> > Perhaps I will only rebuild libgcc, that will at least save the 3 days 
> > needed to bootstrap.
>
> Sorry ;/  FWIW, I tested the revised patch below with qemu on
> mips-linux-gnu, and like you say, it fixed the 4 Array_3 failures
> without introducing any others.  I went ahead and installed it;
> hope that's OK.

Thanks.


> +#if _MIPS_SIM == _ABIO32 && defined __MIPSEB__
> +  reg_offset = 4;
> +#else
> +  reg_offset = 0;
>  #endif
>  

In my patch the 'reg_offset' is a preprocessor macro, but the optimizers 
will undoubtedly generate equivalent code if we use a local variable.


>    for (i = 0; i < 32; i++) {
>      fs->regs.reg[i].how = REG_SAVED_OFFSET;
>      fs->regs.reg[i].loc.offset
> -      = (_Unwind_Ptr)&(sc->sc_regs[i]) - new_cfa;
> +      = (_Unwind_Ptr)&(sc->sc_regs[i]) + reg_offset - new_cfa;
>    }
>    /* The PC points to the faulting instruction, but the unwind tables
>       expect it point to the following instruction.  We compensate by