From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24325 invoked by alias); 2 Dec 2014 19:18:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24313 invoked by uid 89); 2 Dec 2014 19:18:41 -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: e06smtp12.uk.ibm.com Received: from e06smtp12.uk.ibm.com (HELO e06smtp12.uk.ibm.com) (195.75.94.108) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 02 Dec 2014 19:18:40 +0000 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Dec 2014 19:18:36 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 2 Dec 2014 19:18:33 -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 23BB12190041 for ; Tue, 2 Dec 2014 19:18:05 +0000 (GMT) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sB2JIWMH59375636 for ; Tue, 2 Dec 2014 19:18:32 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sB2JIQcD005271 for ; Tue, 2 Dec 2014 12:18:30 -0700 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-196.boeblingen.de.ibm.com [9.152.212.196]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sB2JIABZ004923; Tue, 2 Dec 2014 12:18:13 -0700 From: Andreas Arnez To: Pedro Alves Cc: gdb-patches@sourceware.org, 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> <87iohvp77u.fsf@br87z6lw.de.ibm.com> <547DD8C2.9000403@redhat.com> Date: Tue, 02 Dec 2014 19:18:00 -0000 In-Reply-To: <547DD8C2.9000403@redhat.com> (Pedro Alves's message of "Tue, 02 Dec 2014 15:20:34 +0000") Message-ID: <8761dtq2rx.fsf@br87z6lw.de.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14120219-0009-0000-0000-0000023B9487 X-IsSubscribed: yes X-SW-Source: 2014-12/txt/msg00034.txt.bz2 On Tue, Dec 02 2014, Pedro Alves wrote: > On 12/01/2014 06:15 PM, Andreas Arnez wrote: > >> 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. > > I've pushed the PowerPC transactional memory ptrace support toward modeling > from s390, and the current patches (iterating under review) will return > ENODATA too, but for the exact same scenario, so sounds like we'll be > good there too. > >> 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; > > What are the conditions the ARM kernel checks to return > ENODATA for this regset? I'd assume that it'd be the case of > the machine not really supporting iwmmxt, and thus the check > above already catching this. Someone with more ARM background should probably answer this, but according to elf_set_personality in arch/arm/kernel/elf.c it seems that ENODATA is also returned if the binary is neither EABI nor softfloat, even if HWCAP_IWMXXT is set. > >> >> + if (buf == NULL) >> + { >> + for (i = 0; i < 22; i++) >> + supply_register (regcache, arm_num_regs + i, NULL); >> + return; >> + } >> + > > It probably doesn't hurt to be explicit, but I should note that > registers' are unavailable by default on gdbserver, so a > 'if (buf == NULL) return;' probably would do as well: > > struct regcache * > init_register_cache (struct regcache *regcache, > const struct target_desc *tdesc, > unsigned char *regbuf) > ... > regcache->register_status = xcalloc (1, tdesc->num_registers); > gdb_assert (REG_UNAVAILABLE == 0); In general, if a prior call to fetch_inferior_registers has filled the regset already, I'd expect the store function to reset the registers to "unavailable" again. Otherwise we may have stale left-overs from before. In this particular case this situation can probably only occur if the ELF personality is changed from EABI/softfloat to non-EABI/softfloat, e.g. by an "execve". Again, someone with more ARM knowledge may jump in here. > > > More importantly: > >> @@ -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; > > Doesn't this mean a write attempt to such a read-only regset "succeeds" > silently instead of erroring? > > Does the test exercise this? How does GDB/native behave? Good point. Both gdbserver and GDB silently "succeed". I did not consider this a significant issue so far... Any suggestions how to improve this behavior? > >> @@ -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); > > 'buf != NULL ? (const...' OK, I'll change that. > >> + >> + for (i = 0; i < 16; i++) >> + supply_register (regcache, tr0 + i, >> + buf ? (const char *) buf + 8 * (16 + i) : NULL); >> +} > > 'buf != NULL ? (const...' OK.