On Fri, Oct 27, 2023 at 08:47:53PM +0200, Sam Ravnborg wrote:
> Hi Al,
>
> On Thu, Oct 26, 2023 at 03:16:13AM +0100, Al Viro wrote:
> > Fault handler used to make non-trivial calls, so it needed
> > to set a stack frame up. Used to be
> > save ... - grab a stack frame, old %o... become %i...
> > ....
> > ret - go back to address originally in %o7, currently %i7
> > restore - switch to previous stack frame, in delay slot
> > Non-trivial calls had been gone since ab5e8b331244 and that code should
> > have become
> > retl - go back to address in %o7
> > clr %o0 - have return value set to 0
> > What it had become instead was
> > ret - go back to address in %i7 - return address of *caller*
> > clr %o0 - have return value set to 0
> > which is not good, to put it mildly - we forcibly return 0 from
> > csum_and_copy_{from,to}_iter() (which is what the call of that
> > thing had been inlined into) and do that without dropping the
> > stack frame of said csum_and_copy_..._iter(). Confuses the
> > hell out of the caller of csum_and_copy_..._iter(), obviously...
>
> I wonder how you managed to find this?
Looking at the csum_and_copy_... instances on various architectures,
noticing that and going "how the fuck could it possibly work and
what moron had broken it? Oh, lovely - it couldn't, it doesn't
and that moron had been myself ;-/"
> Do you actually use sparc32 these
> days?
qemu image only, TBH - I have an SS20 box, but it hadn't booted for
about 10 years...
> You could also kill the EX2 define while touchign the file,
> it is no longer used after ab5e8b331244.
Er? No EX2 in checksum_32.S... There is one in copy_user.S,
but that one _is_ used -
copy_user_table_end:
be copy_user_last7
andcc %g1, 4, %g0
EX(ldd [%o1], %g2, and %g1, 0xf)
add %o0, 8, %o0
add %o1, 8, %o1
EX(st %g2, [%o0 - 0x08], and %g1, 0xf)
EX2(st %g3, [%o0 - 0x04], and %g1, 0xf, %g1, sub %g1, 4)
> > Fixes: ab5e8b331244 "sparc32: propagate the calling conventions change down to __csum_partial_copy_sparc_generic()"
> > Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
