From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id AedjFLvd5l8iawAAWB0awg (envelope-from ) for ; Sat, 26 Dec 2020 01:52:43 -0500 Received: by simark.ca (Postfix, from userid 112) id 444141F0AA; Sat, 26 Dec 2020 01:52:43 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 1B9F51E99A for ; Sat, 26 Dec 2020 01:52:42 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 790233858D29; Sat, 26 Dec 2020 06:52:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 790233858D29 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1608965561; bh=+sotmfPWGmVwXLFh040mGfjDYe63Mv51R0/UsWiuuT4=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=NF1L3LQuWnxa6EZ6m0JL3DM1nPH993fsGmrXLTZ1e5QZbUOuWhIH0WEPD9xOWv2jU Y1KlMwnMJdAe5KjIsb4LoeCFwkBfIM14Jlo9X66QjYe8IwE42McO0F4DjTntyFUlEA rxaxMjAsQ+SXJexgAgKQapq6jKAP5z9BP16daCaU= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id AB4923858D29 for ; Sat, 26 Dec 2020 06:52:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AB4923858D29 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 0BQ6qW0w010467 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 26 Dec 2020 01:52:37 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 0BQ6qW0w010467 Received: from [10.0.0.213] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 530FA1E99A; Sat, 26 Dec 2020 01:52:32 -0500 (EST) Subject: Re: [PATCH v3 14/24] AArch64: Implement the memory tagging gdbarch hooks To: Luis Machado , gdb-patches@sourceware.org References: <20201109170435.15766-1-luis.machado@linaro.org> <20201109170435.15766-15-luis.machado@linaro.org> Message-ID: <7811415a-320c-5672-b4d8-a11425372c2f@polymtl.ca> Date: Sat, 26 Dec 2020 01:52:31 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20201109170435.15766-15-luis.machado@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sat, 26 Dec 2020 06:52:32 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: david.spickett@linaro.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2020-11-09 12:04 p.m., Luis Machado via Gdb-patches wrote: > Updates on v2: > > - Update target methods to contain a tag type parameter. > > -- > > This patch implements the memory tagging gdbarch hooks for AArch64, for > the MTE feature. > > gdb/ChangeLog: > > YYYY-MM-DD Luis Machado > > * aarch64-linux-tdep.c: Include target.h, arch-utils.h, value.h. > (aarch64_linux_get_atag, aarch64_linux_tagged_address_p) > (aarch64_linux_memtag_mismatch_p, aarch64_linux_set_memtags) > (aarch64_linux_get_memtag, aarch64_linux_memtag_to_string): New > functions. > (aarch64_linux_init_abi): Initialize MTE-related gdbarch hooks. > * arch/aarch64-mte-linux.c (make_ltag_bits, make_ltag) > (aarch64_linux_set_ltag, aarch64_linux_get_ltag): New functions. > * arch/aarch64-mte-linux.h (MTE_LOGICAL_TAG_START_BIT) > (MTE_LOGICAL_MAX_VALUE): Define. > (make_ltag_bits, make_ltag, aarch64_linux_set_ltag) > (aarch64_linux_get_ltag): New prototype. > --- > gdb/aarch64-linux-tdep.c | 212 +++++++++++++++++++++++++++++++++++ > gdb/arch/aarch64-mte-linux.c | 36 ++++++ > gdb/arch/aarch64-mte-linux.h | 19 ++++ > 3 files changed, 267 insertions(+) > > diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c > index 5f6fb42792..00c4f45035 100644 > --- a/gdb/aarch64-linux-tdep.c > +++ b/gdb/aarch64-linux-tdep.c > @@ -30,6 +30,7 @@ > #include "symtab.h" > #include "tramp-frame.h" > #include "trad-frame.h" > +#include "target.h" > #include "target/target.h" > > #include "regcache.h" > @@ -46,6 +47,9 @@ > > #include "arch/aarch64-mte-linux.h" > > +#include "arch-utils.h" > +#include "value.h" > + > /* Signal frame handling. > > +------------+ ^ > @@ -1437,6 +1441,189 @@ aarch64_linux_gcc_target_options (struct gdbarch *gdbarch) > return {}; > } > > +/* Helper to get the allocation tag from a 64-bit ADDRESS. > + > + Return 0 for success and non-zero otherwise. */ > + > +static int > +aarch64_linux_get_atag (CORE_ADDR address, CORE_ADDR *tag) > +{ I'd suggest making the return value a bool. Or, even better IMO would be to make the return value gdb::optional, an empty optional meaning failure. > + gdb::byte_vector tags; > + > + /* Attempt to fetch the allocation tag. */ > + if (target_fetch_memtags (address, 0, tags, tag_allocation) != 0) > + return 1; > + > + /* Only one tag should've been returned. Make sure we got exactly that. */ > + gdb_assert (tags.size () == 1); > + > + /* Although our tags are 4 bits in size, they are stored in a > + byte. */ > + *tag = tags[0]; > + > + return 0; > +} > + > +/* Implement the tagged_address_p gdbarch method. */ > + > +static bool > +aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, struct value *address) > +{ > + gdb_assert (address != nullptr); > + > + CORE_ADDR addr = value_as_address (address); > + > + /* Remove the top byte for the memory range check. */ > + addr = address_significant (gdbarch, addr); > + > + /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */ > + if (!linux_address_in_memtag_page (addr)) > + return false; > + > + /* We have a valid tag in the top byte of the 64-bit address. */ > + return true; > +} > + > +/* Implement the memtag_mismatch_p gdbarch method. */ > + > +static bool > +aarch64_linux_memtag_mismatch_p (struct gdbarch *gdbarch, > + struct value *address) > +{ > + gdb_assert (address != nullptr); > + > + /* Make sure we are dealing with a tagged address to begin with. */ > + if (!aarch64_linux_tagged_address_p (gdbarch, address)) > + return false; > + > + CORE_ADDR addr = value_as_address (address); > + > + /* Fetch the allocation tag for ADDRESS. */ > + CORE_ADDR atag = 0; > + > + if (aarch64_linux_get_atag (addr, &atag) != 0) > + return false; > + > + /* Fetch the logical tag for ADDRESS. */ > + gdb_byte ltag = aarch64_linux_get_ltag (addr); > + > + /* Are the tags the same? */ > + if (ltag == atag) > + return false; > + > + return true; That could very well be return ltag != atag; I'm wondering about the potential performance impacts of all this. When memory tagging is in use, how often are the memtag functions expected to be called? By the looks of it, it seems like checking for a mismatch does 2 open and parse of /proc/pid/smaps. Is that right, and is this function something that is going to be called often, say, when printing values? > +} > + > +/* Implement the set_memtags gdbarch method. */ > + > +static int > +aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address, > + size_t length, const gdb::byte_vector &tags, > + enum memtag_type tag_type) > +{ > + if (tags.empty ()) > + return 0; > + > + gdb_assert (address != nullptr); > + > + CORE_ADDR addr = value_as_address (address); > + > + /* Set the logical tag or the allocation tag. */ > + if (tag_type == tag_logical) > + { > + /* When setting logical tags, we don't care about the length, since > + we are only setting a single logical tag. */ > + addr = aarch64_linux_set_ltag (addr, tags[0]); > + > + /* Update the value's content with the tag. */ > + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > + gdb_byte *srcbuf = value_contents_raw (address); > + store_unsigned_integer (srcbuf, sizeof (addr), byte_order, addr); > + } > + else > + { > + /* Make sure we are dealing with a tagged address to begin with. */ > + if (!aarch64_linux_tagged_address_p (gdbarch, address)) > + return 1; > + > + /* With G being the number of tag granules and N the number of tags > + passed in, we can have the following cases: > + > + 1 - G == N: Store all the N tags to memory. > + > + 2 - G < N : Warn about having more tags than granules, but write G > + tags. > + > + 3 - G > N : This is a "fill tags" operation. We should use the tags > + as a pattern to fill the granules repeatedly until we have > + written G tags to memory. > + */ > + > + size_t g = get_tag_granules (addr, length, MTE_GRANULE_SIZE); > + size_t n = tags.size (); > + > + if (g < n) > + { > + warning (_("Got more tags than memory granules. Tags will be " > + "truncated.")); > + } Remove curly braces. > diff --git a/gdb/arch/aarch64-mte-linux.h b/gdb/arch/aarch64-mte-linux.h > index e555f0af19..5c5783f28b 100644 > --- a/gdb/arch/aarch64-mte-linux.h > +++ b/gdb/arch/aarch64-mte-linux.h > @@ -32,10 +32,29 @@ > > /* We have one tag per 16 bytes of memory. */ > #define MTE_GRANULE_SIZE 16 > +#define MTE_LOGICAL_TAG_START_BIT 56 > +#define MTE_LOGICAL_MAX_VALUE 0xf > > /* Return the number of tag granules in the memory range > [ADDR, ADDR + LEN) given GRANULE_SIZE. */ > extern size_t get_tag_granules (CORE_ADDR addr, size_t len, > size_t granule_size); > > +/* Return the 4-bit tag made from VALUE. */ > +extern CORE_ADDR make_ltag_bits (CORE_ADDR value); As mentioned previously, I'd suggest giving a name that's better scoped, since these symbols are in a header file / exported. Or if they are just used in aarch64-mte-linux.c, move them there (the defines as well, if possible). Simon