From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id DFE8C387700A for ; Tue, 17 Mar 2020 19:09:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DFE8C387700A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6BAB81E5F8; Tue, 17 Mar 2020 15:09:52 -0400 (EDT) Subject: Re: [PATCH v5] Add support for NetBSD threads in sparc-nat.c From: Simon Marchi To: Kamil Rytarowski , gdb-patches@sourceware.org References: <20200317164603.10840-1-n54@gmx.com> <20200317181826.6947-1-n54@gmx.com> <04a71dc0-61e2-ba23-21ce-2b3a56016194@simark.ca> Message-ID: Date: Tue, 17 Mar 2020 15:09:51 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <04a71dc0-61e2-ba23-21ce-2b3a56016194@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-25.0 required=5.0 tests=GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Mar 2020 19:09:54 -0000 On 2020-03-17 3:07 p.m., Simon Marchi wrote: > On 2020-03-17 2:18 p.m., Kamil Rytarowski wrote: >> NetBSD ptrace(2) accepts thread id (LWP) as the 4th argument for threads. >> >> Define gdb_ptrace() a wrapper function for ptrace(2) that properly passes >> the pid,lwp pair on NetBSD and the result of get_ptrace_pid() for others. >> --- >> gdb/sparc-nat.c | 50 ++++++++++++++++++++++--------------------------- >> 1 file changed, 22 insertions(+), 28 deletions(-) >> >> diff --git a/gdb/sparc-nat.c b/gdb/sparc-nat.c >> index dff0f521565..fadcfd34474 100644 >> --- a/gdb/sparc-nat.c >> +++ b/gdb/sparc-nat.c >> @@ -78,6 +78,19 @@ typedef struct fp_status fpregset_t; >> #define PTRACE_SETFPREGS PT_SETFPREGS >> #endif >> >> +static int >> +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr) >> +{ >> +#ifdef __NetBSD__ >> + /* Support for NetBSD threads: unlike other ptrace implementations in this >> + file, NetBSD requires that we pass both the pid and lwp. */ >> + return ptrace (request, ptid.pid (), addr, ptid.lwp ()); >> +#else >> + pid_t pid = get_ptrace_pid (ptid); >> + return ptrace (request, pid, addr, 0); >> +#endif >> +} >> + >> /* Register set description. */ >> const struct sparc_gregmap *sparc_gregmap; >> const struct sparc_fpregmap *sparc_fpregmap; >> @@ -137,22 +150,7 @@ void >> sparc_fetch_inferior_registers (struct regcache *regcache, int regnum) >> { >> struct gdbarch *gdbarch = regcache->arch (); >> - pid_t pid; >> - >> - /* NOTE: cagney/2002-12-03: This code assumes that the currently >> - selected light weight processes' registers can be written >> - directly into the selected thread's register cache. This works >> - fine when given an 1:1 LWP:thread model (such as found on >> - GNU/Linux) but will, likely, have problems when used on an N:1 >> - (userland threads) or N:M (userland multiple LWP) model. In the >> - case of the latter two, the LWP's registers do not necessarily >> - belong to the selected thread (the LWP could be in the middle of >> - executing the thread switch code). >> - >> - These functions should instead be parameterized with an explicit >> - object (struct regcache, struct thread_info?) into which the LWPs >> - registers can be written. */ >> - pid = get_ptrace_pid (regcache->ptid ()); >> + ptid_t ptid = regcache->ptid (); >> >> if (regnum == SPARC_G0_REGNUM) >> { >> @@ -166,7 +164,7 @@ sparc_fetch_inferior_registers (struct regcache *regcache, int regnum) >> { >> gregset_t regs; >> >> - if (ptrace (PTRACE_GETREGS, pid, (PTRACE_TYPE_ARG3) ®s, 0) == -1) >> + if (gdb_ptrace (PTRACE_GETREGS, ptid, (PTRACE_TYPE_ARG3) ®s) == -1) >> perror_with_name (_("Couldn't get registers")); >> >> sparc_supply_gregset (sparc_gregmap, regcache, -1, ®s); >> @@ -178,7 +176,7 @@ sparc_fetch_inferior_registers (struct regcache *regcache, int regnum) >> { >> fpregset_t fpregs; >> >> - if (ptrace (PTRACE_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1) >> + if (gdb_ptrace (PTRACE_GETFPREGS, ptid, (PTRACE_TYPE_ARG3) &fpregs) == -1) >> perror_with_name (_("Couldn't get floating point status")); >> >> sparc_supply_fpregset (sparc_fpregmap, regcache, -1, &fpregs); >> @@ -189,22 +187,18 @@ void >> sparc_store_inferior_registers (struct regcache *regcache, int regnum) >> { >> struct gdbarch *gdbarch = regcache->arch (); >> - pid_t pid; >> - >> - /* NOTE: cagney/2002-12-02: See comment in fetch_inferior_registers >> - about threaded assumptions. */ >> - pid = get_ptrace_pid (regcache->ptid ()); >> + ptid_t ptid = regcache->ptid (); >> >> if (regnum == -1 || sparc_gregset_supplies_p (gdbarch, regnum)) >> { >> gregset_t regs; >> >> - if (ptrace (PTRACE_GETREGS, pid, (PTRACE_TYPE_ARG3) ®s, 0) == -1) >> + if (gdb_ptrace (PTRACE_GETREGS, ptid, (PTRACE_TYPE_ARG3) ®s) == -1) >> perror_with_name (_("Couldn't get registers")); >> >> sparc_collect_gregset (sparc_gregmap, regcache, regnum, ®s); >> >> - if (ptrace (PTRACE_SETREGS, pid, (PTRACE_TYPE_ARG3) ®s, 0) == -1) >> + if (gdb_ptrace (PTRACE_SETREGS, ptid, (PTRACE_TYPE_ARG3) ®s) == -1) >> perror_with_name (_("Couldn't write registers")); >> >> /* Deal with the stack regs. */ >> @@ -225,7 +219,7 @@ sparc_store_inferior_registers (struct regcache *regcache, int regnum) >> { >> fpregset_t fpregs, saved_fpregs; >> >> - if (ptrace (PTRACE_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1) >> + if (gdb_ptrace (PTRACE_GETFPREGS, ptid, (PTRACE_TYPE_ARG3) &fpregs) == -1) >> perror_with_name (_("Couldn't get floating-point registers")); >> >> memcpy (&saved_fpregs, &fpregs, sizeof (fpregs)); >> @@ -237,8 +231,8 @@ sparc_store_inferior_registers (struct regcache *regcache, int regnum) >> to write the registers if nothing changed. */ >> if (memcmp (&saved_fpregs, &fpregs, sizeof (fpregs)) != 0) >> { >> - if (ptrace (PTRACE_SETFPREGS, pid, >> - (PTRACE_TYPE_ARG3) &fpregs, 0) == -1) >> + if (gdb_ptrace (PTRACE_SETFPREGS, ptid, >> + (PTRACE_TYPE_ARG3) &fpregs) == -1) >> perror_with_name (_("Couldn't write floating-point registers")); >> } >> >> -- >> 2.25.0 >> > > Thanks, this version LGTM. > > Simon Don't forget the ChangeLog entry though. Simon