From patchwork Tue May 22 10:30:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 27396 Received: (qmail 25889 invoked by alias); 22 May 2018 10:31:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25684 invoked by uid 89); 22 May 2018 10:31:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: 9pmail.ess.barracuda.com Received: from 9pmail.ess.barracuda.com (HELO 9pmail.ess.barracuda.com) (64.235.154.211) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 May 2018 10:31:11 +0000 Received: from mipsdag02.mipstec.com (mail2.mips.com [12.201.5.32]) by mx1401.ess.rzc.cudaops.com (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA256 bits=128 verify=NO); Tue, 22 May 2018 10:31:05 +0000 Received: from [10.20.78.154] (10.20.78.154) by mipsdag02.mipstec.com (10.20.40.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1415.2; Tue, 22 May 2018 03:31:06 -0700 Date: Tue, 22 May 2018 11:30:58 +0100 From: "Maciej W. Rozycki" To: Ulrich Weigand , CC: Andreas Arnez Subject: [PATCH] Linux/gdbserver: Correctly handle narrow big-endian register transfers Message-ID: User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 X-ClientProxiedBy: mipsdag02.mipstec.com (10.20.40.47) To mipsdag02.mipstec.com (10.20.40.47) X-BESS-ID: 1526985065-321457-19292-35697-1 X-BESS-VER: 2018.6-r1805181819 X-BESS-Apparent-Source-IP: 12.201.5.32 X-BESS-Outbound-Spam-Score: 0.00 X-BESS-Outbound-Spam-Report: Code version 3.2, rules version 3.2.2.193243 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------- 0.00 BSF_BESS_OUTBOUND META: BESS Outbound X-BESS-Outbound-Spam-Status: SCORE=0.00 using account:ESS59374 scores of KILL_LEVEL=7.0 tests=BSF_BESS_OUTBOUND X-BESS-BRTS-Status: 1 An issue with buffer overruns has been fixed with commit 48d93c7500d9 ("gdbserver fetch/store registers problem on s390x"), , that affected register transfers via the `ptrace' PTRACE_PEEKUSR and PTRACE_POKEUSR requests accessing registers or register parts that are narrower than the `ptrace' transfer data type. However that did not address the issue where on big-endian systems it's most-significant bits of such narrow register data that are appended to or fetched from the buffer. It affects big-endian n64 MIPS targets where $dspctl is 32-bit while the `ptrace' transfer data type is 64-bit and consequently the register is presented and written as all-zeros held in the most-significant part of the data quantity passed: (gdb) info registers zero at v0 v1 R0 0000000000000000 0000000000000001 0000000000000001 0000000000000000 a0 a1 a2 a3 R4 00000001200212b0 0000000000000000 0000000000000021 000000012001a260 a4 a5 a6 a7 R8 000000012001a260 0000000000000004 800000010c60c000 fffffffffffffff8 t0 t1 t2 t3 R12 0000000000000000 000000fff7edab68 0000000000000001 0000000000000000 s0 s1 s2 s3 R16 000000fff7ee2068 0000000120008b80 0000000000000000 0000000000000000 s4 s5 s6 s7 R20 000000000052e5c8 000000000052f008 0000000000000000 0000000000000000 t8 t9 k0 k1 R24 0000000000000000 00000001200027c0 0000000000000000 0000000000000000 gp sp s8 ra R28 00000001200212b0 000000ffffffc850 000000ffffffc850 0000000120005ee8 status lo hi badvaddr 0000000000109cf3 0000000000943efe 000000000000000e 000000012001a008 cause pc 0000000000800024 0000000120005ee8 fcsr fir hi1 lo1 0e800000 00f30000 0000000000000000 0101010101010101 hi2 lo2 hi3 lo3 0202020202020202 0303030303030303 0404040404040404 0505050505050505 dspctl restart 00000000 0000000000000000 (gdb) Fix this problem then by shifting the narrow chunk of data on big-endian systems such that it occupies the most-significant part at the time it is written to or read from the buffer, therefore placing it at the beginning of the memory location of the buffer used: (gdb) info registers zero at v0 v1 R0 0000000000000000 0000000000000001 0000000000000001 0000000000000000 a0 a1 a2 a3 R4 00000001200212b0 0000000000000000 0000000000000021 000000012001a260 a4 a5 a6 a7 R8 000000012001a260 0000000000000004 800000010d82e900 fffffffffffffff8 t0 t1 t2 t3 R12 0000000000000000 000000fff7edab68 0000000000000001 0000000000000000 s0 s1 s2 s3 R16 000000fff7ee2068 0000000120008b80 0000000000000000 0000000000000000 s4 s5 s6 s7 R20 000000000052e5c8 000000000052f008 0000000000000000 0000000000000000 t8 t9 k0 k1 R24 0000000000000000 00000001200027c0 0000000000000000 0000000000000000 gp sp s8 ra R28 00000001200212b0 000000ffffffc850 000000ffffffc850 0000000120005ee8 status lo hi badvaddr 0000000000109cf3 0000000000943efe 000000000000000e 000000012001a008 cause pc 0000000000800024 0000000120005ee8 fcsr fir hi1 lo1 0e800000 00f30000 0000000000000000 0101010101010101 hi2 lo2 hi3 lo3 0202020202020202 0303030303030303 0404040404040404 0505050505050505 dspctl restart 55aa33cc 0000000000000000 (gdb) gdb/gdbserver/ * linux-low.c (fetch_register) [__BYTE_ORDER == __BIG_ENDIAN]: Align partial data quantities with the buffer location pointed. (store_register) [__BYTE_ORDER == __BIG_ENDIAN]: Move partial data quantities extracted from the buffer back into their position. --- Ulrich, This fixes the problem for MIPS, however from my understanding of your commit it will break s390x unless it is further adjusted in a target-dependent manner. So my question is: does `ptrace' indeed place 32-bit quantities transferred in bits 63:32 of the 64-bit integer data quantity passed? If so, then would wrapping this into `#ifndef __s390x__' be OK with you as it's s390x that appears to be an oddball here by representing the same 32-bit integer data quantity differently between 32-bit and 64-bit systems (i.e. you can't just cast `long' to `int')? I have no access to an s390x to verify this change. This has passed regression testing with o32 and n64 targets with `gdbserver' strapped to use PTRACE_PEEKUSR and PTRACE_POKEUSR requests across all registers. Maciej --- gdb/gdbserver/linux-low.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) gdb-gdbserver-linux-fetch-store-register-uneven.diff Index: binutils/gdb/gdbserver/linux-low.c =================================================================== --- binutils.orig/gdb/gdbserver/linux-low.c 2018-05-14 04:01:44.000000000 +0100 +++ binutils/gdb/gdbserver/linux-low.c 2018-05-18 14:48:37.687902646 +0100 @@ -30,6 +30,7 @@ #include "nat/linux-ptrace.h" #include "nat/linux-procfs.h" #include "nat/linux-personality.h" +#include #include #include #include @@ -5532,6 +5533,7 @@ fetch_register (const struct usrregs_inf struct regcache *regcache, int regno) { CORE_ADDR regaddr; + int bufsize; int i, size; char *buf; int pid; @@ -5545,20 +5547,29 @@ fetch_register (const struct usrregs_inf if (regaddr == -1) return; - size = ((register_size (regcache->tdesc, regno) - + sizeof (PTRACE_XFER_TYPE) - 1) - & -sizeof (PTRACE_XFER_TYPE)); - buf = (char *) alloca (size); + size = register_size (regcache->tdesc, regno); + bufsize = ((size + sizeof (PTRACE_XFER_TYPE) - 1) + & -sizeof (PTRACE_XFER_TYPE)); + buf = (char *) alloca (bufsize); pid = lwpid_of (current_thread); for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE)) { + PTRACE_XFER_TYPE val; + errno = 0; - *(PTRACE_XFER_TYPE *) (buf + i) = + val = ptrace (PTRACE_PEEKUSER, pid, /* Coerce to a uintptr_t first to avoid potential gcc warning of coercing an 8 byte integer to a 4 byte pointer. */ (PTRACE_TYPE_ARG3) (uintptr_t) regaddr, (PTRACE_TYPE_ARG4) 0); +#if (__BYTE_ORDER == __BIG_ENDIAN) + /* Move the actual data bytes of a partial data quantity into their + positions at the end of the buffer. */ + if (size - i < sizeof (PTRACE_XFER_TYPE)) + val <<= (sizeof (PTRACE_XFER_TYPE) - (size - i)) * 8; +#endif + *(PTRACE_XFER_TYPE *) (buf + i) = val; regaddr += sizeof (PTRACE_XFER_TYPE); if (errno != 0) { @@ -5580,6 +5591,7 @@ store_register (const struct usrregs_inf struct regcache *regcache, int regno) { CORE_ADDR regaddr; + int bufsize; int i, size; char *buf; int pid; @@ -5593,11 +5605,11 @@ store_register (const struct usrregs_inf if (regaddr == -1) return; - size = ((register_size (regcache->tdesc, regno) - + sizeof (PTRACE_XFER_TYPE) - 1) - & -sizeof (PTRACE_XFER_TYPE)); - buf = (char *) alloca (size); - memset (buf, 0, size); + size = register_size (regcache->tdesc, regno); + bufsize = ((size + sizeof (PTRACE_XFER_TYPE) - 1) + & -sizeof (PTRACE_XFER_TYPE)); + buf = (char *) alloca (bufsize); + memset (buf, 0, bufsize); if (the_low_target.collect_ptrace_register) the_low_target.collect_ptrace_register (regcache, regno, buf); @@ -5607,12 +5619,21 @@ store_register (const struct usrregs_inf pid = lwpid_of (current_thread); for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE)) { + PTRACE_XFER_TYPE val; + errno = 0; + val = *(PTRACE_XFER_TYPE *) (buf + i); +#if (__BYTE_ORDER == __BIG_ENDIAN) + /* Move the actual data bytes of a partial data quantity from their + positions at the end of the buffer. */ + if (size - i < sizeof (PTRACE_XFER_TYPE)) + val = (val >> (sizeof (PTRACE_XFER_TYPE) - (size - i)) * 8 + & (((PTRACE_XFER_TYPE) 1 << (size - i) * 8) - 1)); +#endif ptrace (PTRACE_POKEUSER, pid, /* Coerce to a uintptr_t first to avoid potential gcc warning about coercing an 8 byte integer to a 4 byte pointer. */ - (PTRACE_TYPE_ARG3) (uintptr_t) regaddr, - (PTRACE_TYPE_ARG4) *(PTRACE_XFER_TYPE *) (buf + i)); + (PTRACE_TYPE_ARG3) (uintptr_t) regaddr, (PTRACE_TYPE_ARG4) val); if (errno != 0) { /* At this point, ESRCH should mean the process is