Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 3 Feb 2020 10:24:27 -0500
From: Rich Felker <dalias@...c.org>
To: Mark Corbin <mark@...sco.co.uk>
Cc: musl@...ts.openwall.com
Subject: Re: REG_SP Definition for RISC-V

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

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.