|
Message-ID: <mhng-2ace9726-b623-47a8-aed5-d44aab30358d@palmerdabbelt-glaptop1> Date: Tue, 18 Feb 2020 11:17:30 -0800 (PST) From: Palmer Dabbelt <palmer@...belt.com> To: dalias@...c.org CC: mark@...sco.co.uk, musl@...ts.openwall.com Subject: Re: REG_SP Definition for RISC-V On Tue, 04 Feb 2020 06:26:31 PST (-0800), dalias@...c.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.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.