Patchwork [2/2] target_set_syscall_catchpoint, use gdb::array_view and bool

login
register
mail settings
Submitter Simon Marchi
Date Dec. 3, 2017, 6:18 p.m.
Message ID <19e8ceb2-8c71-9d94-eeff-324200f2582b@simark.ca>
Download mbox | patch
Permalink /patch/24700/
State New
Headers show

Comments

Simon Marchi - Dec. 3, 2017, 6:18 p.m.
On 2017-10-30 11:59 AM, John Baldwin wrote:
> On 10/30/17 3:32 PM, Pedro Alves wrote:
>> I noticed that we're passing down a data/size pair to
>> target_ops::to_set_syscall_catchpoint.  This commit makes use of
>> gdb::array_view instead.  While at it, use bool where appropriate as
>> well.
>>
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* break-catch-syscall.c (insert_catch_syscall)
>> 	(remove_catch_syscall): Adjust to pass reference to
>> 	inf_data->syscalls_counts directly via gdb::array_view.
>> 	* linux-nat.c (linux_child_set_syscall_catchpoint): Adjust to use
>> 	bool and gdb::array_view.
> 
> I believe fbsd-nat.c will need a similar fixup?  It doesn't use the
> values passed but does implement the target method.

Hi John,

Can you check if this updated patch builds properly on FreeBSD?

Thanks!

Simon


From 15a43589c08614d9286f35a4b58368805c4401bb Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 30 Oct 2017 15:32:11 +0000
Subject: [PATCH] target_set_syscall_catchpoint, use gdb::array_view and bool

I noticed that we're passing down a data/size pair to
target_ops::to_set_syscall_catchpoint.  This commit makes use of
gdb::array_view instead.  While at it, use bool where appropriate as
well.

gdb/ChangeLog:

	* break-catch-syscall.c (insert_catch_syscall)
	(remove_catch_syscall): Adjust to pass reference to
	inf_data->syscalls_counts directly via gdb::array_view.
	* fbsd-nat.c (fbsd_set_syscall_catchpoint): Adjust to use bool
	and gdb::array_view.
	* linux-nat.c (linux_child_set_syscall_catchpoint): Likewise.
	* remote.c (remote_set_syscall_catchpoint): Likewise.
	* target-debug.h (target_debug_print_bool): New.
	(define target_debug_print_gdb_array_view_const_int): New.
	* target-delegates.c: Regenerate.
	* target.h (target_ops) <to_set_syscall_catchpoint>: Use
	gdb::array_view and bool.
	(target_set_syscall_catchpoint): Likewise.
---
 gdb/break-catch-syscall.c |  6 ++----
 gdb/fbsd-nat.c            |  5 +++--
 gdb/linux-nat.c           |  6 +++---
 gdb/remote.c              | 24 +++++++++++-------------
 gdb/target-debug.h        |  4 ++++
 gdb/target-delegates.c    | 16 +++++++---------
 gdb/target.h              | 23 ++++++++++-------------
 7 files changed, 40 insertions(+), 44 deletions(-)
John Baldwin - Dec. 6, 2017, 10:38 p.m.
On 12/3/17 10:18 AM, Simon Marchi wrote:
> On 2017-10-30 11:59 AM, John Baldwin wrote:
>> On 10/30/17 3:32 PM, Pedro Alves wrote:
>>> I noticed that we're passing down a data/size pair to
>>> target_ops::to_set_syscall_catchpoint.  This commit makes use of
>>> gdb::array_view instead.  While at it, use bool where appropriate as
>>> well.
>>>
>>> gdb/ChangeLog:
>>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>>
>>> 	* break-catch-syscall.c (insert_catch_syscall)
>>> 	(remove_catch_syscall): Adjust to pass reference to
>>> 	inf_data->syscalls_counts directly via gdb::array_view.
>>> 	* linux-nat.c (linux_child_set_syscall_catchpoint): Adjust to use
>>> 	bool and gdb::array_view.
>>
>> I believe fbsd-nat.c will need a similar fixup?  It doesn't use the
>> values passed but does implement the target method.
> 
> Hi John,
> 
> Can you check if this updated patch builds properly on FreeBSD?
> 
> Thanks!

