From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa34.google.com (mail-vk1-xa34.google.com [IPv6:2607:f8b0:4864:20::a34]) by sourceware.org (Postfix) with ESMTPS id 721D43858D39 for ; Thu, 18 Aug 2022 23:47:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 721D43858D39 Received: by mail-vk1-xa34.google.com with SMTP id g17so463899vkk.6 for ; Thu, 18 Aug 2022 16:47:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:in-reply-to :subject:cc:to:from:user-agent:references:x-gm-message-state:from:to :cc; bh=LMUZPDvxcyKTsjJDplGu1yZyodi/niaDNvcjMtGIBwQ=; b=c2ilwNxiKyeqAqp7IeissYnfT3adUUAAdV+8LlwyYNyM9qwh/qXh7ZVW90WfZwr0Ve obIyjEsD6SsuBYQNfKvGebyUxoUfvInb3HOa7FAAag7y22YeomBFNT1OJg4NresfIXMJ 2OkL9dWL5t5jjfMzoZYlzggj8tSDLpJHtMA5vC8xPN1g+Vq+rVXE+cZqxGk9mV2eASK7 iO04kL6on5qmiztvCPVuf89Q1x5ln6wpkVNjbvAemiu+wgRYMGt1yeBDHZzqm3/EonLb /mEkHdlRGiUfa5HsQSh36vhAKbnyB7et//B+38H3Rv4GMmodHWKYjrl1kWwZvvSEcJdS nu4g== X-Gm-Message-State: ACgBeo3JMFAuhoaIuSIgf82g2lDjsrbFOSgrIOUkXTklXwaqSByzcIKo B2hzlyv7Rgj197aoK639qLynT84wVSwmzw== X-Google-Smtp-Source: AA6agR6J9CwKhctRJG2OOHhZnamVvwREtUu9E/bAh4wjcNbGdOGRdT+pqc+JyFoZ9D1/KQGQg6JU7w== X-Received: by 2002:a1f:2b05:0:b0:383:4805:58cf with SMTP id r5-20020a1f2b05000000b00383480558cfmr2033162vkr.0.1660866440724; Thu, 18 Aug 2022 16:47:20 -0700 (PDT) Received: from localhost ([2804:14c:8780:89d8:3b79:a6db:9142:5828]) by smtp.gmail.com with ESMTPSA id z20-20020ab06294000000b00384ca77a9e8sm1642007uao.27.2022.08.18.16.47.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Aug 2022 16:47:19 -0700 (PDT) References: <20220705140037.135012-1-luis.machado@arm.com> <20220711115534.23810-1-luis.machado@arm.com> User-agent: mu4e 1.8.8; emacs 28.1 From: Thiago Jung Bauermann To: Luis Machado Cc: lsix@lancelotsix.com, gdb-patches@sourceware.org Subject: Re: [PATCH,v2] [aarch64] Fix removal of non-address bits for PAuth In-reply-to: <20220711115534.23810-1-luis.machado@arm.com> Date: Thu, 18 Aug 2022 23:47:17 +0000 Message-ID: <87o7wh2jy2.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Aug 2022 23:47:24 -0000 Hello Luis, The patch looks great to me, FWIW. I have just a few small comments. Luis Machado via Gdb-patches 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 poin= ter) > +{ > + aarch64_gdbarch_tdep *tdep > + =3D (aarch64_gdbarch_tdep *) gdbarch_tdep (gdbarch); > + > + /* By default, we assume TBI and discard the top 8 bits plus the VA ra= nge > + select bit (55). */ > + CORE_ADDR mask =3D (((CORE_ADDR) 1) << (64 - 9)) - 1; > + mask =3D ~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 ju= st > + 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 =3D get_current_regcache (); > + CORE_ADDR cmask, dmask; > + > + if (regs->cooked_read (tdep->pauth_reg_base, &dmask) !=3D REG_VALI= D) > + dmask =3D mask; > + > + if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) !=3D REG_= VALID) > + cmask =3D mask; > + > + if (dmask !=3D 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 =E2=80=9CPointer authentication masks for cod= e and data differ=E2=80=9D? 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=3DFalse, > ) >=20=20 > -Value( > +Method( > comment=3D""" > -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", wh= ich > +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 a= ddress. > + > +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? --=20 Thiago