From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id cYQTIKFwsGpOLCgAWB0awg (envelope-from ) for ; Sun, 20 Sep 2026 19:47:45 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20251104 header.b=CT8Wa2Hi; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 7EC631E051; Sun, 20 Sep 2026 19:47:45 -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,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.1 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 A477A1E033 for ; Sun, 20 Sep 2026 19:47:44 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 345D34BA9012 for ; Sun, 20 Sep 2026 23:47:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 345D34BA9012 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20251104 header.b=CT8Wa2Hi Received: from mail-wr2-x10.google.com (mail-wr2-x10.google.com [IPv6:2a00:1450:4864:30::10]) by sourceware.org (Postfix) with ESMTPS id DB1A14BA23C0 for ; Sun, 20 Sep 2026 23:47:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DB1A14BA23C0 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DB1A14BA23C0 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:30::10 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789948037; cv=none; b=HSu6k4v+M7t0t4P+eFhErIyR/V2VaT4buyVy62BJBdMmAaHrOIylZSfODbTmuzWmry8ljB0811x//WOPXzfnEb3Xfc3Ne9d+L+Snd0yDnY/pZvdjPdwd0gNvKdXBlWo7NBygwz0xdFKoIqhkNXDKxrjcRqv7Owr741gWb9EjSho= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789948037; c=relaxed/simple; bh=r2IuowlM4jBVCA7Izpr7+IyxHQq/fLYpbBMyQIp0tEs=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=roWlBKQ/R/7pdIb4AEoH1vy2gzJTOKku9SdohQ4s4c8ITcoTHpcouOwpbvTogK+2JCA5I9GU/pd6UuWwld9KMMnD9cxQDvZB+BunZgKldka0od0f7ww2IPW7cmMcxT4Fp9qsFDvwWW39J8x+Cy2BQBId44oImu/doK3hoJwANN8= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20251104 header.b=CT8Wa2Hi DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DB1A14BA23C0 Received: by mail-wr2-x10.google.com with SMTP id ffacd0b85a97d-482f6350faaso1203013f8f.0 for ; Sun, 20 Sep 2026 16:47:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1789948036; x=1790552836; darn=sourceware.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=U6vq0FCPSTQBNzTGISVdgWkcVTZjZhYKzShgb5J1Kqw=; b=CT8Wa2Hix3WmiIYHaY+2NjisASu9BdYcRlK+4ZfryfCuCGYwghSwgC2jjkay921q/L O9IxphOlDjl9IAaBZA5B5d6xU2DGEv7Ru905esDLumGjGa1OksyYIGCkL/jo50xJroc+ fxIln5y7dvvLHWdeyfmQMGLe8tztFhbDrEJ8jOAU/2Joa/EpSNTxECmS089/aHL8ssww 9W1Dy78D4jFVDrkZTkK9qzv6Jx2DFCQDSkUMVeRzEmMG1Y86rMf4GkGlrI50k4PjBa/6 DHd+Stp1G9/8Izve+GrWjHzambVhU7BjmC+4DNz5dolmQaoGpAEAow/QFIZ/l2VoXjWi +ayg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1789948036; x=1790552836; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=U6vq0FCPSTQBNzTGISVdgWkcVTZjZhYKzShgb5J1Kqw=; b=A++K9/aMZ6gYL5RoJFTO6oBm/TGNO3M8J5Z3X89WztCE0pwMMjpFBkAIEnyVqBCBzh 0sk7eR5i+xXJw0jO0jTmuUE+B4gGeme8EAtyBnnDIYwc5U4laZURKz4YdOvJ2VCfrK1h AqZ+7P4A3Ynx14ISmCvaRqgo8PRyPrYrowyhNH2Fipdoqql1gl/of+0xuCZZ3VnfGyqs sIL3FjWTQ1D0Xj3GBHTCWFRy21SLGZPXaQWlhS6t0xLvDC8HAuqakx1rVx0cH78mpLqy ax+Tu2Ir0YcNoH6JfrglQb4gzsE+m2jSnzks2zigZxCrDM4aHYmYMuqrUqcprd8n3wzc BePg== X-Forwarded-Encrypted: i=1; AKwUvBzR8wFCEIRIIetieRUSJbBg4p7Vq0Ieg3wB5pfWzIw+Nq4cGCTJqCAR663bhvdPMAMXZxl9dcAMc6I0XQ==@sourceware.org X-Gm-Message-State: AFuF++l/g39TBM2cjm3JuWap8/hT+7+OxWC8wEH3ItYJ5PUSld2KiRRO vcJDfxaeZpDMpjELxmiJunKatqa2ykvPRKkJp4j0wfC9QmHi9pUyhUgOmpd/nQ== X-Gm-Gg: AYBFou3s8TdaR9w5Fm+FA0s4TRJs0aMkSw5yS5bkZYt9SiMwMkCV0KQ2JKqD29IgkQx lmq/hIyH+WgdJV+JS67isFVcIOncsu6SFD+796ublNMMLLRK+tbP5C5gUKZZt72aI88w8sRz6jA LcK+sR1EjJVHJOFEXoQ8b46BgxywLJeXLWpOth+NPeW3ymCpOjvN2gNr7+7yNh6Cya7+2RGplC0 E9FUByMv2xU265ETS0vWLQe7vW0EIG2IqyFK0khcpZquyr3nUhVGxm5mkRhkr82j/WczokWZV0d YZ0sgHZOA5opQcJXzoTf6GfP8YJ7lG3bK1//pSUGrh/LDupMER/xl/1XiJ09wGkkbj8FJ6g1IHE DiGFfWH5wWNqEMVIWz6e9WnqI5Ht/jcnLs4F4TCmdXzkd3/uWcdystCRsKI5vYLFwTH1ivnGBTT COi2G9ur4YDnzKVCJVR1dTqvaxfHdUPR9jxvXGYRibm/f2+BQ7yMoYDncLXFXY6IG+BlIeFu2PM UJdTbHprkM= X-Received: by 2002:a05:6000:310b:b0:487:1662:cf4c with SMTP id ffacd0b85a97d-4871e20b396mr14016818f8f.7.1789948035485; Sun, 20 Sep 2026 16:47:15 -0700 (PDT) Received: from [192.168.0.38] ([86.12.216.189]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4872446067asm18875736f8f.12.2026.09.20.16.47.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 20 Sep 2026 16:47:15 -0700 (PDT) Message-ID: <29b277e4-1a9c-4636-aa05-f67a0d900bd9@gmail.com> Date: Mon, 21 Sep 2026 00:47:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] gdb, aarch64: cache pointer authentication masks per inferior To: Simon Marchi , 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> <1f40fceb-bcd3-450e-9454-f92054cebcce@simark.ca> Content-Language: en-US From: Luis In-Reply-To: <1f40fceb-bcd3-450e-9454-f92054cebcce@simark.ca> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 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 >> >> @@ -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. > 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 (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 > 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 (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.