Just one typo below:

> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 265175a769..1a36414837 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -1163,8 +1163,9 @@ fbsd_remove_exec_catchpoint (struct target_ops *self, int pid)
> 
>  #ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
>  static int
> -fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, int needed,
> -			     int any_count, int table_size, int *table)
> +fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, bool needed,
> +			     int any_count,
> +			     gdb::array_view<const int> syscall_counts))

Double close paren here should be a single, but builds fine aside from that, thanks!
Simon Marchi - Dec. 6, 2017, 10:50 p.m.
On 2017-12-06 05:38 PM, John Baldwin wrote:
> On 12/3/17 10:18 AM, Simon Marchi wrote:
>> On 2017-10-30 11:59 AM, John Baldwin wrote:
>>> On 10/30/17 3:32 PM, Pedro Alves wrote:
>>>> I noticed that we're passing down a data/size pair to
>>>> target_ops::to_set_syscall_catchpoint.  This commit makes use of
>>>> gdb::array_view instead.  While at it, use bool where appropriate as
>>>> well.
>>>>
>>>> gdb/ChangeLog:
>>>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>>>
>>>> 	* break-catch-syscall.c (insert_catch_syscall)
>>>> 	(remove_catch_syscall): Adjust to pass reference to
>>>> 	inf_data->syscalls_counts directly via gdb::array_view.
>>>> 	* linux-nat.c (linux_child_set_syscall_catchpoint): Adjust to use
>>>> 	bool and gdb::array_view.
>>>
>>> I believe fbsd-nat.c will need a similar fixup?  It doesn't use the
>>> values passed but does implement the target method.
>>
>> Hi John,
>>
>> Can you check if this updated patch builds properly on FreeBSD?
>>
>> Thanks!
> 
> Just one typo below:
> 
>> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
>> index 265175a769..1a36414837 100644
>> --- a/gdb/fbsd-nat.c
>> +++ b/gdb/fbsd-nat.c
>> @@ -1163,8 +1163,9 @@ fbsd_remove_exec_catchpoint (struct target_ops *self, int pid)
>>
>>  #ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
>>  static int
>> -fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, int needed,
>> -			     int any_count, int table_size, int *table)
>> +fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, bool needed,
>> +			     int any_count,
>> +			     gdb::array_view<const int> syscall_counts))
> 
> Double close paren here should be a single, but builds fine aside from that, thanks!
> 

Thanks, I pushed the patch with that fixed.

Simon

Patch

diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 41ea424672..7bb2ffef37 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -115,8 +115,7 @@  insert_catch_syscall (struct bp_location *bl)
   return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
 					inf_data->total_syscalls_count != 0,
 					inf_data->any_syscall_count,
