From patchwork Mon Dec 1 18:15:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Arnez X-Patchwork-Id: 4026 Received: (qmail 8880 invoked by alias); 1 Dec 2014 18:15:26 -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 8867 invoked by uid 89); 1 Dec 2014 18:15:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e06smtp16.uk.ibm.com Received: from e06smtp16.uk.ibm.com (HELO e06smtp16.uk.ibm.com) (195.75.94.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 01 Dec 2014 18:15:24 +0000 Received: from /spool/local by e06smtp16.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 1 Dec 2014 18:15:21 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp16.uk.ibm.com (192.168.101.146) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 1 Dec 2014 18:15:19 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 72B1A2190041 for ; Mon, 1 Dec 2014 18:14:50 +0000 (GMT) Received: from d06av04.portsmouth.uk.ibm.com (d06av04.portsmouth.uk.ibm.com [9.149.37.216]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sB1IFIms14680168 for ; Mon, 1 Dec 2014 18:15:18 GMT Received: from d06av04.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sB1IFHRY027338 for ; Mon, 1 Dec 2014 11:15:18 -0700 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-196.boeblingen.de.ibm.com [9.152.212.196]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sB1IFHN8027311; Mon, 1 Dec 2014 11:15:17 -0700 From: Andreas Arnez To: gdb-patches@sourceware.org Cc: Ulrich Weigand Subject: Re: [PATCH 2/2] S390: Fix gdbserver support for TDB References: <1417002962-3424-1-git-send-email-arnez@linux.vnet.ibm.com> <1417002962-3424-3-git-send-email-arnez@linux.vnet.ibm.com> Date: Mon, 01 Dec 2014 19:15:17 +0100 In-Reply-To: <1417002962-3424-3-git-send-email-arnez@linux.vnet.ibm.com> (Andreas Arnez's message of "Wed, 26 Nov 2014 12:56:02 +0100") Message-ID: <87iohvp77u.fsf@br87z6lw.de.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14120118-0025-0000-0000-000002ACA10A X-IsSubscribed: yes On Wed, Nov 26 2014, Andreas Arnez wrote: > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 01f11b7..5d1d9e1 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -4263,6 +4263,14 @@ regsets_fetch_inferior_registers (struct regsets_info *regsets_info, > free (buf); > continue; > } > + else if (errno == ENODATA) > + { > + /* ENODATA may be returned if the regset is currently > + not "active". Provide a zeroed buffer and assume > + that the store_function deals with this > + appropriately. */ > + memset (buf, 0, regset->size); > + } Well, providing a zeroed buffer may not actually the best thing to do here. It is just coincidence that the regset I targeted this for (the TDB on S390) can be recognized as invalid if the buffer contains all zeroes. What really should be done is to enrich the store_function interface such that "no data" can be distinguished from "zero data". The reason I wrote it this way is to avoid impacting other platforms. But after a quick grep through the Linux kernel it seems that there are currently only two regsets for which ENODATA can be returned: the TDB on S390 and the iWMMXt state on ARM. Here's an updated version of the patch. Instead of clearing the buffer, this version passes a NULL pointer to the store function. And the store function for iWMMXt is adjusted accordingly. Note that I've tested this on S390, but not on ARM. -- >8 -- Subject: [PATCH v2] S390: Fix gdbserver support for TDB This makes gdbserver actually provide values for the TDB registers when the inferior was stopped in a transaction. The changes in linux-low.c are needed for treating an unavailable TDB correctly and without warnings. In particular, ENODATA from ptrace is now handled by passing a NULL pointer to the store function. Since this may also occur on ARM for the iWMMXt state, the patch ensures that the respective store function can handle that. The test case 's390-tdbregs.exp' passes with this patch and fails without. gdb/gdbserver/ChangeLog: * linux-low.c (regsets_fetch_inferior_registers): Upon ENODATA from ptrace, pass a NULL pointer to the store function. (regsets_store_inferior_registers): Skip regsets without a fill_function. * linux-arm-low.c (arm_store_wmmxregset): Accept a NULL pointer as buffer and then mark the registers as unavailable. * linux-s390-low.c (s390_fill_last_break): Remove. (s390_store_tdb): New. (s390_regsets): Set fill_function to NULL for NT_S390_LAST_BREAK. Add regset for NT_S390_TDB. (s390_arch_setup): Use regset's size instead of fill_function for loop end condition. --- gdb/gdbserver/linux-arm-low.c | 7 +++++++ gdb/gdbserver/linux-low.c | 11 ++++++++++- gdb/gdbserver/linux-s390-low.c | 33 +++++++++++++++++++++++---------- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c index 8b72523..42dd521 100644 --- a/gdb/gdbserver/linux-arm-low.c +++ b/gdb/gdbserver/linux-arm-low.c @@ -199,6 +199,13 @@ arm_store_wmmxregset (struct regcache *regcache, const void *buf) if (!(arm_hwcap & HWCAP_IWMMXT)) return; + if (buf == NULL) + { + for (i = 0; i < 22; i++) + supply_register (regcache, arm_num_regs + i, NULL); + return; + } + for (i = 0; i < 16; i++) supply_register (regcache, arm_num_regs + i, (char *) buf + i * 8); diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 01f11b7..062140d 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -4263,6 +4263,14 @@ regsets_fetch_inferior_registers (struct regsets_info *regsets_info, free (buf); continue; } + else if (errno == ENODATA) + { + /* The regset is currently not "active". For regsets + where this can occur, store_function must be prepared + to deal with a NULL pointer. */ + free (buf); + buf = NULL; + } else { char s[256]; @@ -4300,7 +4308,8 @@ regsets_store_inferior_registers (struct regsets_info *regsets_info, void *buf, *data; int nt_type, res; - if (regset->size == 0 || regset_disabled (regsets_info, regset)) + if (regset->size == 0 || regset_disabled (regsets_info, regset) + || regset->fill_function == NULL) { regset ++; continue; diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c index 79fa6c0..2cb5a73 100644 --- a/gdb/gdbserver/linux-s390-low.c +++ b/gdb/gdbserver/linux-s390-low.c @@ -290,12 +290,6 @@ s390_fill_gregset (struct regcache *regcache, void *buf) /* Fill and store functions for extended register sets. */ static void -s390_fill_last_break (struct regcache *regcache, void *buf) -{ - /* Last break address is read-only. */ -} - -static void s390_store_last_break (struct regcache *regcache, const void *buf) { const char *p; @@ -316,13 +310,32 @@ s390_store_system_call (struct regcache *regcache, const void *buf) supply_register_by_name (regcache, "system_call", buf); } +static void +s390_store_tdb (struct regcache *regcache, const void *buf) +{ + int tdb0 = find_regno (regcache->tdesc, "tdb0"); + int tr0 = find_regno (regcache->tdesc, "tr0"); + int i; + + for (i = 0; i < 4; i++) + supply_register (regcache, tdb0 + i, + buf ? (const char *) buf + 8 * i : NULL); + + for (i = 0; i < 16; i++) + supply_register (regcache, tr0 + i, + buf ? (const char *) buf + 8 * (16 + i) : NULL); +} + static struct regset_info s390_regsets[] = { { 0, 0, 0, 0, GENERAL_REGS, s390_fill_gregset, NULL }, - /* Last break address is read-only; do not attempt PTRACE_SETREGSET. */ - { PTRACE_GETREGSET, PTRACE_GETREGSET, NT_S390_LAST_BREAK, 0, - EXTENDED_REGS, s390_fill_last_break, s390_store_last_break }, + /* Last break address is read-only; no fill function. */ + { PTRACE_GETREGSET, -1, NT_S390_LAST_BREAK, 0, EXTENDED_REGS, + NULL, s390_store_last_break }, { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_S390_SYSTEM_CALL, 0, EXTENDED_REGS, s390_fill_system_call, s390_store_system_call }, + /* TDB is read-only. */ + { PTRACE_GETREGSET, -1, NT_S390_TDB, 0, EXTENDED_REGS, + NULL, s390_store_tdb }, { 0, 0, 0, -1, -1, NULL, NULL } }; @@ -485,7 +498,7 @@ s390_arch_setup (void) #endif /* Update target_regsets according to available register sets. */ - for (regset = s390_regsets; regset->fill_function != NULL; regset++) + for (regset = s390_regsets; regset->size >= 0; regset++) if (regset->get_request == PTRACE_GETREGSET) switch (regset->nt_type) {