From patchwork Wed Mar 13 15:35:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Aktemur, Tankut Baris" X-Patchwork-Id: 87132 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C0EB23857828 for ; Wed, 13 Mar 2024 15:37:02 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.15]) by sourceware.org (Postfix) with ESMTPS id 5F5A73857815 for ; Wed, 13 Mar 2024 15:36:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5F5A73857815 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=intel.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5F5A73857815 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=198.175.65.15 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710344183; cv=none; b=dP37rBvjumAkFZoII3Uyj6/Gdc+2L+ajLQFJZHZ419GPnLP3p0ZcKjry2nO2TC9jFn8WTF408uQ5zbVhGy3FS79JKXYEcQaPhYe8Nde1ATDITbn1tVkpnahss3TqyNgd9akaObc8ZtqTTjewOzO+XyZ8aJ17HJhcsIXQsfOosRs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710344183; c=relaxed/simple; bh=3tFcbnrmmlfjFQEBA6GGJpTNbDsbnoaugw5WcAES4vY=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=KtlhdKQmCFee5p9es4E/HH88+nhNrQzyrmbfTnuIVpjGTJCfpwGZ3KZPanVMeq84o7dMfyHdybfxfwxKX+jY3tFYXi6KWd91o4nylimByrRJ7KHgH6PBxtpFYUDlutW3TIj4T+cdJtyj41jVQ2K8Zi2Ri9BijOHYqFSXAepH7hI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1710344180; x=1741880180; h=from:to:subject:date:message-id:in-reply-to:references: mime-version:content-transfer-encoding; bh=3tFcbnrmmlfjFQEBA6GGJpTNbDsbnoaugw5WcAES4vY=; b=Y0B9vR5cJ6gMiKGNqJU+cgxwQoWEgVlRcKBGLcKnuWBJjiR++gx6svSh 0UBcSOb/ufsOfByddn5KoNyGGmyIZydrDaHUoUxPuUvSR0IgJ3lf8D+TE jQ8vimh0UmIhPmj+gSnPRJmNmQsxcEpzTttGew4yvBL79K6xXwxr6t6bE hPeLT+HuP4VwvLr+vGeQ2eO7srmVOpC99UDLqc0sU7+BFm9Ro+eCtan2j tmF3kyiHHRoW3kglCSXqQE5sAfEjiWEwU1l3+T6zZ0LvqJMR+ELv4SfRr /Yb+pOelrLfLMm4d9bHhqJPpcHxChleEGbyE15ySlEB0SKECVdFp2MeE6 g==; X-IronPort-AV: E=McAfee;i="6600,9927,11011"; a="8924470" X-IronPort-AV: E=Sophos;i="6.07,122,1708416000"; d="scan'208";a="8924470" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by orvoesa107.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Mar 2024 08:36:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,122,1708416000"; d="scan'208";a="42885627" Received: from gkldtt-dev-004.igk.intel.com (HELO localhost) ([10.123.221.202]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Mar 2024 08:36:19 -0700 From: Tankut Baris Aktemur To: gdb-patches@sourceware.org Subject: [PATCH 3/3] gdb, gdbserver: introduce the 'x' RSP packet for binary memory read Date: Wed, 13 Mar 2024 16:35:45 +0100 Message-Id: <990be8b42f1f6ca33ffed7a8ae7ead327009d847.1710343840.git.tankut.baris.aktemur@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Introduce an RSP packet, 'x', for reading from the remote server memory in binary format. The binary write packet, 'X' already exists. The 'x' packet is essentially the same as 'm', except that the returned data is in binary format. For transferring relatively large data (e.g. shared library files), the 'x' packet can reduce the transfer costs. For example, without this patch, fetching ~100MB of data from a remote target takes (gdb) dump binary memory temp.o 0x00007f3ba4c576c0 0x00007f3bab709400 2024-03-13 16:17:42.626 - command started 2024-03-13 16:18:24.151 - command finished Command execution time: 32.136785 (cpu), 41.525515 (wall) (gdb) whereas with this patch, we obtain (gdb) dump binary memory temp.o 0x00007fec39fce6c0 0x00007fec40a80400 2024-03-13 16:20:48.609 - command started 2024-03-13 16:21:16.873 - command finished Command execution time: 20.447970 (cpu), 28.264202 (wall) (gdb) We see improvements not only when reading bulk data as above, but also when making a large number of small memory access requests. For example, without this patch: (gdb) pipe x/100000xw $pc | wc -l 2024-03-13 16:04:57.112 - command started 25000 2024-03-13 16:05:10.798 - command finished Command execution time: 9.952364 (cpu), 13.686581 (wall) With this patch: (gdb) pipe x/100000xw $pc | wc -l 2024-03-13 16:06:48.160 - command started 25000 2024-03-13 16:06:57.750 - command finished Command execution time: 6.541425 (cpu), 9.589839 (wall) (gdb) Regression-tested on X86-64 using the unix (default) and native-extended-gdbserver board files. Reviewed-By: Eli Zaretskii --- gdb/NEWS | 7 ++++++ gdb/doc/gdb.texinfo | 29 +++++++++++++++++++++++++ gdb/remote.c | 45 +++++++++++++++++++++++++++++++++------ gdbserver/remote-utils.cc | 39 ++++++++++++++++++++++++++++----- gdbserver/remote-utils.h | 2 ++ gdbserver/server.cc | 21 ++++++++++++++++++ 6 files changed, 132 insertions(+), 11 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index d8ac0bb06a7..378ad66b284 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -152,6 +152,13 @@ QThreadOptions in qSupported QThreadOptions packet, and the qSupported response can contain the set of thread options the remote stub supports. +x + + Given ADDR and LENGTH, fetch LENGTH units from the memory at address + ADDR and send the fetched data in binary format. This packet is + equivalent to 'm', except that the data in the response are in + binary format. + *** Changes in GDB 14 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 6099d125a60..42119778ad9 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -43119,6 +43119,35 @@ for success (@pxref{Stop Reply Packets}) @cindex @samp{vStopped} packet @xref{Notification Packets}. +@item x @var{addr},@var{length} +@anchor{x packet} +@cindex @samp{x} packet +Read @var{length} addressable memory units starting at address @var{addr} +(@pxref{addressable memory unit}). Note that @var{addr} may not be aligned +to any particular boundary. + +The stub need not use any particular size or alignment when gathering +data from memory for the response; even if @var{addr} is word-aligned +and @var{length} is a multiple of the word size, the stub is free to +use byte accesses, or not. For this reason, this packet may not be +suitable for accessing memory-mapped I/O devices. +@cindex alignment of remote memory accesses +@cindex size of remote memory accesses +@cindex memory, alignment and size of remote accesses + +This packet is used only if the stub announces support for it using +@samp{qSupported}. + +Reply: +@table @samp +@item @var{XX@dots{}} +Memory contents as binary data (@pxref{Binary Data}). +The reply may contain fewer addressable memory units than requested if the +server was able to read only part of the region of memory. +@item E @var{NN} +for an error +@end table + @item X @var{addr},@var{length}:@var{XX@dots{}} @anchor{X packet} @cindex @samp{X} packet diff --git a/gdb/remote.c b/gdb/remote.c index 14c8b020b1e..406df0265eb 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -194,6 +194,7 @@ struct packet_result enum { PACKET_vCont = 0, PACKET_X, + PACKET_x, PACKET_qSymbol, PACKET_P, PACKET_p, @@ -5763,6 +5764,7 @@ static const struct protocol_feature remote_protocol_features[] = { { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed }, { "memory-tagging", PACKET_DISABLE, remote_supported_packet, PACKET_memory_tagging_feature }, + { "x", PACKET_DISABLE, remote_supported_packet, PACKET_x }, }; static char *remote_support_xml; @@ -9582,24 +9584,53 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, todo_units = std::min (len_units, (ULONGEST) (buf_size_bytes / unit_size) / 2); - /* Construct "m"","". */ + /* Determine which packet format to use. */ + char packet_format = 'm'; +DIAGNOSTIC_PUSH +DIAGNOSTIC_ERROR_SWITCH + switch (m_features.packet_support (PACKET_x)) + { + case PACKET_ENABLE: + packet_format = 'x'; + break; + case PACKET_DISABLE: + packet_format = 'm'; + break; + case PACKET_SUPPORT_UNKNOWN: + internal_error (_("remote_read_bytes_1: bad internal state")); + } +DIAGNOSTIC_POP + + /* Construct "m/x"","". */ memaddr = remote_address_masked (memaddr); p = rs->buf.data (); - *p++ = 'm'; + *p++ = packet_format; p += hexnumstr (p, (ULONGEST) memaddr); *p++ = ','; p += hexnumstr (p, (ULONGEST) todo_units); *p = '\0'; putpkt (rs->buf); - getpkt (&rs->buf); + int packet_len = getpkt (&rs->buf); + if (packet_len < 0) + return TARGET_XFER_E_IO; + if (rs->buf[0] == 'E' && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2]) && rs->buf[3] == '\0') return TARGET_XFER_E_IO; - /* Reply describes memory byte by byte, each byte encoded as two hex - characters. */ + p = rs->buf.data (); - decoded_bytes = hex2bin (p, myaddr, todo_units * unit_size); + if (packet_format == 'x') + decoded_bytes = remote_unescape_input ((const gdb_byte *) p, + packet_len, myaddr, + todo_units * unit_size); + else + { + /* Reply describes memory byte by byte, each byte encoded as two hex + characters. */ + decoded_bytes = hex2bin (p, myaddr, todo_units * unit_size); + } + /* Return what we have. Let higher layers handle partial reads. */ *xfered_len_units = (ULONGEST) (decoded_bytes / unit_size); return (*xfered_len_units != 0) ? TARGET_XFER_OK : TARGET_XFER_EOF; @@ -15839,6 +15870,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (PACKET_X, "X", "binary-download", 1); + add_packet_config_cmd (PACKET_x, "x", "binary-read", 0); + add_packet_config_cmd (PACKET_vCont, "vCont", "verbose-resume", 0); add_packet_config_cmd (PACKET_QPassSignals, "QPassSignals", "pass-signals", diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc index 5e7c38056d0..beccde1a9fe 100644 --- a/gdbserver/remote-utils.cc +++ b/gdbserver/remote-utils.cc @@ -1334,6 +1334,13 @@ decode_M_packet (const char *from, CORE_ADDR *mem_addr_ptr, hex2bin (from, *to_p, *len_ptr); } +void +decode_x_packet (const char *from, CORE_ADDR *mem_addr_ptr, + unsigned int *len_ptr) +{ + decode_m_packet_params (from, mem_addr_ptr, len_ptr, '\0'); +} + int decode_X_packet (char *from, int packet_len, CORE_ADDR *mem_addr_ptr, unsigned int *len_ptr, unsigned char **to_p) @@ -1571,18 +1578,36 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc) wait for the qRelocInsn "response". That requires re-entering the main loop. For now, this is an adequate approximation; allow GDB to access memory. */ - while (cs.own_buf[0] == 'm' || cs.own_buf[0] == 'M' || cs.own_buf[0] == 'X') + while (cs.own_buf[0] == 'm' || cs.own_buf[0] == 'M' + || cs.own_buf[0] == 'X' || cs.own_buf[0] == 'x') { CORE_ADDR mem_addr; unsigned char *mem_buf = NULL; unsigned int mem_len; + int new_packet_len = -1; - if (cs.own_buf[0] == 'm') + if (cs.own_buf[0] == 'm' || cs.own_buf[0] == 'x') { - decode_m_packet (&cs.own_buf[1], &mem_addr, &mem_len); + if (cs.own_buf[0] == 'm') + decode_m_packet (&cs.own_buf[1], &mem_addr, &mem_len); + else + decode_x_packet (&cs.own_buf[1], &mem_addr, &mem_len); + mem_buf = (unsigned char *) xmalloc (mem_len); if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0) - bin2hex (mem_buf, cs.own_buf, mem_len); + { + if (cs.own_buf[0] == 'm') + bin2hex (mem_buf, cs.own_buf, mem_len); + else + { + int out_len_units; + new_packet_len + = remote_escape_output (mem_buf, mem_len, 1, + (gdb_byte *) cs.own_buf, + &out_len_units, PBUFSIZ); + suppress_next_putpkt_log (); + } + } else write_enn (cs.own_buf); } @@ -1604,7 +1629,11 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc) write_enn (cs.own_buf); } free (mem_buf); - if (putpkt (cs.own_buf) < 0) + + int res = ((new_packet_len == -1) + ? putpkt (cs.own_buf) + : putpkt_binary (cs.own_buf, new_packet_len)); + if (res < 0) return -1; len = getpkt (cs.own_buf); if (len < 0) diff --git a/gdbserver/remote-utils.h b/gdbserver/remote-utils.h index 4681ca12f55..65ce604f9dc 100644 --- a/gdbserver/remote-utils.h +++ b/gdbserver/remote-utils.h @@ -57,6 +57,8 @@ void decode_m_packet (const char *from, CORE_ADDR * mem_addr_ptr, unsigned int *len_ptr); void decode_M_packet (const char *from, CORE_ADDR * mem_addr_ptr, unsigned int *len_ptr, unsigned char **to_p); +void decode_x_packet (const char *from, CORE_ADDR *mem_addr_ptr, + unsigned int *len_ptr); int decode_X_packet (char *from, int packet_len, CORE_ADDR * mem_addr_ptr, unsigned int *len_ptr, unsigned char **to_p); int decode_xfer_write (char *buf, int packet_len, diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 74c7763d777..96e6e551784 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -2840,6 +2840,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_memory_tagging ()) strcat (own_buf, ";memory-tagging+"); + /* Binary memory read support. */ + strcat (own_buf, ";x+"); + /* Reinitialize components as needed for the new connection. */ hostio_handle_new_gdb_connection (); target_handle_new_gdb_connection (); @@ -4690,6 +4693,24 @@ process_serial_event (void) else write_enn (cs.own_buf); break; + case 'x': + { + require_running_or_break (cs.own_buf); + decode_x_packet (&cs.own_buf[1], &mem_addr, &len); + int res = gdb_read_memory (mem_addr, mem_buf, len); + if (res < 0) + write_enn (cs.own_buf); + else + { + int out_len_units; + new_packet_len + = remote_escape_output (mem_buf, res, 1, + (gdb_byte *) cs.own_buf, + &out_len_units, PBUFSIZ); + suppress_next_putpkt_log (); + } + } + break; case 'X': require_running_or_break (cs.own_buf); if (decode_X_packet (&cs.own_buf[1], packet_len - 1,