-					inf_data->syscalls_counts.size (),
-					inf_data->syscalls_counts.data ());
+					inf_data->syscalls_counts);
 }

 /* Implement the "remove" breakpoint_ops method for syscall
@@ -148,8 +147,7 @@  remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
   return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
 					inf_data->total_syscalls_count != 0,
 					inf_data->any_syscall_count,
-					inf_data->syscalls_counts.size (),
-					inf_data->syscalls_counts.data ());
+					inf_data->syscalls_counts);
 }

 /* Implement the "breakpoint_hit" breakpoint_ops method for syscall
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 265175a769..1a36414837 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1163,8 +1163,9 @@  fbsd_remove_exec_catchpoint (struct target_ops *self, int pid)

 #ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
 static int
-fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, int needed,
-			     int any_count, int table_size, int *table)
+fbsd_set_syscall_catchpoint (struct target_ops *self, int pid, bool needed,
+			     int any_count,
+			     gdb::array_view<const int> syscall_counts))
 {

   /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 96cb21a2cf..b8f3108937 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -676,8 +676,8 @@  linux_child_remove_exec_catchpoint (struct target_ops *self, int pid)

 static int
 linux_child_set_syscall_catchpoint (struct target_ops *self,
-				    int pid, int needed, int any_count,
-				    int table_size, int *table)
+				    int pid, bool needed, int any_count,
+				    gdb::array_view<const int> syscall_counts)
 {
   if (!linux_supports_tracesysgood ())
     return 1;
@@ -685,7 +685,7 @@  linux_child_set_syscall_catchpoint (struct target_ops *self,
   /* On GNU/Linux, we ignore the arguments.  It means that we only
      enable the syscall catchpoints, but do not disable them.

-     Also, we do not use the `table' information because we do not
+     Also, we do not use the `syscall_counts' information because we do not
      filter system calls here.  We let GDB do the logic for us.  */
   return 0;
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index 0a62ae00e5..5f1c9699e3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2024,8 +2024,8 @@  remote_pass_signals (struct target_ops *self,

 static int
 remote_set_syscall_catchpoint (struct target_ops *self,
-			       int pid, int needed, int any_count,
-			       int table_size, int *table)
+			       int pid, bool needed, int any_count,
+			       gdb::array_view<const int> syscall_counts)
 {
   const char *catch_packet;
   enum packet_result result;
@@ -2037,14 +2037,12 @@  remote_set_syscall_catchpoint (struct target_ops *self,
       return 1;
     }

-  if (needed && !any_count)
+  if (needed && any_count == 0)
     {
-      int i;
-
-      /* Count how many syscalls are to be caught (table[sysno] != 0).  */
-      for (i = 0; i < table_size; i++)
+      /* Count how many syscalls are to be caught.  */
+      for (size_t i = 0; i < syscall_counts.size (); i++)
 	{
-	  if (table[i] != 0)
+	  if (syscall_counts[i] != 0)
 	    n_sysno++;
 	}
     }
@@ -2066,13 +2064,13 @@  remote_set_syscall_catchpoint (struct target_ops *self,
       const int maxpktsz = strlen ("QCatchSyscalls:1") + n_sysno * 9 + 1;
       built_packet.reserve (maxpktsz);
       built_packet = "QCatchSyscalls:1";
-      if (!any_count)
+      if (any_count == 0)
 	{
-	  /* Add in catch_packet each syscall to be caught (table[i] != 0).  */
-	  for (int i = 0; i < table_size; i++)
+	  /* Add in each syscall to be caught.  */
+	  for (size_t i = 0; i < syscall_counts.size (); i++)
 	    {
-	      if (table[i] != 0)
-		string_appendf (built_packet, ";%x", i);
+	      if (syscall_counts[i] != 0)
+		string_appendf (built_packet, ";%zx", i);
 	    }
 	}
       if (built_packet.size () > get_remote_packet_size ())
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index d1d7fb5b83..27e5a55c40 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -56,6 +56,8 @@ 
   target_debug_do_print (((X) ? (X) : "(null)"))
 #define target_debug_print_int(X)		\
   target_debug_do_print (plongest (X))
+#define target_debug_print_bool(X)		\
+  target_debug_do_print ((X) ? "true" : "false")
 #define target_debug_print_long(X)		\
   target_debug_do_print (plongest (X))
 #define target_debug_print_enum_target_xfer_status(X)	\
@@ -166,6 +168,8 @@ 
   target_debug_do_print (plongest (X))
 #define target_debug_print_traceframe_info_up(X) \
   target_debug_do_print (host_address_to_string (X.get ()))
+#define target_debug_print_gdb_array_view_const_int(X)	\
+  target_debug_do_print (host_address_to_string (X.data ()))

 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 9691074bfe..aaf11d81b8 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1286,36 +1286,34 @@  debug_follow_exec (struct target_ops *self, struct inferior *arg1, char *arg2)
 }

 static int
-delegate_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int arg3, int arg4, int *arg5)
+delegate_set_syscall_catchpoint (struct target_ops *self, int arg1, bool arg2, int arg3, gdb::array_view<const int> arg4)
 {
   self = self->beneath;
-  return self->to_set_syscall_catchpoint (self, arg1, arg2, arg3, arg4, arg5);
+  return self->to_set_syscall_catchpoint (self, arg1, arg2, arg3, arg4);
 }

 static int
