Re: [PATCH 02/31] linux-nat: introduce pending_status_str

[Pedro Alves <pedro@palves.net>]

On 2023-02-03 12:00 p.m., Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 
>> I noticed that some debug log output printing an lwp's pending status
>> wasn't considering lp->waitstatus.  This fixes it, by introducing a
>> new pending_status_str function.
> 
> This patch looks fine.  I had one slightly related question:  I took a
> look at the comment on lwp_info::waitstatus in linux-nat.h, which says:
> 
>   /* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus
>      for this LWP's last event.  This may correspond to STATUS above,
>      or to a local variable in lin_lwp_wait.  */
>   struct target_waitstatus waitstatus;
> 
> Am I right in thinking that this comment is wrong; it should say
> TARGET_WAITKIND_IGNORE, not TARGET_WAITKIND_SPURIOUS, right?

You're right.

I tweaked the comments in linux-nat.h in this new version.  Let me know what
you think.

From 10f88baff2e25fb87f37d1665bf283206171dd42 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 12 Nov 2021 20:50:29 +0000
Subject: [PATCH] linux-nat: introduce pending_status_str

I noticed that some debug log output printing an lwp's pending status
wasn't considering lp->waitstatus.  This fixes it, by introducing a
new pending_status_str function.

Also fix the comment in gdb/linux-nat.h describing
lwp_info::waitstatus and details the description of lwp_info::status
while at it.

Change-Id: I66e5c7a363d30a925b093b195d72925ce5b6b980
---
 gdb/linux-nat.c | 19 ++++++++++++++++---
 gdb/linux-nat.h | 11 +++++++----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index e5391b9ce35..9d811bbf3ff 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -255,6 +255,19 @@ is_leader (lwp_info *lp)
   return lp->ptid.pid () == lp->ptid.lwp ();
 }
 
+/* Convert an LWP's pending status to a std::string.  */
+
+static std::string
+pending_status_str (lwp_info *lp)
+{
+  gdb_assert (lwp_status_pending_p (lp));
+
+  if (lp->waitstatus.kind () != TARGET_WAITKIND_IGNORE)
+    return lp->waitstatus.to_string ();
+  else
+    return status_to_str (lp->status);
+}
+
 \f
 /* LWP accessors.  */
 
@@ -1647,8 +1660,8 @@ linux_nat_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
 	 this thread with a signal?  */
       gdb_assert (signo == GDB_SIGNAL_0);
 
-      linux_nat_debug_printf ("Short circuiting for status 0x%x",
-			      lp->status);
+      linux_nat_debug_printf ("Short circuiting for status %s",
+			      pending_status_str (lp).c_str ());
 
       if (target_can_async_p ())
 	{
@@ -3137,7 +3150,7 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
   if (lp != NULL)
     {
       linux_nat_debug_printf ("Using pending wait status %s for %s.",
-			      status_to_str (lp->status).c_str (),
+			      pending_status_str (lp).c_str (),
 			      lp->ptid.to_string ().c_str ());
     }
 
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 45534c92386..770fe924427 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -232,7 +232,9 @@ struct lwp_info : intrusive_list_node<lwp_info>
   /* The last resume GDB requested on this thread.  */
   resume_kind last_resume_kind = resume_continue;
 
-  /* If non-zero, a pending wait status.  */
+  /* If non-zero, a pending wait status.  A pending process exit is
+     recorded in WAITSTATUS, because W_EXITCODE(0,0) happens to be
+     0.  */
   int status = 0;
 
   /* When 'stopped' is set, this is where the lwp last stopped, with
@@ -260,9 +262,10 @@ struct lwp_info : intrusive_list_node<lwp_info>
   /* Non-zero if we expect a duplicated SIGINT.  */
   int ignore_sigint = 0;
 
-  /* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus
-     for this LWP's last event.  This may correspond to STATUS above,
-     or to a local variable in lin_lwp_wait.  */
+  /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+     this LWP's last event.  This usually corresponds to STATUS above,
+     however because W_EXITCODE(0,0) happens to be 0, a process exit
+     will be recorded here, while 'status == 0' is ambiguous.  */
   struct target_waitstatus waitstatus;
 
   /* Signal whether we are in a SYSCALL_ENTRY or

base-commit: 2562954ede66f32bff7d985e752b8052c2ae5775
prerequisite-patch-id: bbc9918ac5f79de07a29f34ec072794d270f942d
-- 
2.36.0