move riscv64 register index constants to signal.h

Submitted by Rich Felker on Feb. 4, 2020, 2:31 p.m.

Details

Message ID 20200204143136.GV1663@brightrain.aerifal.cx
State New
Series "move riscv64 register index constants to signal.h"
Headers show

Commit Message

Rich Felker Feb. 4, 2020, 2:31 p.m.
On Tue, Feb 04, 2020 at 09:26:31AM -0500, Rich Felker wrote:
> > > > I guess that it would probably be best to change the libsigsegv code to
> > > > use a value of '2' instead of the REG_SP definition. I'll look at
> > > > submitting a patch to the project.
> > > 
> > > I think using a symbolic name is both more informative and more
> > > portable (since the layout of the saved registers is an OS choice,
> > > nothing universal to the architecture). The question is just where the
> > > macro should be obtained from. As long as glibc (and any other
> > > platforms that might be relevant?) has a sys/reg.h, it wouldn't hurt
> > > to just add the include and continue using the macro, regardless of
> > > whether musl moves it later.
> > 
> > Glibc and uClibc don't have a sys/reg.h - is there a way that it could be 
> > included conditionally for musl only?
> 
> If you want a configure test to detect it the yes; otherwise no. But
> this suggests the way we did it is wrong. We should not be making this
> kind of mess. I should probably just move the definitions...

Patch attached. Any objections?

Rich
From 329e79299daaa994b8e75941331a1093051ea5d9 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Tue, 4 Feb 2020 09:29:13 -0500
Subject: [PATCH] move riscv64 register index constants to signal.h

under _GNU_SOURCE for namespace cleanliness, analogous to other archs.
the original placement in sys/reg.h seems not to have been motivated;
such a header isn't even present on other implementations.
---
 arch/riscv64/bits/reg.h    | 6 ------
 arch/riscv64/bits/signal.h | 9 +++++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/arch/riscv64/bits/reg.h b/arch/riscv64/bits/reg.h
index c800788c..2633f39d 100644
--- a/arch/riscv64/bits/reg.h
+++ b/arch/riscv64/bits/reg.h
@@ -1,8 +1,2 @@ 
 #undef __WORDSIZE
 #define __WORDSIZE 64
-#define REG_PC 0
-#define REG_RA 1
-#define REG_SP 2
-#define REG_TP 4
-#define REG_S0 8
-#define REG_A0 10
diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
index 2ff4be30..b006334f 100644
--- a/arch/riscv64/bits/signal.h
+++ b/arch/riscv64/bits/signal.h
@@ -35,6 +35,15 @@  typedef struct mcontext_t {
 	union __riscv_mc_fp_state __fpregs;
 } mcontext_t;
 
+#if defined(_GNU_SOURCE)
+#define REG_PC 0
+#define REG_RA 1
+#define REG_SP 2
+#define REG_TP 4
+#define REG_S0 8
+#define REG_A0 10
+#endif
+
 #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 typedef unsigned long greg_t;
 typedef unsigned long gregset_t[32];

Comments

Mark Corbin Feb. 11, 2020, 2:19 p.m.
On Tuesday, 4 February 2020 14:31:36 GMT Rich Felker wrote:
> On Tue, Feb 04, 2020 at 09:26:31AM -0500, Rich Felker wrote:
> > > > > I guess that it would probably be best to change the libsigsegv code
> > > > > to
> > > > > use a value of '2' instead of the REG_SP definition. I'll look at
> > > > > submitting a patch to the project.
> > > > 
> > > > I think using a symbolic name is both more informative and more
> > > > portable (since the layout of the saved registers is an OS choice,
> > > > nothing universal to the architecture). The question is just where the
> > > > macro should be obtained from. As long as glibc (and any other
> > > > platforms that might be relevant?) has a sys/reg.h, it wouldn't hurt
> > > > to just add the include and continue using the macro, regardless of
> > > > whether musl moves it later.
> > > 
> > > Glibc and uClibc don't have a sys/reg.h - is there a way that it could
> > > be
> > > included conditionally for musl only?
> > 
> > If you want a configure test to detect it the yes; otherwise no. But
> > this suggests the way we did it is wrong. We should not be making this
> > kind of mess. I should probably just move the definitions...
> 
> Patch attached. Any objections?
> 
> Rich

Thanks Rich, that solves the problem.