* [musl] REG_SP Definition for RISC-V @ 2020-02-03 11:42 Mark Corbin 2020-02-03 13:32 ` Rich Felker 0 siblings, 1 reply; 10+ messages in thread From: Mark Corbin @ 2020-02-03 11:42 UTC (permalink / raw) To: musl Hello I'm trying to fix a build issue with libsigsegv [1] for RISC-V when compiling against musl 1.1.24 (under Buildroot). The build fails because the array index 'REG_SP' (for indexing into uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. This constant is defined by glibc in sysdeps/unix/sysv/linux/riscv/sys/ucontext.h I was wondering whether the appropriate fix is just to add '#define REG_SP 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a REG_SP definition in arch/riscv64/bits/reg.h which isn't being included). Alternatively I could submit a patch to libsigsegv to modify the index into the '__gregs' array to be '2' rather than 'REG_SP', however there could be other glibc compatible RISC-V packages that make use of the 'REG_SP' definition. I'm happy to generate and submit any patches as appropriate. Thanks Mark Corbin [1] http://savannah.gnu.org/projects/libsigsegv/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-03 11:42 [musl] REG_SP Definition for RISC-V Mark Corbin @ 2020-02-03 13:32 ` Rich Felker 2020-02-03 15:17 ` Mark Corbin 0 siblings, 1 reply; 10+ messages in thread From: Rich Felker @ 2020-02-03 13:32 UTC (permalink / raw) To: musl; +Cc: Mark Corbin On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > Hello > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when compiling > against musl 1.1.24 (under Buildroot). > > The build fails because the array index 'REG_SP' (for indexing into > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. This > constant is defined by glibc in sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > > I was wondering whether the appropriate fix is just to add '#define REG_SP 2' to > the top of arch/riscv64/bits/signal.h ? (Note that there is a REG_SP definition > in arch/riscv64/bits/reg.h which isn't being included). > > Alternatively I could submit a patch to libsigsegv to modify the index into > the '__gregs' array to be '2' rather than 'REG_SP', however there could be > other glibc compatible RISC-V packages that make use of the 'REG_SP' > definition. > > I'm happy to generate and submit any patches as appropriate. Generally, we like to avoid this kind of REG_* (or even bare names) register macro in signal.h since it's highly namespace-polluting (can break software using them for its own purposes that has no knowledge that some arch has a reg by that name in its signal.h bits) and only expose them under _GNU_SOURCE when we do. Right now musl has them exposed via <sys/reg.h>. I'm not sure if there's any precedent for that or if glibc only has them in <signal.h> So my leaning would be to leave it as it is and ask applications to include <sys/reg.h> if they want these macros. But if it looks like this is contrary to what maintainers of other software want to do, we could consider putting them under _GNU_SOURCE with <signal.h> like many other archs do. Rich ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-03 13:32 ` Rich Felker @ 2020-02-03 15:17 ` Mark Corbin 2020-02-03 15:24 ` Rich Felker 0 siblings, 1 reply; 10+ messages in thread From: Mark Corbin @ 2020-02-03 15:17 UTC (permalink / raw) To: musl; +Cc: Rich Felker On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > > Hello > > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when > > compiling against musl 1.1.24 (under Buildroot). > > > > The build fails because the array index 'REG_SP' (for indexing into > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. This > > constant is defined by glibc in > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > > > > I was wondering whether the appropriate fix is just to add '#define REG_SP > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being included). > > > > Alternatively I could submit a patch to libsigsegv to modify the index > > into > > the '__gregs' array to be '2' rather than 'REG_SP', however there could be > > other glibc compatible RISC-V packages that make use of the 'REG_SP' > > definition. > > > > I'm happy to generate and submit any patches as appropriate. > > Generally, we like to avoid this kind of REG_* (or even bare names) > register macro in signal.h since it's highly namespace-polluting (can > break software using them for its own purposes that has no knowledge > that some arch has a reg by that name in its signal.h bits) and only > expose them under _GNU_SOURCE when we do. Right now musl has them > exposed via <sys/reg.h>. I'm not sure if there's any precedent for > that or if glibc only has them in <signal.h> I spent some time looking for a good method of handling this, but couldn't really find any consistency between architectures. I think that most of them access the appropriate register array using a numeric value rather than a register name in this scenario. > > So my leaning would be to leave it as it is and ask applications to > include <sys/reg.h> if they want these macros. But if it looks like > this is contrary to what maintainers of other software want to do, we > could consider putting them under _GNU_SOURCE with <signal.h> like > many other archs do. 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. Thanks Mark Corbin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-03 15:17 ` Mark Corbin @ 2020-02-03 15:24 ` Rich Felker 2020-02-04 10:03 ` Mark Corbin 0 siblings, 1 reply; 10+ messages in thread From: Rich Felker @ 2020-02-03 15:24 UTC (permalink / raw) To: Mark Corbin; +Cc: musl On Mon, Feb 03, 2020 at 03:17:15PM +0000, Mark Corbin wrote: > On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: > > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > > > Hello > > > > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when > > > compiling against musl 1.1.24 (under Buildroot). > > > > > > The build fails because the array index 'REG_SP' (for indexing into > > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. This > > > constant is defined by glibc in > > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > > > > > > I was wondering whether the appropriate fix is just to add '#define REG_SP > > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a > > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being included). > > > > > > Alternatively I could submit a patch to libsigsegv to modify the index > > > into > > > the '__gregs' array to be '2' rather than 'REG_SP', however there could be > > > other glibc compatible RISC-V packages that make use of the 'REG_SP' > > > definition. > > > > > > I'm happy to generate and submit any patches as appropriate. > > > > Generally, we like to avoid this kind of REG_* (or even bare names) > > register macro in signal.h since it's highly namespace-polluting (can > > break software using them for its own purposes that has no knowledge > > that some arch has a reg by that name in its signal.h bits) and only > > expose them under _GNU_SOURCE when we do. Right now musl has them > > exposed via <sys/reg.h>. I'm not sure if there's any precedent for > > that or if glibc only has them in <signal.h> > > I spent some time looking for a good method of handling this, but couldn't > really find any consistency between architectures. I think that most of them > access the appropriate register array using a numeric value rather than a > register name in this scenario. > > > > > So my leaning would be to leave it as it is and ask applications to > > include <sys/reg.h> if they want these macros. But if it looks like > > this is contrary to what maintainers of other software want to do, we > > could consider putting them under _GNU_SOURCE with <signal.h> like > > many other archs do. > > 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. Rich ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-03 15:24 ` Rich Felker @ 2020-02-04 10:03 ` Mark Corbin 2020-02-04 14:26 ` Rich Felker 0 siblings, 1 reply; 10+ messages in thread From: Mark Corbin @ 2020-02-04 10:03 UTC (permalink / raw) To: musl; +Cc: Rich Felker On Monday, 3 February 2020 15:24:27 GMT Rich Felker wrote: > On Mon, Feb 03, 2020 at 03:17:15PM +0000, Mark Corbin wrote: > > On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: > > > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > > > > Hello > > > > > > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when > > > > compiling against musl 1.1.24 (under Buildroot). > > > > > > > > The build fails because the array index 'REG_SP' (for indexing into > > > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. > > > > This > > > > constant is defined by glibc in > > > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > > > > > > > > I was wondering whether the appropriate fix is just to add '#define > > > > REG_SP > > > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a > > > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being > > > > included). > > > > > > > > Alternatively I could submit a patch to libsigsegv to modify the index > > > > into > > > > the '__gregs' array to be '2' rather than 'REG_SP', however there > > > > could be > > > > other glibc compatible RISC-V packages that make use of the 'REG_SP' > > > > definition. > > > > > > > > I'm happy to generate and submit any patches as appropriate. > > > > > > Generally, we like to avoid this kind of REG_* (or even bare names) > > > register macro in signal.h since it's highly namespace-polluting (can > > > break software using them for its own purposes that has no knowledge > > > that some arch has a reg by that name in its signal.h bits) and only > > > expose them under _GNU_SOURCE when we do. Right now musl has them > > > exposed via <sys/reg.h>. I'm not sure if there's any precedent for > > > that or if glibc only has them in <signal.h> > > > > I spent some time looking for a good method of handling this, but couldn't > > really find any consistency between architectures. I think that most of > > them access the appropriate register array using a numeric value rather > > than a register name in this scenario. > > > > > So my leaning would be to leave it as it is and ask applications to > > > include <sys/reg.h> if they want these macros. But if it looks like > > > this is contrary to what maintainers of other software want to do, we > > > could consider putting them under _GNU_SOURCE with <signal.h> like > > > many other archs do. > > > > 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? Thanks Mark ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-04 10:03 ` Mark Corbin @ 2020-02-04 14:26 ` Rich Felker 2020-02-04 14:31 ` [musl] [PATCH] move riscv64 register index constants to signal.h Rich Felker 2020-02-18 19:17 ` [musl] REG_SP Definition for RISC-V Palmer Dabbelt 0 siblings, 2 replies; 10+ messages in thread From: Rich Felker @ 2020-02-04 14:26 UTC (permalink / raw) To: Mark Corbin; +Cc: musl On Tue, Feb 04, 2020 at 10:03:59AM +0000, Mark Corbin wrote: > On Monday, 3 February 2020 15:24:27 GMT Rich Felker wrote: > > On Mon, Feb 03, 2020 at 03:17:15PM +0000, Mark Corbin wrote: > > > On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: > > > > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > > > > > Hello > > > > > > > > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when > > > > > compiling against musl 1.1.24 (under Buildroot). > > > > > > > > > > The build fails because the array index 'REG_SP' (for indexing into > > > > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. > > > > > This > > > > > constant is defined by glibc in > > > > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > > > > > > > > > > I was wondering whether the appropriate fix is just to add '#define > > > > > REG_SP > > > > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a > > > > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being > > > > > included). > > > > > > > > > > Alternatively I could submit a patch to libsigsegv to modify the index > > > > > into > > > > > the '__gregs' array to be '2' rather than 'REG_SP', however there > > > > > could be > > > > > other glibc compatible RISC-V packages that make use of the 'REG_SP' > > > > > definition. > > > > > > > > > > I'm happy to generate and submit any patches as appropriate. > > > > > > > > Generally, we like to avoid this kind of REG_* (or even bare names) > > > > register macro in signal.h since it's highly namespace-polluting (can > > > > break software using them for its own purposes that has no knowledge > > > > that some arch has a reg by that name in its signal.h bits) and only > > > > expose them under _GNU_SOURCE when we do. Right now musl has them > > > > exposed via <sys/reg.h>. I'm not sure if there's any precedent for > > > > that or if glibc only has them in <signal.h> > > > > > > I spent some time looking for a good method of handling this, but couldn't > > > really find any consistency between architectures. I think that most of > > > them access the appropriate register array using a numeric value rather > > > than a register name in this scenario. > > > > > > > So my leaning would be to leave it as it is and ask applications to > > > > include <sys/reg.h> if they want these macros. But if it looks like > > > > this is contrary to what maintainers of other software want to do, we > > > > could consider putting them under _GNU_SOURCE with <signal.h> like > > > > many other archs do. > > > > > > 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... Rich ^ permalink raw reply [flat|nested] 10+ messages in thread
* [musl] [PATCH] move riscv64 register index constants to signal.h 2020-02-04 14:26 ` Rich Felker @ 2020-02-04 14:31 ` Rich Felker 2020-02-11 14:19 ` Mark Corbin 2020-02-18 19:17 ` [musl] REG_SP Definition for RISC-V Palmer Dabbelt 1 sibling, 1 reply; 10+ messages in thread From: Rich Felker @ 2020-02-04 14:31 UTC (permalink / raw) To: Mark Corbin; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1113 bytes --] 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 [-- Attachment #2: 0001-move-riscv64-register-index-constants-to-signal.h.patch --] [-- Type: text/plain, Size: 1417 bytes --] 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(-) 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]; -- 2.21.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] [PATCH] move riscv64 register index constants to signal.h 2020-02-04 14:31 ` [musl] [PATCH] move riscv64 register index constants to signal.h Rich Felker @ 2020-02-11 14:19 ` Mark Corbin 0 siblings, 0 replies; 10+ messages in thread From: Mark Corbin @ 2020-02-11 14:19 UTC (permalink / raw) To: musl; +Cc: Rich Felker 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-04 14:26 ` Rich Felker 2020-02-04 14:31 ` [musl] [PATCH] move riscv64 register index constants to signal.h Rich Felker @ 2020-02-18 19:17 ` Palmer Dabbelt 2020-02-19 3:17 ` Rich Felker 1 sibling, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2020-02-18 19:17 UTC (permalink / raw) To: dalias; +Cc: mark, musl On Tue, 04 Feb 2020 06:26:31 PST (-0800), dalias@libc.org wrote: > On Tue, Feb 04, 2020 at 10:03:59AM +0000, Mark Corbin wrote: >> On Monday, 3 February 2020 15:24:27 GMT Rich Felker wrote: >> > On Mon, Feb 03, 2020 at 03:17:15PM +0000, Mark Corbin wrote: >> > > On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: >> > > > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: >> > > > > Hello >> > > > > >> > > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when >> > > > > compiling against musl 1.1.24 (under Buildroot). >> > > > > >> > > > > The build fails because the array index 'REG_SP' (for indexing into >> > > > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. >> > > > > This >> > > > > constant is defined by glibc in >> > > > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h >> > > > > >> > > > > I was wondering whether the appropriate fix is just to add '#define >> > > > > REG_SP >> > > > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a >> > > > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being >> > > > > included). >> > > > > >> > > > > Alternatively I could submit a patch to libsigsegv to modify the index >> > > > > into >> > > > > the '__gregs' array to be '2' rather than 'REG_SP', however there >> > > > > could be >> > > > > other glibc compatible RISC-V packages that make use of the 'REG_SP' >> > > > > definition. >> > > > > >> > > > > I'm happy to generate and submit any patches as appropriate. >> > > > >> > > > Generally, we like to avoid this kind of REG_* (or even bare names) >> > > > register macro in signal.h since it's highly namespace-polluting (can >> > > > break software using them for its own purposes that has no knowledge >> > > > that some arch has a reg by that name in its signal.h bits) and only >> > > > expose them under _GNU_SOURCE when we do. Right now musl has them >> > > > exposed via <sys/reg.h>. I'm not sure if there's any precedent for >> > > > that or if glibc only has them in <signal.h> >> > > >> > > I spent some time looking for a good method of handling this, but couldn't >> > > really find any consistency between architectures. I think that most of >> > > them access the appropriate register array using a numeric value rather >> > > than a register name in this scenario. >> > > >> > > > So my leaning would be to leave it as it is and ask applications to >> > > > include <sys/reg.h> if they want these macros. But if it looks like >> > > > this is contrary to what maintainers of other software want to do, we >> > > > could consider putting them under _GNU_SOURCE with <signal.h> like >> > > > many other archs do. >> > > >> > > 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... The glibc definitions are in sys/ucontext.h as that's also where the relevant structures are defined. They're all within a _GNU_SOURCE to avoid polluting the namespace too much. Maybe the best bet is to have a riscv64-specific sys/ucontext.h? I don't see any other ports with their own sys/ucontext.h, though. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] REG_SP Definition for RISC-V 2020-02-18 19:17 ` [musl] REG_SP Definition for RISC-V Palmer Dabbelt @ 2020-02-19 3:17 ` Rich Felker 0 siblings, 0 replies; 10+ messages in thread From: Rich Felker @ 2020-02-19 3:17 UTC (permalink / raw) To: musl On Tue, Feb 18, 2020 at 11:17:30AM -0800, Palmer Dabbelt wrote: > On Tue, 04 Feb 2020 06:26:31 PST (-0800), dalias@libc.org wrote: > >On Tue, Feb 04, 2020 at 10:03:59AM +0000, Mark Corbin wrote: > >>On Monday, 3 February 2020 15:24:27 GMT Rich Felker wrote: > >>> On Mon, Feb 03, 2020 at 03:17:15PM +0000, Mark Corbin wrote: > >>> > On Monday, 3 February 2020 13:32:25 GMT Rich Felker wrote: > >>> > > On Mon, Feb 03, 2020 at 11:42:30AM +0000, Mark Corbin wrote: > >>> > > > Hello > >>> > > > > >>> > > > I'm trying to fix a build issue with libsigsegv [1] for RISC-V when > >>> > > > compiling against musl 1.1.24 (under Buildroot). > >>> > > > > >>> > > > The build fails because the array index 'REG_SP' (for indexing into > >>> > > > uc_mcontext.__gregs[]) is not defined in arch/riscv64/bits/signal.h. > >>> > > > This > >>> > > > constant is defined by glibc in > >>> > > > sysdeps/unix/sysv/linux/riscv/sys/ucontext.h > >>> > > > > >>> > > > I was wondering whether the appropriate fix is just to add '#define > >>> > > > REG_SP > >>> > > > 2' to the top of arch/riscv64/bits/signal.h ? (Note that there is a > >>> > > > REG_SP definition in arch/riscv64/bits/reg.h which isn't being > >>> > > > included). > >>> > > > > >>> > > > Alternatively I could submit a patch to libsigsegv to modify the index > >>> > > > into > >>> > > > the '__gregs' array to be '2' rather than 'REG_SP', however there > >>> > > > could be > >>> > > > other glibc compatible RISC-V packages that make use of the 'REG_SP' > >>> > > > definition. > >>> > > > > >>> > > > I'm happy to generate and submit any patches as appropriate. > >>> > > > >>> > > Generally, we like to avoid this kind of REG_* (or even bare names) > >>> > > register macro in signal.h since it's highly namespace-polluting (can > >>> > > break software using them for its own purposes that has no knowledge > >>> > > that some arch has a reg by that name in its signal.h bits) and only > >>> > > expose them under _GNU_SOURCE when we do. Right now musl has them > >>> > > exposed via <sys/reg.h>. I'm not sure if there's any precedent for > >>> > > that or if glibc only has them in <signal.h> > >>> > > >>> > I spent some time looking for a good method of handling this, but couldn't > >>> > really find any consistency between architectures. I think that most of > >>> > them access the appropriate register array using a numeric value rather > >>> > than a register name in this scenario. > >>> > > >>> > > So my leaning would be to leave it as it is and ask applications to > >>> > > include <sys/reg.h> if they want these macros. But if it looks like > >>> > > this is contrary to what maintainers of other software want to do, we > >>> > > could consider putting them under _GNU_SOURCE with <signal.h> like > >>> > > many other archs do. > >>> > > >>> > 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... > > The glibc definitions are in sys/ucontext.h as that's also where the relevant > structures are defined. They're all within a _GNU_SOURCE to avoid polluting > the namespace too much. Maybe the best bet is to have a riscv64-specific > sys/ucontext.h? I don't see any other ports with their own sys/ucontext.h, > though. The structures have to be defined in signal.h, which they are. glibc handles this by having signal.h include ucontext.h, which is somewhat wrong-ish but probably ok by the way the feature test macros are handled. The reason is historical; long ago when ucontext api was standard, POSIX specified ucontext.h, and since the only remaining standard use of ucontext_t/mcontext_t is for SA_SIGINFO signal handlers, they moved it to signal.h when removing ucontext.h. Rich ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-19 3:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-03 11:42 [musl] REG_SP Definition for RISC-V Mark Corbin 2020-02-03 13:32 ` Rich Felker 2020-02-03 15:17 ` Mark Corbin 2020-02-03 15:24 ` Rich Felker 2020-02-04 10:03 ` Mark Corbin 2020-02-04 14:26 ` Rich Felker 2020-02-04 14:31 ` [musl] [PATCH] move riscv64 register index constants to signal.h Rich Felker 2020-02-11 14:19 ` Mark Corbin 2020-02-18 19:17 ` [musl] REG_SP Definition for RISC-V Palmer Dabbelt 2020-02-19 3:17 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).