public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
To: Luis Machado <luis.machado@arm.com>
Cc: lsix@lancelotsix.com, gdb-patches@sourceware.org
Subject: Re: [PATCH,v2] [aarch64] Fix removal of non-address bits for PAuth
Date: Thu, 18 Aug 2022 23:47:17 +0000	[thread overview]
Message-ID: <87o7wh2jy2.fsf@linaro.org> (raw)
In-Reply-To: <20220711115534.23810-1-luis.machado@arm.com>


Hello Luis,

The patch looks great to me, FWIW.
I have just a few small comments.

Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

> While at it, make GDB and GDBServer share some more code for aarch64

Nice!

> +/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> +   non address bits from a pointer value.  */
> +
> +static CORE_ADDR
> +aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> +  aarch64_gdbarch_tdep *tdep
> +    = (aarch64_gdbarch_tdep *) gdbarch_tdep (gdbarch);
> +
> +  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +     select bit (55).  */
> +  CORE_ADDR mask = (((CORE_ADDR) 1) << (64 - 9)) - 1;
> +  mask = ~mask;

One suggestion is to put the above in gdb/arch/aarch64.h as:

/* By default, we assume TBI and discard the top 8 bits plus the VA range
   select bit (55).  */
#define AARCH64_TOP_BITS_MASK (~((((CORE_ADDR) 1) << (64 - 9)) - 1))

and then use it here and in gdbserver's version of this function so that
there's a little bit more code shared between them.

> +
> +  if (tdep->has_pauth ())
> +    {
> +      /* Fetch the PAC masks.  These masks are per-process, so we can just
> +	 fetch data from whatever thread we have at the moment.
> +
> +	 Also, we have both a code mask and a data mask.  For now they are the
> +	 same, but this may change in the future.  */
> +      struct regcache *regs = get_current_regcache ();
> +      CORE_ADDR cmask, dmask;
> +
> +      if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
> +	dmask = mask;
> +
> +      if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
> +	cmask = mask;
> +
> +      if (dmask != cmask)
> +	{
> +	  /* Warn if the masks are different.  */
> +	  warning (_("C mask and D mask differ"));

This warning is a bit hard to read if one isn't familiar enough with
PAuth. Perhaps something like “Pointer authentication masks for code and
data differ”?

Same comment applies to the warning message in the gdbserver version of
this function.

> diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
> index fc10e8600ba..c6836b63c50 100644
> --- a/gdb/gdbarch-components.py
> +++ b/gdb/gdbarch-components.py
> @@ -1116,15 +1116,21 @@ possible it should be in TARGET_READ_PC instead).
>      invalid=False,
>  )
>  
> -Value(
> +Method(
>      comment="""
> -On some machines, not all bits of an address word are significant.
> -For example, on AArch64, the top bits of an address known as the "tag"
> -are ignored by the kernel, the hardware, etc. and can be regarded as
> -additional data associated with the address.
> +On some architectures, not all bits of a pointer are significant.
> +On AArch64, for example, the top bits of a pointer may carry a "tag", which
> +can be ignored by the kernel and the hardware. The "tag" can be regarded as
> +additional data associated with the pointer, but it is not part of the address.
> +
> +Given a pointer for the architecture, this hook removes all the
> +non-significant bits and sign-extends things as needed. It is used mostly
> +for data pointers, as opposed to code pointers.

Is the last sentence just FYI or does it imply some consequence for the
GDB user or even GDB itself?

-- 
Thiago

  parent reply	other threads:[~2022-08-18 23:47 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05 14:00 [PATCH] [AArch64] " Luis Machado
2022-07-05 18:12 ` John Baldwin
2022-07-06 11:38 ` Lancelot SIX
2022-07-08 11:36   ` Luis Machado
2022-07-11 11:55 ` [PATCH,v2] [aarch64] " Luis Machado
2022-07-18  8:16   ` [Ping v1][PATCH,v2] " Luis Machado
2022-08-01 11:09     ` [Ping v2][PATCH,v2] " Luis Machado
2022-08-08 11:34   ` [Ping v3][PATCH,v2] " Luis Machado
2022-08-18 15:49   ` [Ping v4][PATCH,v2] " Luis Machado
2022-08-18 23:47   ` Thiago Jung Bauermann [this message]
2022-08-19  9:52     ` [PATCH,v2] " Luis Machado
2022-08-19 14:06       ` Thiago Jung Bauermann
2022-08-23 20:29 ` [PATCH,v3] " Luis Machado
2022-08-24 18:44   ` Thiago Jung Bauermann
2022-09-01  9:29   ` [PING][PATCH,v3] " Luis Machado
2022-09-07  8:21   ` Luis Machado
2022-09-12 12:47   ` Luis Machado
2022-09-20 12:26   ` Luis Machado
2022-09-22 12:59   ` [PATCH,v3] " Lancelot SIX
2022-09-22 16:39     ` Luis Machado
2022-09-23  7:58       ` Lancelot SIX
2022-10-03 11:37   ` [PING][PATCH,v3] " Luis Machado
2022-10-10 12:18   ` Luis Machado
2022-10-17 10:04   ` Luis Machado
2022-10-25 13:52   ` Luis Machado
2022-11-10  1:00   ` Luis Machado
2022-11-29 22:19   ` Luis Machado
2022-12-09 16:42   ` Luis Machado
2022-12-09 19:14   ` [PATCH,v3] " Simon Marchi
2022-12-12 14:21     ` Luis Machado
2022-12-12 15:07       ` Simon Marchi
2022-12-12 17:13 ` [PATCH v4] " Luis Machado
2022-12-12 18:54   ` Simon Marchi
2022-12-13  9:18     ` Luis Machado
2022-12-13 10:27 ` [PATCH v5] " Luis Machado
2022-12-16 10:57 ` [PATCH v6] " Luis Machado
2022-12-16 11:20   ` 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=87o7wh2jy2.fsf@linaro.org \
    --to=thiago.bauermann@linaro.org \
    --cc=gdb-patches@sourceware.org \
    --cc=lsix@lancelotsix.com \
    --cc=luis.machado@arm.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).