From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id XKO0DswRqGr2TgwAWB0awg (envelope-from ) for ; Mon, 14 Sep 2026 11:25:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1789399500; bh=jPMOG4XwrZ7ms4raD8wicTl+W8Pm517eX28ypYnCJ7c=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=A7TvHomqlp5gY6yPVSJrI2TghDQqddUacRzTEY9nOtFu6EYNe1EZUYVzfq1Xy8XO3 E9De2ijjkG+tRZi1B/T4m6RbmoLNH0OZOzuqc6JVz2dchM10zJs6t/PXF79aR2FJla uDg6lfzep0IsyneIV/WrqoSei6nrbaRcqE0rLq2g= Received: by simark.ca (Postfix, from userid 112) id 24A781E066; Mon, 14 Sep 2026 11:25:00 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=XNsrY8Wv; dkim-atps=neutral Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id E78351E01F for ; Mon, 14 Sep 2026 11:24:58 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F069E4BA2E1D for ; Mon, 14 Sep 2026 15:24:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F069E4BA2E1D Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=XNsrY8Wv Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B95934B9DB48 for ; Mon, 14 Sep 2026 15:24:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B95934B9DB48 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B95934B9DB48 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789399460; cv=none; b=XqnpUFoSZRrge9RQmrlEFs7l9j3MsVqwE4OLL4RpVik+q89Hm9p237QRjdLq8Nv/G2schR1t3J0K6ihw/R0h97vDDreVizmmGZ/7QPr2HSIZ9ZgJXrXDsPvuMolIN0lGITaK2Qc5E/Ejn2qaCjBLhqf0VC782GLt/vyjpCVcBgo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789399460; c=relaxed/simple; bh=jPMOG4XwrZ7ms4raD8wicTl+W8Pm517eX28ypYnCJ7c=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=Iv486+agU4pWVCdBHFQ8PAdAuJ5p87XWW/NaoyiScn5To252nNwViBxOWemiHw0CAwya7INkzeR4HIkwdLSjR0AsJ7vldabibYKQe42Ql9iiQZ+CCWA6MNRocUswGMVNF0niAOQnOHZxwCMvM8tm0ho8aols53fZ56fRiQxsvB0= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=XNsrY8Wv DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B95934B9DB48 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1789399460; bh=jPMOG4XwrZ7ms4raD8wicTl+W8Pm517eX28ypYnCJ7c=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=XNsrY8WvnP33kurLDwnfLzqiY2gmiJovvS4OyNFyPtFKgsvlTXvHgLQ9mro4Yf9EZ XLlBcLnJbYgSO3HIAcXhFHpB2aZV3xS3WDYLwgHhrX4M1HyPm0S58K1dSUatfTMaQY RjI7ci7LEF+8DotCqXeITGihypTJDUNBL6i8irKk= Received: by simark.ca (Postfix) id D4D5D1E01F; Mon, 14 Sep 2026 11:24:19 -0400 (EDT) Message-ID: <1f40fceb-bcd3-450e-9454-f92054cebcce@simark.ca> Date: Mon, 14 Sep 2026 11:24:18 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] gdb, aarch64: cache pointer authentication masks per inferior To: Luis Machado , gdb-patches@sourceware.org Cc: thiago.bauermann@linaro.org References: <20260912222011.2395686-1-luis.machado.foss@gmail.com> <20260912222011.2395686-2-luis.machado.foss@gmail.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20260912222011.2395686-2-luis.machado.foss@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 9/12/26 6:20 PM, Luis Machado wrote: > aarch64_remove_non_address_bits recomputed the pointer authentication > masks on every call by walking the current inferior's thread list, > looking up its regcache, and reading the dmask/cmask registers from > the target. This function is called from memory_xfer_partial for > every memory transfer GDB performs, as well as from the watchpoint > and breakpoint address-masking hooks, so the lookup happens far more > often than the masks can possibly change. Not against this patch, but just a precision: after the first read (and until the thread resumes), the registers are cached in the regcache, so a read wouldn't have to read the registers from the target again (which is probably the most expensive part). And just wondering, do you see a real world impact with this patch, is there some measurable improvement? > The masks are fixed for the life of a process, since they reflect the > kernel's VA-size configuration at exec time, and are shared by all of > a process' threads. Cache the computed mask per inferior (one slot > each for the low and high VA ranges) instead of recomputing it on > every call, only populating the cache when a thread is actually > stopped so a transient "thread running" state never gets cached as a > permanent answer. The cache is invalidated on inferior exit, > inferior appeared, and exec, since those are the only points where a > process' mask configuration could legitimately change. > --- > gdb/aarch64-tdep.c | 154 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 112 insertions(+), 42 deletions(-) > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 950ad4f6aae..c7003687eab 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -57,6 +57,9 @@ > > /* For inferior_ptid and current_inferior (). */ > #include "inferior.h" > +/* For gdb::observers::inferior_exit et al, used to invalidate the pauth > + mask cache. */ > +#include "observable.h" IMO these comments are useless. > /* For std::sqrt and std::pow. */ > #include > > @@ -4333,6 +4336,47 @@ aarch64_memtag_to_string (struct gdbarch *gdbarch, struct value *tag_value) > return string_printf ("0x%s", phex_nz (tag)); > } > > +/* Cached pointer authentication masks for an inferior. The masks are > + fixed for the life of a process (they reflect the kernel's VA-size > + configuration at exec time) and are shared by all its threads. We only > + need to compute them once per inferior instead of on every call to > + aarch64_remove_non_address_bits. LOW is used for user-space (low VA > + range) pointers, HIGH is used for kernel-space (high VA range) > + pointers. HIGH is only ever populated on targets that provide the > + high-range mask registers. */ > + > +struct aarch64_pauth_mask_cache > +{ > + std::optional low; > + std::optional high; > +}; > + > +/* Per-inferior pauth mask cache. */ > + > +static const registry::key > + aarch64_pauth_mask_cache_data; To follow the conventions used elsewhere, I would suggest make a per-inferior object, with a pauth mask field in it: struct aarch64_per_inferior { struct pauth_mask { std::optional low; std::optional high; }; }; The registry key variable would be called "aarch64_per_inferior_data", "aarch64_per_inferior_key", or something like that. Then you'd have a function "get_aarch64_per_inferior" that uses try_emplace to abstract the "create if needed" operation. > + > +/* Drop INF's cached pauth masks. */ > + > +static void > +aarch64_invalidate_pauth_mask_cache (inferior *inf) > +{ > + aarch64_pauth_mask_cache_data.clear (inf); > +} > + > +/* Drop the pauth mask cache of every inferior sharing PSPACE. This is > + attached to the all_objfiles_removed observer, which fires on exec. > + Exec is the only point after startup where a process' VA-size > + configuration, and hence its masks, could legitimately change. */ > + > +static void > +aarch64_pauth_mask_cache_objfiles_removed (program_space *pspace) > +{ > + for (inferior *inf : all_inferiors ()) > + if (inf->pspace == pspace) > + aarch64_invalidate_pauth_mask_cache (inf); > +} We have an observable inferior_execd, can't you use that? When we need to reset some cached inferior state, we often use the trio of observables: - inferior created/appeared - inferior execd - inferior exited I never remember the different between created and appeared, it might be related to run vs attach, not sure. There is probably one more correct that the other. > + > /* See aarch64-tdep.h. */ > > CORE_ADDR > @@ -4353,54 +4397,72 @@ aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer) > momentarily), we use the inferior ptid. */ > if (inferior_ptid != null_ptid) > { > - /* If we do have an inferior, attempt to fetch its thread's thread_info > - struct. */ > - thread_info *thread = current_inferior ()->find_thread (inferior_ptid); > + inferior *inf = current_inferior (); > + bool kernel_address = (pointer & VA_RANGE_SELECT_BIT_MASK) != 0; > > - /* If the thread is running, we will not be able to fetch the mask > - registers. */ > - if (thread != nullptr && thread->state () != THREAD_RUNNING) > - { > - /* Otherwise, fetch the register cache and the masks. */ > - struct regcache *regs > - = get_thread_regcache (current_inferior ()->process_target (), > - inferior_ptid); > - > - /* Use the gdbarch from the register cache to check for pointer > - authentication support, as it matches the features found in > - that particular thread. */ > - aarch64_gdbarch_tdep *tdep > - = gdbarch_tdep (regs->arch ()); > + aarch64_pauth_mask_cache *cache > + = aarch64_pauth_mask_cache_data.get (inf); > + if (cache == nullptr) > + cache = &aarch64_pauth_mask_cache_data.emplace (inf); > > - /* Is there pointer authentication support? */ > - if (tdep->has_pauth ()) > + std::optional &cached_mask > + = kernel_address ? cache->high : cache->low; > + > + if (cached_mask.has_value ()) > + mask = *cached_mask; > + else > + { > + /* If we do have an inferior, attempt to fetch its thread's > + thread_info struct. */ > + thread_info *thread = inf->find_thread (inferior_ptid); > + > + /* If the thread is running, we will not be able to fetch the mask > + registers. Leave the cache empty for this slot, we'll get > + another chance to compute and cache it once some thread of > + this inferior is next stopped. */ > + if (thread != nullptr && thread->state () != THREAD_RUNNING) I know it's pre-existing, but checking for THREAD_RUNNING seems wrong to me. The thread can be running from the point of view of the user by really stopped at the ptrace level. It happens for instance when evaluating a breakpoint condition, since we haven't decided yet if the breakpoint should cause a user visible stop or not. I > { > - CORE_ADDR cmask, dmask; > - int dmask_regnum > - = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base); > - int cmask_regnum > - = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base); > - > - /* If we have a kernel address and we have kernel-mode address > - mask registers, use those instead. */ > - if (tdep->pauth_reg_count > 2 > - && pointer & VA_RANGE_SELECT_BIT_MASK) > + /* Otherwise, fetch the register cache and the masks. */ > + struct regcache *regs > + = get_thread_regcache (inf->process_target (), inferior_ptid); > + > + /* Use the gdbarch from the register cache to check for pointer > + authentication support, as it matches the features found in > + that particular thread. */ > + aarch64_gdbarch_tdep *tdep > + = gdbarch_tdep (regs->arch ()); > + > + /* Is there pointer authentication support? */ > + if (tdep->has_pauth ()) > { > - dmask_regnum > - = AARCH64_PAUTH_DMASK_HIGH_REGNUM (tdep->pauth_reg_base); > - cmask_regnum > - = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base); > + CORE_ADDR cmask, dmask; > + int dmask_regnum > + = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base); > + int cmask_regnum > + = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base); > + > + /* If we have a kernel address and we have kernel-mode > + address mask registers, use those instead. */ > + if (tdep->pauth_reg_count > 2 && kernel_address) > + { > + dmask_regnum > + = AARCH64_PAUTH_DMASK_HIGH_REGNUM (tdep->pauth_reg_base); > + cmask_regnum > + = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base); > + } > + > + /* We have both a code mask and a data mask. For now they > + are the same, but this may change in the future. */ > + if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID) > + dmask = mask; > + > + if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID) > + cmask = mask; > + > + mask |= aarch64_mask_from_pac_registers (cmask, dmask); > } > > - /* We have both a code mask and a data mask. For now they are > - the same, but this may change in the future. */ > - if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID) > - dmask = mask; > - > - if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID) > - cmask = mask; > - > - mask |= aarch64_mask_from_pac_registers (cmask, dmask); > + cached_mask = mask; This function becomes a bit complicated, I wouldn't find if you wanted to move this scope (fetch the masks from the inferior for real) to a helper function. Simon