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 3F5093945C16 for ; Wed, 18 Mar 2020 22:21:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3F5093945C16 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) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id A622C1E5F8; Wed, 18 Mar 2020 18:21:06 -0400 (EDT) Subject: Re: [PATCH v2] Disable get_ptrace_pid for NetBSD To: Kamil Rytarowski , gdb-patches@sourceware.org Cc: tom@tromey.com References: <20200318214934.24306-1-n54@gmx.com> <20200318215531.25248-1-n54@gmx.com> From: Simon Marchi Message-ID: <1d5be028-c95c-a04c-a6b5-67984dae17f3@simark.ca> Date: Wed, 18 Mar 2020 18:21:05 -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: <20200318215531.25248-1-n54@gmx.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=0.0 required=5.0 tests=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: Wed, 18 Mar 2020 22:21:08 -0000 On 2020-03-18 5:55 p.m., Kamil Rytarowski wrote: > @@ -313,8 +325,12 @@ inf_ptrace_target::kill () > target_mourn_inferior (inferior_ptid); > } > > +#ifndef __NetBSD__ > /* Return which PID to pass to ptrace in order to observe/control the > - tracee identified by PTID. */ > + tracee identified by PTID. > + > + Unlike most other Operating Systems, NetBSD tracks both pid and lwp > + and avoids this function. */ To align this function with our current way of documenting functions, please move this comment to the declaration of get_ptrace_pid, in inf-ptrace.h, and put this here: /* See inf-ptrace.h. */ > > pid_t > get_ptrace_pid (ptid_t ptid) > @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) > pid = ptid.pid (); > return pid; > } > +#endif > > /* Resume execution of thread PTID, or all threads if PTID is -1. If > STEP is nonzero, single-step it. If SIGNAL is nonzero, give it > @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) > void > inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > { > - pid_t pid; > int request; > > if (minus_one_ptid == ptid) > /* Resume all threads. Traditionally ptrace() only supports > single-threaded processes, so simply resume the inferior. */ > - pid = inferior_ptid.pid (); > - else > - pid = get_ptrace_pid (ptid); > + ptid = inferior_ptid; I'm not certain about this part (which I suggested). With the existing code, if inferior_ptid is {pid=10, lwp=20}, we'll call ptrace with pid=10. With the patch, we'll call ptrace with pid=20. I'm not entirely sure if/when resume can be called with ptid == minus_one_ptid. And if it's called with minus_one_ptid, is it possible that ptid.pid != ptid.lwp? In any case, it's probably safer to do: ptid = ptid_t (inferior_ptid.pid ()); to retain the existing behavior. WDYT? > > if (catch_syscall_enabled () > 0) > request = PT_SYSCALL; > @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > where it was. If GDB wanted it to start some other way, we have > already written a new program counter value to the child. */ > errno = 0; > - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > if (errno != 0) > perror_with_name (("ptrace")); > } > @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > const gdb_byte *writebuf, > ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) > { > - pid_t pid = get_ptrace_pid (inferior_ptid); > + ptid_t ptid = inferior_ptid; > > switch (object) > { > @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > piod.piod_len = len; > > errno = 0; > - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) > + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) > { > /* Return the actual number of bytes read or written. */ > *xfered_len = piod.piod_len; > @@ -565,7 +579,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > return TARGET_XFER_EOF; > } > #endif > - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, > + *xfered_len = inf_ptrace_peek_poke (ptid.pid (), readbuf, writebuf, > offset, len); > return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; I'm pretty sure that this change is not right. For non-NetBSD targets, this used to pass `inferior_ptid.lwp` (the result of get_ptrace_pid). Now, we pass `inferior_ptid.pid`. I think you'll need to pass the ptid to inf_ptrace_peek_poke and use gdb_ptrace inside that function too. Simon