From: Luis <luis.machado.foss@gmail.com>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Cc: thiago.bauermann@linaro.org
Subject: Re: [PATCH 1/2] gdb, aarch64: cache pointer authentication masks per inferior
Date: Mon, 21 Sep 2026 00:47:14 +0100 [thread overview]
Message-ID: <29b277e4-1a9c-4636-aa05-f67a0d900bd9@gmail.com> (raw)
In-Reply-To: <1f40fceb-bcd3-450e-9454-f92054cebcce@simark.ca>
Hi,
On 14/09/2026 16:24, Simon Marchi wrote:
> 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).
True. I had forgotten about that cache for a bit. I'll reword the commit
message.
>
> And just wondering, do you see a real world impact with this patch, is
> there some measurable improvement?
On impact, not a very significant one. Mostly a small local speedup due
to skipping some function calls when we have a mask cache. Testing on Pi
4 and 5 I don´t see a significant difference on stepping, memory reads,
backtraces, software watchpoints etc (native or gdbserver).
A microbenchmark of function calls goes from about 79 ns to 9 ns with a
warm regcache. So very minor.
There is one other improvement though, see below.
>
>> 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.
>
Dropped now.
>> /* For std::sqrt and std::pow. */
>> #include <cmath>
>>
>> @@ -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<CORE_ADDR> low;
>> + std::optional<CORE_ADDR> high;
>> +};
>> +
>> +/* Per-inferior pauth mask cache. */
>> +
>> +static const registry<inferior>::key<aarch64_pauth_mask_cache>
>> + 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<CORE_ADDR> low;
> std::optional<CORE_ADDR> 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.
>
Done. Thanks.
>> +
>> +/* 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.
>
Switched to using inferior_execd now, alongside the other two
(inferior_exit and inferior_appeared) Thanks for the heads up.
>> +
>> /* 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<aarch64_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<CORE_ADDR> &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
>
Yeah. So this is another improvement area. I'm adding another fixup
patch to this series that addresses this.
With the above check most of the calls in a stepi loop, and nearly all
of them in a software watchpoint or conditional breakpoint loop, found
the thread marked running and silently used the default mask.
Using can_access_registers_thread instead makes this work more reliably
and actually reads the mask value whenever available, only using the
default masks when we can´t read the registers of the thread.
>> {
>> - 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<aarch64_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.
True. I've split some code out into a helper now.
I'll send a v2 for this.
next prev parent reply other threads:[~2026-09-20 23:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-12 22:20 [PATCH 0/2] " Luis Machado
2026-09-12 22:20 ` [PATCH 1/2] " Luis Machado
2026-09-14 15:24 ` Simon Marchi
2026-09-20 23:47 ` Luis [this message]
2026-09-12 22:20 ` [PATCH 2/2] gdb, aarch64: add selftest for the pauth mask cache Luis Machado
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=29b277e4-1a9c-4636-aa05-f67a0d900bd9@gmail.com \
--to=luis.machado.foss@gmail.com \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
--cc=thiago.bauermann@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox