[RFC PATCH] riscv: remove riscv-specific sigcontext.h

[Vincent Chen]

On Fri, Jul 10, 2020 at 6:25 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Jun 29, 2020 at 10:54 PM Vincent Chen <vincent.chen@sifive.com> wrote:
> >
> > Many RISC-V extensions are under development, such as Vector extension,
> > even though RISC-V allows vendors to customize their extension. A new
> > extension may introduce new registers to the contents of the signal
> > context. To align the contents of struct sigcontext between kernel and
> > glibc, the developers need to perform the same modifications twice. This
> > RFC patch attempts to use the Glibc generic sigcontext.h to reduce the
> > modification from two to one. Because The element names of struct
> > sigcontext in these two sigcontext.h are different, this change will cause
> > some backward-incompatible issues. To evaluate the impact, I issued a
> > discussion in the RISC-V software group
> > https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/52cbGQCyy2s.
> > I followed the suggestions to build the OpenEmbedded with this Glibc change,
> > and all utility programs were passed. Therefore, I think that the
>
> What did you build? Can you provide more details.

Sure. I followed the README in https://github.com/riscv/meta-riscv to
build the OpenEmbedded. To enable OpenEmbedded to include this change
in Glibc sigcontext.h before executing the ". ./meta-riscv/setup.sh",
I put this Glibc patch in
openembedded-core/meta/recipes-core/glibc/glibc/ folder and an
informal GCC patch for fixing the stack unwinding mechanism in
openembedded-core/meta/recipes-devtools/gcc/gcc-10.1/ folder. Then, I
modified Glibc's recipe, glibc_2.31.bb, and GCC's recipes,
gcc-10.1.inc, to include these two patches.
   Based on the above modifications, I built multiple images, such as
MACHINE=freedom-u540 core-image-base, MACHINE=freedom-u540
core-image-weston, MACHINE=qemuriscv64 core-image-full-cmdline and
MACHINE=qemuriscv32 core-image-base. These images all can pass the
compilation. Hence I think there are very few programs that uses
struct sigcontext. If you find out what evaluations or experiments I
am missing, please tell me. Thank you


> Have you done any userspace testing as well?
>
Do you mean what userspace testing I have done for this Glibc patch?
If yes, I used glibc test suite to test this Glibc patch + required
modification in GCC stack unwinding, and it passes all the tests
including unwinding tests (excluding some known failure cases).


> > backward-incompatible issue may not be serious at this moment. If everyone
> > thinks this change is reasonable and feasible, I will discuss with compiler
> > guys to come up with a solution to apply the new sigcontext.h to the GCC
> > stack unwinding.
>
> Will this affect LLVM as well?
>
As far as I knew, LLVM does not use struct sigcontext to access the
signal context. Instead, it directly uses memory offset to access the
signal context. Hence, I think this Glibc change will not affect LLVM.

> Alistair
>
> > ---
> >  sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h | 31 -------------------------
> >  1 file changed, 31 deletions(-)
> >  delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h
> >
> > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h
> > deleted file mode 100644
> > index 4b7c09e..0000000
> > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h
> > +++ /dev/null
> > @@ -1,31 +0,0 @@
> > -/* Machine-dependent signal context structure for Linux.  RISC-V version.
> > -   Copyright (C) 1996-2020 Free Software Foundation, Inc.  This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library.  If not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#ifndef _BITS_SIGCONTEXT_H
> > -#define _BITS_SIGCONTEXT_H 1
> > -
> > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H
> > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead."
> > -#endif
> > -
> > -struct sigcontext {
> > -  /* gregs[0] holds the program counter.  */
> > -  unsigned long int gregs[32];
> > -  unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16)));
> > -};
> > -
> > -#endif
> > --
> > 2.7.4
> >