From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id B72FC385843D for ; Fri, 24 Feb 2023 00:17:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B72FC385843D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id DD9F930067D6; Fri, 24 Feb 2023 01:17:40 +0100 (CET) Date: Fri, 24 Feb 2023 01:17:40 +0100 From: Mark Wielaard To: German Gomez Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH 0/4] Add AARCH64 pointer authentication support Message-ID: <20230224001740.GB9039@gnu.wildebeest.org> References: <20220425140311.95231-1-german.gomez@arm.com> <20220428195649.GF23335@gnu.wildebeest.org> <0d2e8ebc-df2a-ca51-4c82-18f0433e9f4c@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0d2e8ebc-df2a-ca51-4c82-18f0433e9f4c@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-3031.1 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi German, On Thu, May 19, 2022 at 02:30:23PM +0100, German Gomez via Elfutils-devel wrote: > On 28/04/2022 20:56, Mark Wielaard wrote: > > I haven't been able to look at the actual patches yet. And I am on > > vacation this week. But I'll review next week after I am back. > > Thanks a lot for looking. And then it took a couple of months to actually review the patches, sorry. The first two patches look OK with one small issue where/how to define DW_AARCH64_RA_SIGN_STATE (lets put it in cfi.h instead of dwarf.h). I have some concerns about the last two patches because they define new public api that is aarch64 specific. > > A quick scan shows we need a aarch64 special public function, which > > would be slightly ugly imho. I had hoped it could be a variant of the > > func_addr_mask. But maybe this is too different to make more generic. > > I did consider func_addr_mask initially, but when I wrote the patch it > wasn't exposed as a perf-thread value. Currently PAC masks are constant > but might be different from thread to thread in the future. So I placed > it in the Thread struct. Aha. I assumed it was per process, but it makes sense if it is per-thread. > I agree the arch-specific naming is not pretty. I think I can certainly > rework it into a more generic feature. But I think I would need to make > sure that the masks can be supplied to the Thread struct before the    > unwind. Are there any other architectures with a similar "mask"? Maybe we can make it a special dwfl_thread_state_registers call with e.g. firstreg -1 and nregs 1? That way we don't need a new function, just a "magic" negative register number. Also we need to extract the pauth mask somehow in libdwfl/linux-core-attach.c Cheers, Mark