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

Message ID 1593496449-24439-1-git-send-email-vincent.chen@sifive.com
State New
Headers show
Series
  • [RFC] riscv: remove riscv-specific sigcontext.h
Related show

Commit Message

Vincent Chen June 30, 2020, 5:54 a.m.
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
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.
---
 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

-- 
2.7.4

Comments

H.J. Lu via Libc-alpha July 9, 2020, 10:15 p.m. | #1
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.

Have you done any userspace testing as well?

> 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?

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

>
Vincent Chen July 13, 2020, 10:16 a.m. | #2
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

> >

Patch

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