This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 3/4] Refactor sigcontextinfo.h
On 20/08/2019 09:11, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> diff --git a/sysdeps/unix/sysv/linux/m68k/register-dump.h b/sysdeps/unix/sysv/linux/m68k/register-dump.h
>> index b9941e813e..69f8e4c3f0 100644
>> --- a/sysdeps/unix/sysv/linux/m68k/register-dump.h
>> +++ b/sysdeps/unix/sysv/linux/m68k/register-dump.h
>> static void
>> -register_dump (int fd, struct sigcontext *ctx)
>> +register_dump (int fd, struct ucontext_t *ctx)
>> {
>> + /* Linux does not save the FP state in case of emulated floating point. */
>> +#ifndef __mcoldfire__
>> + __asm__ volatile (".chip 68k/68881\n\t"
>> + "fmovem.x %%fp0-%%fp7, %0\n\t"
>> + "fmovem.l %%fpcr, %1\n\t"
>> + "fmovem.l %%fpsr, %2\n\t"
>> + "fmovem.l %%fpiar, %3\n\t"
>> + ".chip 68k"
>> + : "=m" (*ctx->uc_mcontext.fpregs.f_fpregs),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr)
>> + : /* no inputs. */
>> + : "memory");
>> +#elif defined __mcffpu__
>> + __asm__ volatile ("fmovemd %%fp0-%%fp7, %0\n\t"
>> + "fmovel %%fpcr, %1\n\t"
>> + "fmovel %%fpsr, %2\n\t"
>> + "fmovel %%fpiar, %3\n\t"
>> + : "=m" (*ctx->uc_mcontext.fpregs.f_fpregs),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr),
>> + "=m" (ctx->uc_mcontext.fpregs.f_pcr)
>> + : /* no inputs. */
>> + : "memory");
>> +#endif
>
> f_pcr used three times in both cases looks like a cut-and-paste error.
> If this is correct, it needs a comment.
Good catch, in fact the second field should the f_psr and third f_fpiaddr
from m68k struct fpregset_t.
>
> I'm surprised that this does not need a softfloat conditional. Is the
> sense of the comment really correct? Based on the code, the registers
> are saved by the kernel in the softfloat case (the opposite sense)
> because there is no code to overwrite it.
In fact I think I got this wrong checking on kernel code. The
rt_save_fpu_state function on arch/m68k/kernel/signal.c already handles
FPU_IS_EMU, so this snippet is really unnecessary. I will remove it.