-tdefault_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int arg3, int arg4, int *arg5)
+tdefault_set_syscall_catchpoint (struct target_ops *self, int arg1, bool arg2, int arg3, gdb::array_view<const int> arg4)
 {
   return 1;
 }

 static int
-debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int arg3, int arg4, int *arg5)
+debug_set_syscall_catchpoint (struct target_ops *self, int arg1, bool arg2, int arg3, gdb::array_view<const int> arg4)
 {
   int result;
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_set_syscall_catchpoint (...)\n", debug_target.to_shortname);
-  result = debug_target.to_set_syscall_catchpoint (&debug_target, arg1, arg2, arg3, arg4, arg5);
+  result = debug_target.to_set_syscall_catchpoint (&debug_target, arg1, arg2, arg3, arg4);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_set_syscall_catchpoint (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_int (arg1);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg2);
+  target_debug_print_bool (arg2);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_int (arg3);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg4);
-  fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int_p (arg5);
+  target_debug_print_gdb_array_view_const_int (arg4);
   fputs_unfiltered (") = ", gdb_stdlog);
   target_debug_print_int (result);
   fputs_unfiltered ("\n", gdb_stdlog);
diff --git a/gdb/target.h b/gdb/target.h
index 638e2f06e6..7863a8d843 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -607,7 +607,8 @@  struct target_ops
     void (*to_follow_exec) (struct target_ops *, struct inferior *, char *)
       TARGET_DEFAULT_IGNORE ();
     int (*to_set_syscall_catchpoint) (struct target_ops *,
-				      int, int, int, int, int *)
+				      int, bool, int,
+				      gdb::array_view<const int>)
       TARGET_DEFAULT_RETURN (1);
     int (*to_has_exited) (struct target_ops *, int, int, int *)
       TARGET_DEFAULT_RETURN (0);
@@ -1615,28 +1616,24 @@  void target_follow_exec (struct inferior *inf, char *execd_pathname);

 /* Syscall catch.

-   NEEDED is nonzero if any syscall catch (of any kind) is requested.
-   If NEEDED is zero, it means the target can disable the mechanism to
+   NEEDED is true if any syscall catch (of any kind) is requested.
+   If NEEDED is false, it means the target can disable the mechanism to
    catch system calls because there are no more catchpoints of this type.

    ANY_COUNT is nonzero if a generic (filter-less) syscall catch is
-   being requested.  In this case, both TABLE_SIZE and TABLE should
-   be ignored.
+   being requested.  In this case, SYSCALL_COUNTS should be ignored.

-   TABLE_SIZE is the number of elements in TABLE.  It only matters if
-   ANY_COUNT is zero.
-
-   TABLE is an array of ints, indexed by syscall number.  An element in
-   this array is nonzero if that syscall should be caught.  This argument
-   only matters if ANY_COUNT is zero.
+   SYSCALL_COUNTS is an array of ints, indexed by syscall number.  An
+   element in this array is nonzero if that syscall should be caught.
+   This argument only matters if ANY_COUNT is zero.

    Return 0 for success, 1 if syscall catchpoints are not supported or -1
    for failure.  */

-#define target_set_syscall_catchpoint(pid, needed, any_count, table_size, table) \
+#define target_set_syscall_catchpoint(pid, needed, any_count, syscall_counts) \
      (*current_target.to_set_syscall_catchpoint) (&current_target,	\
 						  pid, needed, any_count, \
-						  table_size, table)
+						  syscall_counts)

 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */