From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 306CF3858C50 for ; Tue, 5 Apr 2022 12:47:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 306CF3858C50 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 235Cl9Yu012908 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 5 Apr 2022 08:47:14 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 235Cl9Yu012908 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (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 CB0201E787; Tue, 5 Apr 2022 08:47:08 -0400 (EDT) Message-ID: <87169b93-8be2-5ccd-6b58-51b395a367bd@polymtl.ca> Date: Tue, 5 Apr 2022 08:47:08 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH] Use current_inferior ()->pid for AIX Content-Language: en-US To: Aditya Vidyadhar Kamath , Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya References: <5f142468-bc68-9128-d4d6-80cf36f12a48@polymtl.ca> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 5 Apr 2022 12:47:09 +0000 X-Spam-Status: No, score=-3039.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 05 Apr 2022 12:47:19 -0000 Hi Aditya, I don't think that using current_inferior throughout is a good solution to your problems. It will be case by case, unfortunately there's no easy way out of this but to dig in GDB and understand its internals a bit. > From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001 > From: Aditya Vidyadhar Kamath > > Date: Wed, 30 Mar 2022 11:19:14 +0530 > Subject: [PATCH] Use current_inferior ()->pid for AIX. > > --- > gdb/aix-thread.c | 15 ++++++++------- > gdb/rs6000-aix-nat.c | 8 ++++---- > 2 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c > index 85be4c15f1c..9331de3beb2 100644 > --- a/gdb/aix-thread.c > +++ b/gdb/aix-thread.c > @@ -708,7 +708,7 @@ get_signaled_thread (void) > > while (1) > { > - if (getthrds (inferior_ptid.pid (), &thrinf, > + if (getthrds (current_inferior ()->pid, &thrinf, > sizeof (thrinf), &ktid, 1) != 1) This function doesn't seem to depend on the current inferior/thread, so it could easily be changed to receive the pid as a parameter. > break; > > @@ -791,7 +791,7 @@ sync_threadlists (void) > > /* Apply differences between the two arrays to GDB's thread list. */ > > - infpid = inferior_ptid.pid (); > + infpid = current_inferior ()->pid; > for (pi = gi = 0; pi < pcount || gi < gcount;) > { > if (pi == pcount) > @@ -883,11 +883,11 @@ pd_update (int set_infpid) > struct thread_info *thread = NULL; > > if (!pd_active) > - return inferior_ptid; > + return ptid_t (current_inferior ()->pid); > > status = pthdb_session_update (pd_session); > if (status != PTHDB_SUCCESS) > - return inferior_ptid; > + return ptid_t (current_inferior ()->pid); Here, the pd_update function seems to want to return the event ptid unmodified. So, perhaps that ptid could be passed by the caller and returned here. Or, pd_update could be changed to return an optional and the caller deals with returning the right ptid. > > sync_threadlists (); > > @@ -897,7 +897,7 @@ pd_update (int set_infpid) > if (tid != 0) > thread = iterate_over_threads (iter_tid, &tid); > if (!thread) > - ptid = inferior_ptid; > + ptid = ptid_t (current_inferior ()->pid); > else > { > ptid = thread->ptid; > @@ -921,7 +921,7 @@ pd_activate (int set_infpid) > &pd_session); > if (status != PTHDB_SUCCESS) > { > - return inferior_ptid; > + return ptid_t (current_inferior ()->pid); This is the same idea as in pd_update. > } > pd_active = 1; > return pd_update (set_infpid); > @@ -932,11 +932,12 @@ pd_activate (int set_infpid) > static void > pd_deactivate (void) > { > + ptid_t ptdrtn = ptid_t (current_inferior ()->pid) > if (!pd_active) > return; > pthdb_session_destroy (pd_session); > > - pid_to_prc (&inferior_ptid); > + pid_to_prc (&ptdrtn); Since pid_to_prc only writes to ptdrtn, it essentially a no-op. I would try just removing that call. It was used to modify inferior_ptid after the thread layer was disabled, which does not seem useful to me. > diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c > index 8563aea313a..0b8e4d2c687 100644 > --- a/gdb/rs6000-aix-nat.c > +++ b/gdb/rs6000-aix-nat.c > @@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object, > ULONGEST offset, ULONGEST len, > ULONGEST *xfered_len) > { > - pid_t pid = inferior_ptid.pid (); > + pid_t pid = current_inferior ()->pid; > int arch64 = ARCH64 (); > > switch (object) Here, inferior_ptid should be valid, this change should not be necessary. > @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, > > /* Claim it exited with unknown signal. */ > ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN); > - return inferior_ptid; > + return ptid_t (current_inferior ()->pid); > } > > /* Ignore terminated detached child processes. */ > - if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ()) > + if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid)) > pid = -1; > } > while (pid == -1); The target_ops::wait method implemenattions should ne rely on the current inferior either. I don't think that this target supports debugging multiple processes at the same time, but imagine that it does, eventually. When rs6000_nat_target::wait gets called, the current inferior is one of the inferiors managed by that target, at random. So what the wait method needs to do is fetch an event from the OS, and returning, regardless of which inferior it applies to (except if a ptid argument is given). So, you should never need to use either inferior_ptid nor current_inferior in wait. > @@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries > if (writebuf) > return TARGET_XFER_E_IO; > > - gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid); > + gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid)); > result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (), > readbuf, offset, len, 1); This is used by xfer_partial, where inferior_ptid should be valid, so this change shouldn't be necessary. Simon