From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62524 invoked by alias); 28 May 2015 18:50:54 -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 62514 invoked by uid 89); 28 May 2015 18:50:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-oi0-f49.google.com Received: from mail-oi0-f49.google.com (HELO mail-oi0-f49.google.com) (209.85.218.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 28 May 2015 18:50:52 +0000 Received: by oifu123 with SMTP id u123so39610883oif.1 for ; Thu, 28 May 2015 11:50:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=O3//O9gOHwHhjltYQ4Cx0JoUdZH+13EgDAVCLSkFR6A=; b=CLJ/upFJmG2/Naxn6ZNSyHtgAQSc/18+DsuZbDVTOHyIPyRoDy4nVgw3Ni4qQZCFl1 lOqRg0Tc1kpgrupU8Iu7M+qQFsSJTH5hsOYQg3aYUnxy+YdtPjkHwmezY2mC0JwmQip5 VBPAgRZKEcKDMnb5Oik3Oc8+1UkVzbaMcXUwYTqqcgiT0f8doqSb3yWjvRZbIJg+eAdD 8W0L5Ol+tgEmvZ1ETMfl8YDsK7YE/Q2SmWAXl6Fco1CNwyhMl9pJtgBweiF1OD9VHloR zTnqeZJQeJWs72/ILxnOIsMadlsU/w3eSf/JbOXEnSmMYE3Eniv3uz/F9W45Vdoq5y/r FYUA== X-Gm-Message-State: ALoCoQkftCCtMreQlf8eMWaNoU38PRc0CSFb6l06ZI9vGJkI4gCEQuZWKYz095KXpu0TRmsjvZdN MIME-Version: 1.0 X-Received: by 10.202.198.151 with SMTP id w145mr3601156oif.72.1432839050331; Thu, 28 May 2015 11:50:50 -0700 (PDT) Received: by 10.182.89.99 with HTTP; Thu, 28 May 2015 11:50:50 -0700 (PDT) In-Reply-To: <1432822816-32327-5-git-send-email-yao.qi@linaro.org> References: <1432822816-32327-1-git-send-email-yao.qi@linaro.org> <1432822816-32327-5-git-send-email-yao.qi@linaro.org> Date: Thu, 28 May 2015 18:50:00 -0000 Message-ID: Subject: Re: [PATCH 4/6] Fetch and store GP registers by PTRACE_{G,S}ETREGSET From: Doug Evans To: Yao Qi Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg00692.txt.bz2 On Thu, May 28, 2015 at 7:20 AM, Yao Qi wrote: > If kernel supports PTRACE_GETREGSET, GDB uses PTRACE_{G,S}ETREGSET > to fetch and store GP registers. > > gdb: > > 2015-05-28 Yao Qi > > * arm-linux-nat.c (fetch_register): Use PTRACE_GETREGSET. > (fetch_regs): Likewise. > (store_regs): Use PTRACE_SETREGSET. > (store_register): Likewise. > --- > gdb/arm-linux-nat.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 72 insertions(+), 7 deletions(-) > > diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c > index 877559e..0a86ed6 100644 > --- a/gdb/arm-linux-nat.c > +++ b/gdb/arm-linux-nat.c > @@ -225,7 +225,18 @@ fetch_register (struct regcache *regcache, int regno) > /* Get the thread id for the ptrace call. */ > tid = GET_THREAD_ID (inferior_ptid); > > - ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + if (have_ptrace_getregset == 1) Hi. The == 1 in this test hinders readability (to me anyway). [It occurs here and in 5/6, 6/6.] The name suggests the variable is a boolean, so I'm left wondering "Can it have values other than 0/1, and is the else clause correct for those other values?" Digging deeper the reader would find the variable is tri-state, but the initial -1 value should never be seen here (at least that's the intuitive choice). If one wanted to add an assert that the value is not -1 here that'd be ok, though one could also argue it's overkill. I don't have a preference either way. But I suggest removing the "== 1" in the test. > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + > if (ret < 0) > { > warning (_("Unable to fetch general register.")); > @@ -266,8 +277,19 @@ fetch_regs (struct regcache *regcache) > > /* Get the thread id for the ptrace call. */ > tid = GET_THREAD_ID (inferior_ptid); > - > - ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + > + if (have_ptrace_getregset == 1) > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + > if (ret < 0) > { > warning (_("Unable to fetch general registers.")); > @@ -306,7 +328,18 @@ store_register (const struct regcache *regcache, int regno) > tid = GET_THREAD_ID (inferior_ptid); > > /* Get the general registers from the process. */ > - ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + if (have_ptrace_getregset == 1) > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + > if (ret < 0) > { > warning (_("Unable to fetch general registers.")); > @@ -322,7 +355,18 @@ store_register (const struct regcache *regcache, int regno) > regcache_raw_collect (regcache, ARM_PC_REGNUM, > (char *) ®s[ARM_PC_REGNUM]); > > - ret = ptrace (PTRACE_SETREGS, tid, 0, ®s); > + if (have_ptrace_getregset == 1) > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_SETREGS, tid, 0, ®s); > + > if (ret < 0) > { > warning (_("Unable to store general register.")); > @@ -340,7 +384,18 @@ store_regs (const struct regcache *regcache) > tid = GET_THREAD_ID (inferior_ptid); > > /* Fetch the general registers. */ > - ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + if (have_ptrace_getregset == 1) > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_GETREGS, tid, 0, ®s); > + > if (ret < 0) > { > warning (_("Unable to fetch general registers.")); > @@ -357,7 +412,17 @@ store_regs (const struct regcache *regcache) > regcache_raw_collect (regcache, ARM_PS_REGNUM, > (char *) ®s[ARM_CPSR_GREGNUM]); > > - ret = ptrace (PTRACE_SETREGS, tid, 0, ®s); > + if (have_ptrace_getregset == 1) > + { > + struct iovec iov; > + > + iov.iov_base = ®s; > + iov.iov_len = sizeof (regs); > + > + ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov); > + } > + else > + ret = ptrace (PTRACE_SETREGS, tid, 0, ®s); > > if (ret < 0) > { > -- > 1.9.1 >