From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x730.google.com (mail-qk1-x730.google.com [IPv6:2607:f8b0:4864:20::730]) by sourceware.org (Postfix) with ESMTPS id 40FD0384840F for ; Thu, 24 Jun 2021 14:37:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 40FD0384840F Received: by mail-qk1-x730.google.com with SMTP id c138so15056625qkg.5 for ; Thu, 24 Jun 2021 07:37:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=eT/EDDDA1DybsAAsFJA05QXENQ4hhnoojBr3CUnWjKQ=; b=GN9MuU4DEoyjrZM0LPFC90/ft4wT3t8AWR1lruRVF8Z7lTOUbL1A+yAGGZ6/esBlkX fdsTvH+NNixUV1oqqSnh4KLox5xue2rvHLR2Tr0wRdZ4D6/auZtWqWSmO1Hjj8yi1BM3 1tdTjOVqt7xJ0/L01T8V+4YbMjOzNkaCvnJwk6CJ87y5Hy8zHRqdBvMPJoaLtH0ZzeCm qa1tPRaBnp3Fgn+9c/Q0k97w7x80iOn3vh+G02ZP6vjNthd+0PvvhTuFyEr/qLHpXeg0 RAtTXaMFZ9R049EHa0bCTsU84VdeLIaFFliWf644pPXlpGpHpskAYNE+knoZKvWX7MsK 2h1Q== X-Gm-Message-State: AOAM532oAp+kLwcUPTM1YdfdB1KsZaHjCiLFMg5svzhfGWKIZjpDGwsq 6xfgiD8KoQ7VtU+Z/T3W7YTK/Q== X-Google-Smtp-Source: ABdhPJyH7QkkEVdQXaS1d3TRrECZr70yYfwuHVieS7nx1fBEIveB2BjsUvVVEHpQJit3ROgpCRIWBQ== X-Received: by 2002:a37:ac07:: with SMTP id e7mr6038583qkm.204.1624545438743; Thu, 24 Jun 2021 07:37:18 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:a0:a5f2:6f63:fee6:6d84? ([2804:7f0:4841:a0:a5f2:6f63:fee6:6d84]) by smtp.gmail.com with ESMTPSA id h2sm2720183qkf.106.2021.06.24.07.37.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 24 Jun 2021 07:37:18 -0700 (PDT) Subject: Re: [PATCH,v5][AArch64] MTE corefile support To: Alan Hayward Cc: "gdb-patches\\@sourceware.org" , Simon Marchi , "jhb@freebsd.org" , "david.spickett@linaro.org" , Catalin Marinas References: <20210518202047.3492211-1-luis.machado@linaro.org> <20210601174519.4157316-1-luis.machado@linaro.org> From: Luis Machado Message-ID: Date: Thu, 24 Jun 2021 11:37:13 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 24 Jun 2021 14:37:21 -0000 On 6/24/21 11:00 AM, Alan Hayward wrote: > > >> On 1 Jun 2021, at 18:45, Luis Machado wrote: >> >> Updates on v5: >> >> - Fixed format warning output. >> >> Updates on v4: >> >> - Calculate sizes based on individual struct field sizes. >> >> Updates on v3: >> >> - Addressed review comments. >> - New files gdbsupport/memtag.h, gdb/memtag.h and gdb/memtag.c. >> - Updated code documentation. >> - Removed code duplication. >> >> Updates on v2: >> >> - Reworked core_target::fetch_memtags to handle cases where address + len runs >> over the NT_MEMTAG note. >> - Turned a few magic numbers into constants. There is an unfortunate duplication >> of a constant (NT_MEMTAG_HEADER_SIZE). It is a generic constant, but there is >> no file generic enough that gets included by both corelow and linux-tdep. >> - More sanity checks to make sure the note format is correct. >> - Documented aarch64_linux_decode_memtag_note a little more. >> >> --- >> >> Teach GDB how to dump memory tags when using the gcore command and how >> to read them back from a core file generated via gcore or the kernel. >> >> Each tagged memory range (listed in /proc//smaps) gets dumped to its >> own NT_MEMTAG note. A section named ".memtag" is created for each of those >> when reading the core file back. >> >> Dumping memory tags >> - >> >> When using the gcore command to dump a core file, GDB will go through the maps >> in /proc//smaps looking for tagged ranges. Each of those entries gets >> passed to an arch-specific gdbarch hook that generates a vector of blobs of >> memory tag data that are blindly put into a NT_MEMTAG note. >> >> The vector is used because we may have, in the future, multiple tag types for >> a particular memory range. >> >> Each of the NT_MEMTAG notes have a generic header and a arch-specific header, >> like so: >> >> struct tag_dump_header >> { >> uint16_t format; // Only NT_MEMTAG_TYPE_AARCH_MTE at present >> uint64_t start_vma; >> uint64_t end_vma; >> }; >> >> struct tag_dump_mte >> { >> uint16_t granule_byte_size; >> uint16_t tag_bit_size; >> uint16_t __unused; >> }; >> >> The only bits meant to be generic are the tag_dump_format, start_vma and >> end_vma fields. >> >> The format-specific data is supposed to be opaque and only useful for the >> arch-specific code. >> >> We can extend the format in the future to make room for other memory tag >> layouts. >> >> Reading memory tags >> - >> >> When reading a core file that contains NT_MEMTAG entries, GDB will use >> a different approach to check for tagged memory range. Rather than looking >> at /proc//smaps, it will now look for ".memtag" sections with the right >> memory range. >> >> When reading tags, GDB will now use the core target's implementation of >> fetch_memtags (store_memtags doesn't exist for core targets). Then the data >> is fed into an arch-specific hook that will decode the memory tag format and >> return a vector of tags. >> >> I've added a test to exercise writing and reading of memory tags in core >> files. >> >> gdb/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * Makefile.in (COMMON_SFILES): Add memtag.c. >> * NEWS: Mention core file support for memory tagging. >> * aarch64-linux-tdep.c: Include elf/common.h. >> Include gdbsupport/memtag.h. >> (MAX_TAGS_TO_TRANSFER): New constant. >> (aarch64_linux_create_memtag_notes_from_range): New function. >> (aarch64_linux_decode_memtag_note): Likewise. >> (aarch64_linux_init_abi): Register new core file hooks. >> (NT_MEMTAG_TOTAL_HEADER_SIZE): New constant. >> * arch/aarch64-mte-linux.h (tag_dump_mte): New struct. >> (AARCH64_MTE_TAG_BIT_SIZE): New constant. >> * corelow.c: Include gdbsupport/memtag.h and memtag.h. >> (core_target) : New >> method overrides. >> * gdbarch.c: Regenerate. >> * gdbarch.h: Likewise. >> * gdbarch.sh (create_memtag_notes_from_range): New hook. >> (decode_memtag_note): Likewise. >> * linux-tdep.c: Include gdbsupport/memtag.h and memtag.h. >> (linux_address_in_memtag_page): Renamed to... >> (linux_process_address_in_memtag_page): ... this. >> (linux_core_file_address_in_memtag_page): New function. >> (linux_address_in_memtag_page): Likewise. >> (linux_make_memtag_corefile_notes): Likewise. >> (linux_make_corefile_notes): Handle memory tag notes. >> * memtag.c: New file. >> * memtag.h: New file. >> >> gdb/doc/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * gdb.texinfo (AArch64 Memory Tagging Extension): Mention support >> for memory tagging in core files. >> >> gdb/testsuite/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * gdb.arch/aarch64-mte-gcore.c: New file. >> * gdb.arch/aarch64-mte-gcore.exp: New file. >> >> gdbsupport/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * memtag.h: New file. >> --- >> gdb/Makefile.in | 1 + >> gdb/NEWS | 4 + >> gdb/aarch64-linux-tdep.c | 221 +++++++++++++++++++ >> gdb/arch/aarch64-mte-linux.h | 17 ++ >> gdb/corelow.c | 63 ++++++ >> gdb/doc/gdb.texinfo | 4 + >> gdb/gdbarch.c | 64 ++++++ >> gdb/gdbarch.h | 16 ++ >> gdb/gdbarch.sh | 6 + >> gdb/linux-tdep.c | 97 +++++++- >> gdb/memtag.c | 88 ++++++++ >> gdb/memtag.h | 46 ++++ >> gdb/testsuite/gdb.arch/aarch64-mte-gcore.c | 93 ++++++++ >> gdb/testsuite/gdb.arch/aarch64-mte-gcore.exp | 111 ++++++++++ >> gdbsupport/memtag.h | 39 ++++ >> 15 files changed, 867 insertions(+), 3 deletions(-) >> create mode 100644 gdb/memtag.c >> create mode 100644 gdb/memtag.h >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-mte-gcore.c >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-mte-gcore.exp >> create mode 100644 gdbsupport/memtag.h >> > > Note there were a few minor merge conflicts, nothing to worry about though. I'll check again and will refresh the patch. > > >> diff --git a/gdb/Makefile.in b/gdb/Makefile.in >> index f664d964536..12fb3b390b1 100644 >> --- a/gdb/Makefile.in >> +++ b/gdb/Makefile.in >> @@ -1100,6 +1100,7 @@ COMMON_SFILES = \ >> memattr.c \ >> memory-map.c \ >> memrange.c \ >> + memtag.c \ >> minidebug.c \ >> minsyms.c \ >> mipsread.c \ >> diff --git a/gdb/NEWS b/gdb/NEWS >> index ab678acec8b..58b9f739d4f 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -3,6 +3,10 @@ >> >> *** Changes since GDB 10 >> >> +* GDB now supports dumping memory tag data for AArch64 MTE. It also supports >> + reading memory tag data for AArch64 MTE from core files generated by >> + the gcore command or the Linux kernel. >> + >> * GDB now supports general memory tagging functionality if the underlying >> architecture supports the proper primitives and hooks. Currently this is >> enabled only for AArch64 MTE. >> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c >> index e9761ed2189..04498f3b6c0 100644 >> --- a/gdb/aarch64-linux-tdep.c >> +++ b/gdb/aarch64-linux-tdep.c >> @@ -52,6 +52,9 @@ >> #include "value.h" >> >> #include "gdbsupport/selftest.h" >> +#include "gdbsupport/memtag.h" >> + >> +#include "elf/common.h" >> >> /* Signal frame handling. >> >> @@ -1779,6 +1782,213 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch, >> } >> } >> >> +/* Memory tag note header size. Includes both the generic and the >> + arch-specific parts. */ >> +#define NT_MEMTAG_TOTAL_HEADER_SIZE (NT_MEMTAG_GENERIC_HEADER_SIZE \ >> + + NT_MEMTAG_MTE_HEADER_SIZE) >> + >> +/* Maximum number of tags to request. */ >> +#define MAX_TAGS_TO_TRANSFER 1024 >> + >> +/* AArch64 Linux implementation of the aarch64_create_memtag_notes_from_range >> + gdbarch hook. Create core file notes for memory tags. */ >> + >> +static std::vector >> +aarch64_linux_create_memtag_notes_from_range (struct gdbarch *gdbarch, >> + CORE_ADDR start_address, >> + CORE_ADDR end_address) >> +{ >> + /* We only handle MTE tags for now. */ >> + >> + /* Figure out how many tags we need to store in this memory range. */ >> + size_t granules = aarch64_mte_get_tag_granules (start_address, >> + end_address - start_address, >> + AARCH64_MTE_GRANULE_SIZE); >> + >> + /* Vector of memory tag notes. Add the MTE note (we only have MTE tags >> + at the moment). */ >> + std::vector notes (1); >> + >> + /* If there are no tag granules to fetch, just return. */ >> + if (granules == 0) >> + return notes; >> + >> + /* Adjust the MTE note size to hold the header + tags. */ >> + notes[0].resize (NT_MEMTAG_TOTAL_HEADER_SIZE + granules); > > Should this be NT_MEMTAG_TOTAL_HEADER_SIZE + (granules * AARCH64_MTE_GRANULE_SIZE) > Get_granules has already divided by the granule size. > We are storing 1 tag per byte. So the number of granules is the number of tags. If we multiply by AARCH64_MTE_GRANULE_SIZE, we will have 16 times more tags. >> + >> + CORE_ADDR address = start_address; >> + /* Vector of tags. */ >> + gdb::byte_vector tags; >> + >> + while (granules > 0) >> + { >> + /* Transfer tags in chunks. */ >> + gdb::byte_vector tags_read; >> + size_t xfer_len >> + = (granules >= MAX_TAGS_TO_TRANSFER)? >> + MAX_TAGS_TO_TRANSFER * AARCH64_MTE_GRANULE_SIZE : >> + granules * AARCH64_MTE_GRANULE_SIZE; >> + >> + if (!target_fetch_memtags (address, xfer_len, tags_read, >> + static_cast (memtag_type::allocation))) >> + { >> + warning (_("Failed to read MTE tags from memory range [%s,%s]."), >> + phex_nz (start_address, sizeof (start_address)), >> + phex_nz (end_address, sizeof (end_address))); >> + notes.resize (0); > > If you do the original resize after the while loop, then there would be no need to resize here. > I'm not sure I understand. Where do you suggest the resizing to be placed?