[PATCH 1/2] gdb: linux-nat: make linux_nat_filter_event return void

[PATCH 1/2] gdb: linux-nat: make linux_nat_filter_event return void

[Simon Marchi]

I noticed that linux_nat_filter_event returns a value, but its caller
doesn't use it.  This has been since 9c02b52532ac ("linux-nat.c: better
starvation avoidance, handle non-stop mode too").  Before that commit,
the return value was used to tell the caller whether to continue
processing that event or not.  But since then, the model is that we pull
all events from the kernel and linux_nat_filter_event just saves the
status to the lwp_info structure if it thinks it's relevant.  And the
caller, linux_nat_wait_1, selects a status at random amongst the threads
with a pending status.  So essentially, the return value of
linux_nat_filter_event does not have a reason to be anymore.  Change it
so it returns void.

gdb/ChangeLog:

	* linux-nat.c (linux_nat_filter_event): Return void.

Change-Id: I35662868910f5122772ed92a512adfbf4da12d87
---
 gdb/linux-nat.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 3c7117bff6e6..ac24b9bdde9d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2817,9 +2817,10 @@ resumed_callback (struct lwp_info *lp)
 }
 
 /* Check if we should go on and pass this event to common code.
-   Return the affected lwp if we should, or NULL otherwise.  */
 
-static struct lwp_info *
+   If so, save the status to the lwp_info structure associated to LWPID.  */
+
+static void
 linux_nat_filter_event (int lwpid, int status)
 {
   struct lwp_info *lp;
@@ -2857,7 +2858,7 @@ linux_nat_filter_event (int lwpid, int status)
       linux_nat_debug_printf ("saving LWP %ld status %s in stopped_pids list",
 			      (long) lwpid, status_to_str (status));
       add_to_pid_list (&stopped_pids, lwpid, status);
-      return NULL;
+      return;
     }
 
   /* Make sure we don't report an event for the exit of an LWP not in
@@ -2865,7 +2866,7 @@ linux_nat_filter_event (int lwpid, int status)
      if we detach from a program we originally forked and then it
      exits.  */
   if (!WIFSTOPPED (status) && !lp)
-    return NULL;
+    return;
 
   /* This LWP is stopped now.  (And if dead, this prevents it from
      ever being continued.)  */
@@ -2889,7 +2890,7 @@ linux_nat_filter_event (int lwpid, int status)
 	 on.  */
       status = W_STOPCODE (SIGTRAP);
       if (linux_handle_syscall_trap (lp, 0))
-	return NULL;
+	return;
     }
   else
     {
@@ -2905,7 +2906,7 @@ linux_nat_filter_event (int lwpid, int status)
       linux_nat_debug_printf ("Handling extended status 0x%06x", status);
 
       if (linux_handle_extended_wait (lp, status))
-	return NULL;
+	return;
     }
 
   /* Check if the thread has exited.  */
@@ -2921,7 +2922,7 @@ linux_nat_filter_event (int lwpid, int status)
 	     was not the end of the debugged application and should be
 	     ignored.  */
 	  exit_lwp (lp);
-	  return NULL;
+	  return;
 	}
 
       /* Note that even if the leader was ptrace-stopped, it can still
@@ -2937,7 +2938,7 @@ linux_nat_filter_event (int lwpid, int status)
       /* Store the pending event in the waitstatus, because
 	 W_EXITCODE(0,0) == 0.  */
       store_waitstatus (&lp->waitstatus, status);
-      return lp;
+      return;
     }
 
   /* Make sure we don't report a SIGSTOP that we sent ourselves in
@@ -2963,7 +2964,7 @@ linux_nat_filter_event (int lwpid, int status)
 
 	  linux_resume_one_lwp (lp, lp->step, GDB_SIGNAL_0);
 	  gdb_assert (lp->resumed);
-	  return NULL;
+	  return;
 	}
     }
 
@@ -2985,7 +2986,7 @@ linux_nat_filter_event (int lwpid, int status)
       gdb_assert (lp->resumed);
 
       /* Discard the event.  */
