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

Pedro Alves
2023-03-10T17:15:37Z

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