From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SUBJ_OBFU_PUNCT_FEW autolearn=ham autolearn_force=no version=3.4.2 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by inbox.vuxu.org (OpenSMTPD) with SMTP id 5de07c1d for ; Wed, 19 Feb 2020 03:43:06 +0000 (UTC) Received: (qmail 17798 invoked by uid 550); 19 Feb 2020 03:43:05 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 8187 invoked from network); 19 Feb 2020 03:17:25 -0000 Date: Tue, 18 Feb 2020 22:17:12 -0500 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20200219031712.GY1663@brightrain.aerifal.cx> References: <20200204142631.GU1663@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] REG_SP Definition for RISC-V 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 . I'm not sure if there's any precedent for > >>> > > that or if glibc only has them in > >>> > > >>> > 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 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 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