-      return NULL;
+      return;
     }
 
   /* Don't report signals that GDB isn't interested in, such as
@@ -3034,7 +3035,7 @@ linux_nat_filter_event (int lwpid, int status)
 	     target_pid_to_str (lp->ptid).c_str (),
 	     (signo != GDB_SIGNAL_0
 	      ? strsignal (gdb_signal_to_host (signo)) : "0"));
-	  return NULL;
+	  return;
 	}
     }
 
@@ -3042,7 +3043,6 @@ linux_nat_filter_event (int lwpid, int status)
   gdb_assert (lp);
   lp->status = status;
   save_stop_reason (lp);
-  return lp;
 }
 
 /* Detect zombie thread group leaders, and "exit" them.  We can't reap
-- 
2.30.0

[PATCH 2/2] gdbserver: linux-low: make linux_process_target::filter_event return void

[Simon Marchi]

Same as the previous patch, but for GDBserver.  The return value of this
method is never used, change it to return void.

gdbserver/ChangeLog:

	* linux-low.cc (linux_process_target::filter_event): Return
	void.
	* linux-low.h (class linux_process_target) <filter_event>:
	Return void.

Change-Id: I79e5dc04d9b21b9f01c6d675fa463d1b1a703b3a
---
 gdbserver/linux-low.cc | 20 ++++++++++----------
 gdbserver/linux-low.h  |  8 ++++----
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index d167914d63ee..d5c9d85c3d53 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -2243,7 +2243,7 @@ linux_low_ptrace_options (int attached)
   return options;
 }
 
-lwp_info *
+void
 linux_process_target::filter_event (int lwpid, int wstat)
 {
   client_state &cs = get_client_state ();
@@ -2292,10 +2292,10 @@ linux_process_target::filter_event (int lwpid, int wstat)
   if (child == NULL && WIFSTOPPED (wstat))
     {
       add_to_pid_list (&stopped_pids, lwpid, wstat);
-      return NULL;
+      return;
     }
   else if (child == NULL)
-    return NULL;
+    return;
 
   thread = get_lwp_thread (child);
 
@@ -2325,12 +2325,12 @@ linux_process_target::filter_event (int lwpid, int wstat)
 	     report this one right now.  Leave the status pending for
 	     the next time we're able to report it.  */
 	  mark_lwp_dead (child, wstat);
-	  return child;
+	  return;
 	}
       else
 	{
 	  delete_lwp (child);
-	  return NULL;
+	  return;
 	}
     }
 
@@ -2358,7 +2358,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
 		 the first instruction.  */
 	      child->status_pending_p = 1;
 	      child->status_pending = wstat;
-	      return child;
+	      return;
 	    }
 	}
     }
@@ -2397,7 +2397,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
 	{
 	  /* The event has been handled, so just return without
 	     reporting it.  */
-	  return NULL;
+	  return;
 	}
     }
 
@@ -2433,7 +2433,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
 	    debug_printf ("LLW: SIGSTOP caught for %s "
 			  "while stopping threads.\n",
 			  target_pid_to_str (ptid_of (thread)));
-	  return NULL;
+	  return;
 	}
       else
 	{
@@ -2444,13 +2444,13 @@ linux_process_target::filter_event (int lwpid, int wstat)
 			  target_pid_to_str (ptid_of (thread)));
 
 	  resume_one_lwp (child, child->stepping, 0, NULL);
-	  return NULL;
+	  return;
 	}
     }
 
   child->status_pending_p = 1;
   child->status_pending = wstat;
-  return child;
+  return;
 }
 
 bool
diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
index 0234df533582..be97526ced7b 100644
--- a/gdbserver/linux-low.h
+++ b/gdbserver/linux-low.h
@@ -326,10 +326,10 @@ class linux_process_target : public process_stratum_target
      to a new LWP representing the new program.  */
   int handle_extended_wait (lwp_info **orig_event_lwp, int wstat);
 
