From patchwork Wed Dec 3 18:18:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Arnez X-Patchwork-Id: 4060 Received: (qmail 12412 invoked by alias); 3 Dec 2014 18:18:16 -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 12396 invoked by uid 89); 3 Dec 2014 18:18:15 -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: e06smtp10.uk.ibm.com Received: from e06smtp10.uk.ibm.com (HELO e06smtp10.uk.ibm.com) (195.75.94.106) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 03 Dec 2014 18:18:14 +0000 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 3 Dec 2014 18:18:10 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp10.uk.ibm.com (192.168.101.140) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 3 Dec 2014 18:18:09 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 9B64717D8042 for ; Wed, 3 Dec 2014 18:18:27 +0000 (GMT) Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com [9.149.37.250]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sB3II8DH42729506 for ; Wed, 3 Dec 2014 18:18:08 GMT Received: from d06av09.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sB3II8Pi009862 for ; Wed, 3 Dec 2014 11:18:08 -0700 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-196.boeblingen.de.ibm.com [9.152.212.196]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sB3II7Nf009844; Wed, 3 Dec 2014 11:18:07 -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> <8761dtq2rx.fsf@br87z6lw.de.ibm.com> Date: Wed, 03 Dec 2014 19:18:07 +0100 In-Reply-To: <8761dtq2rx.fsf@br87z6lw.de.ibm.com> (Andreas Arnez's message of "Tue, 02 Dec 2014 20:18:10 +0100") Message-ID: <871togppgg.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: 14120318-0041-0000-0000-00000259FBDA X-IsSubscribed: yes On Tue, Dec 02 2014, Andreas Arnez wrote: > On Tue, Dec 02 2014, Pedro Alves wrote: > >> On 12/01/2014 06:15 PM, Andreas Arnez wrote: >> >>> >>> + 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. Hm, I noticed that this probably deserves some more explanation. While it is true that the registers are marked unavailable when initializing a new regcache, the regcache seems to survive without another initialization between calls to fetch_inferior_registers. I've verified this in my tests, and I've also not seen any code that would perform such a re-initialization. I wonder why that is the case, and whether we would like to change that. If so, the patch could avoid touching ARM code, wouldn't need special treatment of NULL in the TDB store function, and would treat ENODATA like any other error from ptrace, except that the warning would be suppressed. I think this would also improve the behavior of other errors from ptrace, but maybe there's a good reason for falling back to the "last known" register values in this case. Or maybe there's a performance reason for avoiding the re-initialization? For illustration, why don't we do something like the (untested) patch below? diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c index 718ae8c..b0f6a22 100644 --- a/gdb/gdbserver/regcache.c +++ b/gdb/gdbserver/regcache.c @@ -52,6 +52,8 @@ get_thread_regcache (struct thread_info *thread, int fetch) struct thread_info *saved_thread = current_thread; current_thread = thread; + memset (regcache->register_status, REG_UNAVAILABLE, + regcache->tdesc->num_registers); fetch_inferior_registers (regcache, -1); current_thread = saved_thread; regcache->registers_valid = 1;