From: Rich Felker <dalias@libc.org>
To: Mark Corbin <mark@dibsco.co.uk>
Cc: musl@lists.openwall.com
Subject: Re: [musl] REG_SP Definition for RISC-V
Date: Tue, 4 Feb 2020 09:26:31 -0500 [thread overview]
Message-ID: <20200204142631.GU1663@brightrain.aerifal.cx> (raw)
In-Reply-To: <2463923.aTGIt9PGv2@laptop>
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
next prev parent reply other threads:[~2020-02-04 14:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-03 11:42 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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200204142631.GU1663@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=mark@dibsco.co.uk \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).