-  /* Do low-level handling of the event, and check if we should go on
-     and pass it to caller code.  Return the affected lwp if we are, or
-     NULL otherwise.  */
-  lwp_info *filter_event (int lwpid, int wstat);
+  /* Do low-level handling of the event, and check if this is an event we want
+     to report.  Is so, store it as a pending status in the lwp_info structure
+     corresponding to LWPID.  */
+  void filter_event (int lwpid, int wstat);
 
   /* Wait for an event from child(ren) WAIT_PTID, and return any that
      match FILTER_PTID (leaving others pending).  The PTIDs can be:
-- 
2.30.0

Re: [PATCH 2/2] gdbserver: linux-low: make linux_process_target::filter_event return void

[Luis Machado]

On 2/1/21 4:36 PM, Simon Marchi via Gdb-patches wrote:
> Same as the previous patch, but for GDBserver.  The return value of this
> method is never used, change it to return void.
> 
> gdbserver/ChangeLog:
> 
> 	* linux-low.cc (linux_process_target::filter_event): Return
> 	void.
> 	* linux-low.h (class linux_process_target) <filter_event>:
> 	Return void.
> 
> Change-Id: I79e5dc04d9b21b9f01c6d675fa463d1b1a703b3a
> ---
>   gdbserver/linux-low.cc | 20 ++++++++++----------
>   gdbserver/linux-low.h  |  8 ++++----
>   2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index d167914d63ee..d5c9d85c3d53 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -2243,7 +2243,7 @@ linux_low_ptrace_options (int attached)
>     return options;
>   }
>   
> -lwp_info *
> +void
>   linux_process_target::filter_event (int lwpid, int wstat)
>   {
>     client_state &cs = get_client_state ();
> @@ -2292,10 +2292,10 @@ linux_process_target::filter_event (int lwpid, int wstat)
>     if (child == NULL && WIFSTOPPED (wstat))
>       {
>         add_to_pid_list (&stopped_pids, lwpid, wstat);
> -      return NULL;
> +      return;
>       }
>     else if (child == NULL)
> -    return NULL;
> +    return;
>   
>     thread = get_lwp_thread (child);
>   
> @@ -2325,12 +2325,12 @@ linux_process_target::filter_event (int lwpid, int wstat)
>   	     report this one right now.  Leave the status pending for
>   	     the next time we're able to report it.  */
>   	  mark_lwp_dead (child, wstat);
> -	  return child;
> +	  return;
>   	}
>         else
>   	{
>   	  delete_lwp (child);
> -	  return NULL;
> +	  return;
>   	}
>       }
>   
> @@ -2358,7 +2358,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
>   		 the first instruction.  */
>   	      child->status_pending_p = 1;
>   	      child->status_pending = wstat;
> -	      return child;
> +	      return;
>   	    }
>   	}
>       }
> @@ -2397,7 +2397,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
>   	{
>   	  /* The event has been handled, so just return without
>   	     reporting it.  */
> -	  return NULL;
> +	  return;
>   	}
>       }
>   
> @@ -2433,7 +2433,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
>   	    debug_printf ("LLW: SIGSTOP caught for %s "
>   			  "while stopping threads.\n",
>   			  target_pid_to_str (ptid_of (thread)));
> -	  return NULL;
> +	  return;
>   	}
>         else
>   	{
> @@ -2444,13 +2444,13 @@ linux_process_target::filter_event (int lwpid, int wstat)
>   			  target_pid_to_str (ptid_of (thread)));
>   
>   	  resume_one_lwp (child, child->stepping, 0, NULL);
> -	  return NULL;
> +	  return;
>   	}
>       }
>   
>     child->status_pending_p = 1;
>     child->status_pending = wstat;
> -  return child;
> +  return;
>   }
>   
>   bool
> diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
> index 0234df533582..be97526ced7b 100644
> --- a/gdbserver/linux-low.h
> +++ b/gdbserver/linux-low.h
> @@ -326,10 +326,10 @@ class linux_process_target : public process_stratum_target
>        to a new LWP representing the new program.  */
>     int handle_extended_wait (lwp_info **orig_event_lwp, int wstat);
>   
> -  /* Do low-level handling of the event, and check if we should go on
> -     and pass it to caller code.  Return the affected lwp if we are, or
> -     NULL otherwise.  */
> -  lwp_info *filter_event (int lwpid, int wstat);
> +  /* Do low-level handling of the event, and check if this is an event we want
> +     to report.  Is so, store it as a pending status in the lwp_info structure
> +     corresponding to LWPID.  */
> +  void filter_event (int lwpid, int wstat);
>   
>     /* Wait for an event from child(ren) WAIT_PTID, and return any that
>        match FILTER_PTID (leaving others pending).  The PTIDs can be:
> 

Both patches look OK to me.

Re: [PATCH 2/2] gdbserver: linux-low: make linux_process_target::filter_event return void

[Simon Marchi]

On 2021-02-01 2:48 p.m., Luis Machado wrote:
> Both patches look OK to me.

Thanks, I pushed both.

Simon