From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x742.google.com (mail-qk1-x742.google.com [IPv6:2607:f8b0:4864:20::742]) by sourceware.org (Postfix) with ESMTPS id A664B393A075 for ; Fri, 17 Jul 2020 12:33:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A664B393A075 Received: by mail-qk1-x742.google.com with SMTP id r22so8497694qke.13 for ; Fri, 17 Jul 2020 05:33:29 -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=3YueWc9DRViv9yfY3n1qph1Fiy8Fza3p5lCmcpL9Ymc=; b=Fge6X65eNveILo7tzRQQbXMNWWr7Y6fuHBLFk+2xWZgT0k29lwb2b8cr3e9YpR86UM TqzI5c2eGprdTAVUCE4OQaM1GWgjfBNSmW0dad8/VIdbyrIQyop37Peus6Q3DlfukSW1 B0YNEM/EV/k4z8ZmrCuPTXhpqZqrLyWGeF9whV0Iq8ojF1uGv7uoLUJioUatV45Bkoch 3Qt2ui91hHsvaLG0TRbOAZNNuMNzHcC39uaysxFWKuWc5Mt3Cdt5xpKPeHboGW2gHI1c AIRR0bKQhshYBzX3N2crO7tI5nsRZgTX/91/htclHv5hszBQOOvf2HOAQZlezp/hnavV C2Mg== X-Gm-Message-State: AOAM531UsPgWRUYD+ruuR54cQEhySPPWo9RuvXcr/pD7NZ8777sYfEQJ 4J3uyGoiN+NW9RE9hc3UH+gBnA== X-Google-Smtp-Source: ABdhPJz9Cc8vCCGpTrWZ5ErR+wZEh+aDGMFyiWDQoPCXYWoMlTAu5mneqXRaK2LWrbfgSWuicmZ4iQ== X-Received: by 2002:a37:5bc6:: with SMTP id p189mr8533058qkb.112.1594989209150; Fri, 17 Jul 2020 05:33:29 -0700 (PDT) Received: from ?IPv6:2804:7f0:8283:82c3:cd49:627f:e7fe:c7df? ([2804:7f0:8283:82c3:cd49:627f:e7fe:c7df]) by smtp.gmail.com with ESMTPSA id 142sm9914655qki.35.2020.07.17.05.33.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 17 Jul 2020 05:33:28 -0700 (PDT) Subject: Re: [PATCH 00/23] Memory Tagging Support + AArch64 Linux implementation To: Alan Hayward Cc: gdb-patches , Omair Javaid , Catalin Marinas , "david.spickett@linaro.org" , "jose.marchesi@oracle.com" , nd References: <20200715194513.16641-1-luis.machado@linaro.org> <733523A7-36D3-40E8-860F-8536CBA921AA@arm.com> From: Luis Machado Message-ID: <8e4fa724-8d66-7997-e8b6-80c7b97bf00e@linaro.org> Date: Fri, 17 Jul 2020 09:33:24 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <733523A7-36D3-40E8-860F-8536CBA921AA@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: Fri, 17 Jul 2020 12:33:40 -0000 On 7/16/20 1:49 PM, Alan Hayward wrote: > > >> On 15 Jul 2020, at 20:44, Luis Machado wrote: >> >> This patch series implements general memory tagging support for GDB, as well >> as an implementation for AArch64 Linux. >> >> Memory tagging improves memory safety by tagging various parts of memory and >> raising exceptions when the allocation tag (the one associated with a range of >> memory addresses) does not match the logical tag contained in a pointer that is >> used to access the memory area. >> >> We already have an implementation of such a mechanism for sparc64 (ADI), but >> it is target-specific and not exposed to the rest of GDB. This series aims to >> make the infrastructure available to other targets that may wish to support >> their specific memory tagging approaches. For AArch64 Linux this is called >> MTE (Memory Tagging Extensions). >> >> The series is split into a set that deals with generic changes to GDB's >> infrastructure (target methods, gdbarch hooks and remote packets), a set that >> implements support for AArch64 Linux and one last set that implements new >> commands, updates the documentation and adds tests. >> >> The goal is to make it so the architecture independent parts of GDB don't >> need to interpret tag formats, given the formats are likely different >> for each architecture. For this reason, GDB will handle tags as a sequence of >> bytes and will not assume a particular format. >> >> The architecture-specific code can handle the sequence of bytes appropriately. > > I’ve reviewed earlier versions of this series, and my comments have been applied. > I’ve still taken a look through each patch in this latest series, and I’m mostly > happy now. Instead of replying to each patch in turn, these are my bulk set of > comments: > > [PATCH 01/23] New target methods for memory tagging support > [PATCH 02/23] New gdbarch memory tagging hooks > - Looks good to me. > > [PATCH 03/23] Add GDB-side remote target support for memory tagging > [PATCH 04/23] Unit testing for GDB-side remote memory tagging handling > [PATCH 05/23] GDBserver remote packet support for memory tagging > [PATCH 06/23] Unit tests for gdbserver memory tagging remote packets > [PATCH 07/23] Documentation for memory tagging remote packets > - Looks ok, but I’m not that familiar with the remote packet interface. > Worth someone else looking over. > > [PATCH 08/23] AArch64: Add MTE CPU feature check support > [PATCH 09/23] AArch64: Add target description/feature for MTE registers > [PATCH 10/23] AArch64: Add MTE register set support for GDB and gdbserver > [PATCH 11/23] AArch64: Add MTE ptrace requests > [PATCH 12/23] AArch64: Implement memory tagging target methods for AArch64 > [PATCH 13/23] Refactor parsing of /proc//smaps > [PATCH 14/23] AArch64: Implement the memory tagging gdbarch hooks > [PATCH 15/23] AArch64: Add unit testing for logical tag set/get operations > [PATCH 16/23] AArch64: Report tag violation error information > [PATCH 17/23] AArch64: Add gdbserver MTE support > - These all Look good to me. My only concern here is that the Linux kernel support > is a WIP. But given GDB is the main user of these interfaces, and that you’ve been > reviewing the Linux patches, then I’m happy as long as they have all been tested > together. I have exercised the GDB series with Catalin's kernel series under QEMU. So the GDB changes match what the kernel is exposing right now. > > [PATCH 18/23] New mtag commands > [PATCH 19/23] Documentation for the new mtag commands > - I’m happy with the new commands. But would like to know what others think, and > I’m not familiar enough with the implementation of the commands to do a detailed > review. Same here. The idea is to provide some basic commands as a first step. Other commands can be added as needed. > > [PATCH 20/23] Extend "x" and "print" commands to support memory tagging > - Am I correct in thinking that p/x cannot cause a segfault? (we don’t want > one to happen) They can run into a segfault if, for example, you invoke a function that segfaults. But they won't trigger one if you attempt to access variables using invalid tags. Those will only happen when the process itself causes a tag violation. GDB strips the top byte of the pointer on memory operations as well, so the addresses will have no tags in most cases. > > [PATCH 21/23] Document new "x" and "print" memory tagging extensions > [PATCH 22/23] Add NEWS entry. > - Looks good (assuming the commands aren’t changed) > > >> >> Luis Machado (23): >> New target methods for memory tagging support >> New gdbarch memory tagging hooks >> Add GDB-side remote target support for memory tagging >> Unit testing for GDB-side remote memory tagging handling >> GDBserver remote packet support for memory tagging >> Unit tests for gdbserver memory tagging remote packets >> Documentation for memory tagging remote packets >> AArch64: Add MTE CPU feature check support >> AArch64: Add target description/feature for MTE registers >> AArch64: Add MTE register set support for GDB and gdbserver >> AArch64: Add MTE ptrace requests >> AArch64: Implement memory tagging target methods for AArch64 >> Refactor parsing of /proc//smaps >> AArch64: Implement the memory tagging gdbarch hooks >> AArch64: Add unit testing for logical tag set/get operations >> AArch64: Report tag violation error information >> AArch64: Add gdbserver MTE support >> New mtag commands >> Documentation for the new mtag commands >> Extend "x" and "print" commands to support memory tagging >> Document new "x" and "print" memory tagging extensions >> Add NEWS entry. >> Add memory tagging testcases >> >> gdb/Makefile.in | 3 + >> gdb/NEWS | 32 ++ >> gdb/aarch64-linux-nat.c | 121 ++++++- >> gdb/aarch64-linux-tdep.c | 330 ++++++++++++++++- >> gdb/aarch64-tdep.c | 40 ++- >> gdb/aarch64-tdep.h | 12 +- >> gdb/arch-utils.c | 50 +++ >> gdb/arch-utils.h | 23 ++ >> gdb/arch/aarch64-mte-linux.c | 70 ++++ >> gdb/arch/aarch64-mte-linux.h | 66 ++++ >> gdb/arch/aarch64.c | 7 +- >> gdb/arch/aarch64.h | 7 +- >> gdb/configure.nat | 3 +- >> gdb/configure.tgt | 1 + >> gdb/doc/gdb.texinfo | 174 ++++++++- >> gdb/features/Makefile | 1 + >> gdb/features/aarch64-mte.c | 14 + >> gdb/features/aarch64-mte.xml | 11 + >> gdb/gdbarch.c | 137 ++++++++ >> gdb/gdbarch.h | 53 +++ >> gdb/gdbarch.sh | 36 ++ >> gdb/linux-tdep.c | 356 ++++++++++++------- >> gdb/linux-tdep.h | 4 + >> gdb/nat/aarch64-mte-linux-ptrace.c | 200 +++++++++++ >> gdb/nat/aarch64-mte-linux-ptrace.h | 50 +++ >> gdb/printcmd.c | 468 ++++++++++++++++++++++++- >> gdb/remote.c | 230 ++++++++++++ >> gdb/target-delegates.c | 84 +++++ >> gdb/target.h | 25 ++ >> gdb/testsuite/gdb.arch/aarch64-mte.c | 107 ++++++ >> gdb/testsuite/gdb.arch/aarch64-mte.exp | 371 ++++++++++++++++++++ >> gdb/testsuite/gdb.base/memtag.c | 22 ++ >> gdb/testsuite/gdb.base/memtag.exp | 64 ++++ >> gdb/testsuite/lib/gdb.exp | 16 + >> gdb/valprint.h | 1 + >> gdbserver/Makefile.in | 1 + >> gdbserver/configure.srv | 2 + >> gdbserver/linux-aarch64-ipa.cc | 8 +- >> gdbserver/linux-aarch64-low.cc | 89 ++++- >> gdbserver/linux-aarch64-tdesc.cc | 10 +- >> gdbserver/linux-aarch64-tdesc.h | 3 +- >> gdbserver/remote-utils.cc | 40 +-- >> gdbserver/remote-utils.h | 2 + >> gdbserver/server.cc | 214 +++++++++++ >> gdbserver/server.h | 3 + >> gdbserver/target.cc | 20 ++ >> gdbserver/target.h | 17 + >> gdbsupport/rsp-low.cc | 2 +- >> include/elf/common.h | 3 + >> 49 files changed, 3421 insertions(+), 182 deletions(-) >> create mode 100644 gdb/arch/aarch64-mte-linux.c >> create mode 100644 gdb/arch/aarch64-mte-linux.h >> create mode 100644 gdb/features/aarch64-mte.c >> create mode 100644 gdb/features/aarch64-mte.xml >> create mode 100644 gdb/nat/aarch64-mte-linux-ptrace.c >> create mode 100644 gdb/nat/aarch64-mte-linux-ptrace.h >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-mte.c >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-mte.exp >> create mode 100644 gdb/testsuite/gdb.base/memtag.c >> create mode 100644 gdb/testsuite/gdb.base/memtag.exp >> >> -- >> 2.17.1 >> >