From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from brightrain.aerifal.cx (brightrain.aerifal.cx [216.12.86.13]) by sourceware.org (Postfix) with ESMTPS id 15EA23858D28 for ; Wed, 9 Mar 2022 11:52:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 15EA23858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=libc.org Authentication-Results: sourceware.org; spf=none smtp.mailfrom=libc.org Date: Wed, 9 Mar 2022 06:52:36 -0500 From: Rich Felker To: =?utf-8?B?U8O2cmVu?= Tempel Cc: Ian Lance Taylor , gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com, schwab@linux-m68k.org Subject: Re: [PATCH v3] libgo: Don't use pt_regs member in mcontext_t Message-ID: <20220309115236.GU7074@brightrain.aerifal.cx> References: <20220306212140.GI7074@brightrain.aerifal.cx> <20220307070957.27119-1-soeren@soeren-tempel.net> <34I5IKEDM1UVV.3F9GPVS0YSS1O@8pit.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <34I5IKEDM1UVV.3F9GPVS0YSS1O@8pit.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Mar 2022 11:52:40 -0000 On Wed, Mar 09, 2022 at 08:26:11AM +0100, Sören Tempel wrote: > Ian Lance Taylor wrote: > > Have you tested this in 32-bit mode? It does not look correct based > > on the glibc definitions. Looking at glibc it seems that it ought to > > be > > As stated in the commit message, I have only tested this on Alpine Linux > ppc64le (which uses musl libc). Unfortunately, I don't have access to a > 32-bit PowerPC machine and hence haven't performed any tests with it. > > > reg.sigpc = ((ucontext_t*)(context))->uc_mcontext.uc_regs->gregs[32]; > > While this should work with glibc, it doesn't work with musl. In order > to support both (musl and glibc) on 32-bit PowerPC, we would have to do > something along the lines of: > > #ifdef __PPC__ > #if defined(__PPC64__) /* ppc64 glibc & musl */ > ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.gp_regs[32] > #elif defined(__GLIBC__) /* ppc32 glibc */ > reg.sigpc = ((ucontext_t*)(context))->uc_mcontext.uc_regs->gregs[32]; > #else /* ppc32 musl */ > ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.gregs[32]; > #endif /* __PPC64__ */ > #endif /* __PPC__ */ > > In light of these observations, maybe using asm/ptrace.h and .regs (as > proposed in the v1 patch) is the "better" (i.e. more readable) solution > for now? I agree with Rich that using .regs is certainly a "code smell", > but this gigantic ifdef block also looks pretty smelly to me. That being > said, I can also send a v4 which uses this ifdef block. I still prefer the #ifdef chain here, because it's at least using the intended API. The problem is just that we can't have a matching API because the only API glibc offers on ppc32 contradicts the POSIX requirements. I'm also not understanding how the .regs approach in the v1 patch was ever supposed to work with musl anyway. The relevant line was: ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip; and musl has no uc_mcontext.regs member because uc_mcontext is mcontext_t, not the glibc ppc32 pointer union thing. The only .regs in musl's ppc32 signal.h/ucontext.h is in struct sigcontext, not anywhere in ucontext_t. Rich