[PATCH 0/2] Make make-target-delegates grok C++ type names better

[PATCH 0/2] Make make-target-delegates grok C++ type names better

[Pedro Alves]

Using type names like std::string, std::vector<T> etc. in a target
method interface runs into a limitation in make-target-delegates.  The
first patch in the series fixes that.  I've wanted this before in my
multi-target series, and with C++-ification progressing, I guess
others will run into this too.  Better just fix it.

The second patch makes use of that fix in code that I recently noticed
today could use gdb::array_view instead of a pointer+size pair, as an
example.

Pedro Alves (2):
  Make make-target-delegates grok namespace scope op and template params
  target_set_syscall_catchpoint, use gdb::array_view and bool

 gdb/break-catch-syscall.c |  6 ++----
 gdb/linux-nat.c           |  4 ++--
 gdb/make-target-delegates | 12 +++++++++++-
 gdb/remote.c              | 24 +++++++++++-------------
 gdb/target-debug.h        |  6 +++++-
 gdb/target-delegates.c    | 18 ++++++++----------
 gdb/target.h              | 23 ++++++++++-------------
 7 files changed, 49 insertions(+), 44 deletions(-)

-- 
2.5.5

[PATCH 1/2] Make make-target-delegates grok namespace scope op and template params

[Pedro Alves]

The next patch will want to use gdb::array_view<int> as parameter type
of a target_ops method.  However, that runs into a
make-target-delegates limitation: target_debug_foo calls in
target-delegates.c for parameters/return types with namespace scope
operators ("::") or template parameters, end up looking like:

 @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int a
    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);

which obviously isn't something that compiles.  The problem is that
make-target-delegates wasn't ever taught that '::', '<', and '>' can
appear in parameter/return types.  You could work around it by hidding
the unsupported characters behind a typedef in the target method
declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
better to just remove the limitation.

While at it, also fix an "abuse" of reserved identifiers.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* make-target-delegates (munge_type): Also munge '<', '>', and
	':'.  Avoid double underscores in identifiers, and trailing
	underscores.
	* target-debug.h
	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
	...
	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
	* target-delegates.c: Regenerate.
---
 gdb/make-target-delegates | 12 +++++++++++-
 gdb/target-debug.h        |  2 +-
 gdb/target-delegates.c    |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
index fd51c64..1773232 100755
--- a/gdb/make-target-delegates
+++ b/gdb/make-target-delegates
@@ -232,8 +232,18 @@ sub munge_type($) {
 	$result = $1;
     } else {
 	($result = $typename) =~ s/\s+$//;
-	$result =~ s/[ ()]/_/g;
+	$result =~ s/[ ()<>:]/_/g;
 	$result =~ s/[*]/p/g;
+
+	# Identifers with double underscores are reserved to the C++
+	# implementation.
+	$result =~ s/_+/_/g;
+
+	# Avoid ending the function name with underscore, for
+	# cosmetics.  Trailing underscores appear after munging types
+	# with template parameters, like e.g. "foo<int>".
+	$result =~ s/_$//g;
+
 	$result = 'target_debug_print_' . $result;
     }
 
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 14196b4..068495e 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -116,7 +116,7 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_mem_region_vector(X) \
   target_debug_do_print (host_address_to_string (X.data ()))
-#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
+#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_const_struct_target_desc_p(X)	\
   target_debug_do_print (host_address_to_string (X))
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index e0d7a9a..1cbe6f8 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -3352,7 +3352,7 @@ debug_static_tracepoint_markers_by_strid (struct target_ops *self, const char *a
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_const_char_p (arg1);
   fputs_unfiltered (") = ", gdb_stdlog);
-  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
+  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
   fputs_unfiltered ("\n", gdb_stdlog);
   return result;
 }
-- 
2.5.5

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

[Pedro Alves]

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.
	* 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/linux-nat.c           |  4 ++--
 gdb/remote.c              | 24 +++++++++++-------------
 gdb/target-debug.h        |  4 ++++
 gdb/target-delegates.c    | 16 +++++++---------
 gdb/target.h              | 23 ++++++++++-------------
 6 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index b0ed4d2..221ae9a 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/linux-nat.c b/gdb/linux-nat.c
index 5d8f9f3..1f953a6 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;
diff --git a/gdb/remote.c b/gdb/remote.c
index 57dbd1e..3b35fba 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2048,8 +2048,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;
@@ -2061,14 +2061,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++;
 	}
     }
@@ -2090,13 +2088,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 068495e..383d4ea 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 1cbe6f8..8702a9e 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 e7577e1..7bcdefb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -612,7 +612,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);
@@ -1620,28 +1621,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, table) \
      (*current_target.to_set_syscall_catchpoint) (&current_target,	\
 						  pid, needed, any_count, \
-						  table_size, table)
+						  table)
 
 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */
-- 
2.5.5

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

[John Baldwin]

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.

-- 
John Baldwin

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

[Simon Marchi]

On 2017-10-30 11:32, Pedro Alves wrote:
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 1cbe6f8..8702a9e 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 e7577e1..7bcdefb 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -612,7 +612,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>)

Is there any advantage of using array_view instead of vector directly?  
In case we decide to change from vector to another "kind" of array?

>        TARGET_DEFAULT_RETURN (1);
>      int (*to_has_exited) (struct target_ops *, int, int, int *)
>        TARGET_DEFAULT_RETURN (0);
> @@ -1620,28 +1621,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, table) \
>       (*current_target.to_set_syscall_catchpoint) (&current_target,	\
>  						  pid, needed, any_count, \
> -						  table_size, table)
> +						  table)

The doc talks about SYSCALL_COUNTS, but the parameter is named table.

Simon

Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params

[Simon Marchi]

On 2017-10-30 11:32, Pedro Alves wrote:
> The next patch will want to use gdb::array_view<int> as parameter type
> of a target_ops method.  However, that runs into a
> make-target-delegates limitation: target_debug_foo calls in
> target-delegates.c for parameters/return types with namespace scope
> operators ("::") or template parameters, end up looking like:
> 
>  @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops
> *self, int arg1, int arg2, int a
>     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);
> 
> which obviously isn't something that compiles.  The problem is that
> make-target-delegates wasn't ever taught that '::', '<', and '>' can
> appear in parameter/return types.  You could work around it by hidding
> the unsupported characters behind a typedef in the target method
> declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
> better to just remove the limitation.
> 
> While at it, also fix an "abuse" of reserved identifiers.
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* make-target-delegates (munge_type): Also munge '<', '>', and
> 	':'.  Avoid double underscores in identifiers, and trailing
> 	underscores.
> 	* target-debug.h
> 	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
> 	...
> 	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
> 	* target-delegates.c: Regenerate.
> ---
>  gdb/make-target-delegates | 12 +++++++++++-
>  gdb/target-debug.h        |  2 +-
>  gdb/target-delegates.c    |  2 +-
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
> index fd51c64..1773232 100755
> --- a/gdb/make-target-delegates
> +++ b/gdb/make-target-delegates
> @@ -232,8 +232,18 @@ sub munge_type($) {
>  	$result = $1;
>      } else {
>  	($result = $typename) =~ s/\s+$//;
> -	$result =~ s/[ ()]/_/g;
> +	$result =~ s/[ ()<>:]/_/g;
>  	$result =~ s/[*]/p/g;
> +
> +	# Identifers with double underscores are reserved to the C++
> +	# implementation.
> +	$result =~ s/_+/_/g;
> +
> +	# Avoid ending the function name with underscore, for
> +	# cosmetics.  Trailing underscores appear after munging types
> +	# with template parameters, like e.g. "foo<int>".
> +	$result =~ s/_$//g;
> +
>  	$result = 'target_debug_print_' . $result;
>      }
> 
> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
> index 14196b4..068495e 100644
> --- a/gdb/target-debug.h
> +++ b/gdb/target-debug.h
> @@ -116,7 +116,7 @@
>    target_debug_do_print (host_address_to_string (X))
>  #define target_debug_print_mem_region_vector(X) \
>    target_debug_do_print (host_address_to_string (X.data ()))
> -#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
> +#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
>    target_debug_do_print (host_address_to_string (X))
>  #define target_debug_print_const_struct_target_desc_p(X)	\
>    target_debug_do_print (host_address_to_string (X))
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index e0d7a9a..1cbe6f8 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -3352,7 +3352,7 @@ debug_static_tracepoint_markers_by_strid (struct
> target_ops *self, const char *a
>    fputs_unfiltered (", ", gdb_stdlog);
>    target_debug_print_const_char_p (arg1);
>    fputs_unfiltered (") = ", gdb_stdlog);
> -  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
> +  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
>    fputs_unfiltered ("\n", gdb_stdlog);
>    return result;
>  }

LGTM, I'll make a patch to remove the mem_region_vector typedef.

Simon

[PATCH] Remove mem_region_vector typedef

[Simon Marchi]

[Something we can push once that series is in.]

Now that make-target-delegates understands namespaces and templates,
this typedef is no longer useful.

gdb/ChangeLog:

	* target.h (mem_region_vector): Remove.
	(struct target_ops) <to_memory_map>: Change return type to
	std::vector<mem_region>.
	* target-debug.h (target_debug_print_mem_region_vector): Rename
	to ...
	(target_debug_print_std_vector_mem_region): ... this.
	* target-delegates.c: Re-generate.
---
 gdb/target-debug.h     |  2 +-
 gdb/target-delegates.c | 10 +++++-----
 gdb/target.h           |  7 +------
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 383d4ea..27e5a55 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -116,7 +116,7 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_bfd_p(X) \
   target_debug_do_print (host_address_to_string (X))
-#define target_debug_print_mem_region_vector(X) \
+#define target_debug_print_std_vector_mem_region(X) \
   target_debug_do_print (host_address_to_string (X.data ()))
 #define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
   target_debug_do_print (host_address_to_string (X))
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 8702a9e..aaf11d8 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -2146,29 +2146,29 @@ debug_get_memory_xfer_limit (struct target_ops *self)
   return result;
 }
 
-static mem_region_vector
+static std::vector<mem_region>
 delegate_memory_map (struct target_ops *self)
 {
   self = self->beneath;
   return self->to_memory_map (self);
 }
 
-static mem_region_vector
+static std::vector<mem_region>
 tdefault_memory_map (struct target_ops *self)
 {
   return std::vector<mem_region> ();
 }
 
-static mem_region_vector
+static std::vector<mem_region>
 debug_memory_map (struct target_ops *self)
 {
-  mem_region_vector result;
+  std::vector<mem_region> result;
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_memory_map (...)\n", debug_target.to_shortname);
   result = debug_target.to_memory_map (&debug_target);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_memory_map (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (") = ", gdb_stdlog);
-  target_debug_print_mem_region_vector (result);
+  target_debug_print_std_vector_mem_region (result);
   fputs_unfiltered ("\n", gdb_stdlog);
   return result;
 }
diff --git a/gdb/target.h b/gdb/target.h
index 7bcdefb..a4e696f 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -418,11 +418,6 @@ typedef void async_callback_ftype (enum inferior_event_type event_type,
 #define TARGET_DEFAULT_RETURN(ARG)
 #define TARGET_DEFAULT_FUNC(ARG)
 
-/* Define a typedef, because make-target-delegates doesn't currently handle type
-   names with templates.  */
-
-typedef std::vector<mem_region> mem_region_vector;
-
 struct target_ops
   {
     struct target_ops *beneath;	/* To the target under this one.  */
@@ -778,7 +773,7 @@ struct target_ops
        This method should not cache data; if the memory map could
        change unexpectedly, it should be invalidated, and higher
        layers will re-fetch it.  */
-    mem_region_vector (*to_memory_map) (struct target_ops *)
+    std::vector<mem_region> (*to_memory_map) (struct target_ops *)
       TARGET_DEFAULT_RETURN (std::vector<mem_region> ());
 
     /* Erases the region of flash memory starting at ADDRESS, of
-- 
2.7.4

Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params

[Simon Marchi]

On 2017-10-30 12:02 PM, Simon Marchi wrote:
> On 2017-10-30 11:32, Pedro Alves wrote:
>> The next patch will want to use gdb::array_view<int> as parameter type
>> of a target_ops method.  However, that runs into a
>> make-target-delegates limitation: target_debug_foo calls in
>> target-delegates.c for parameters/return types with namespace scope
>> operators ("::") or template parameters, end up looking like:
>>
>>  @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops
>> *self, int arg1, int arg2, int a
>>     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);
>>
>> which obviously isn't something that compiles.  The problem is that
>> make-target-delegates wasn't ever taught that '::', '<', and '>' can
>> appear in parameter/return types.  You could work around it by hidding
>> the unsupported characters behind a typedef in the target method
>> declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
>> better to just remove the limitation.
>>
>> While at it, also fix an "abuse" of reserved identifiers.
>>
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* make-target-delegates (munge_type): Also munge '<', '>', and
>> 	':'.  Avoid double underscores in identifiers, and trailing
>> 	underscores.
>> 	* target-debug.h
>> 	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
>> 	...
>> 	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
>> 	* target-delegates.c: Regenerate.
>> ---
>>  gdb/make-target-delegates | 12 +++++++++++-
>>  gdb/target-debug.h        |  2 +-
>>  gdb/target-delegates.c    |  2 +-
>>  3 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
>> index fd51c64..1773232 100755
>> --- a/gdb/make-target-delegates
>> +++ b/gdb/make-target-delegates
>> @@ -232,8 +232,18 @@ sub munge_type($) {
>>  	$result = $1;
>>      } else {
>>  	($result = $typename) =~ s/\s+$//;
>> -	$result =~ s/[ ()]/_/g;
>> +	$result =~ s/[ ()<>:]/_/g;
>>  	$result =~ s/[*]/p/g;
>> +
>> +	# Identifers with double underscores are reserved to the C++
>> +	# implementation.
>> +	$result =~ s/_+/_/g;
>> +
>> +	# Avoid ending the function name with underscore, for
>> +	# cosmetics.  Trailing underscores appear after munging types
>> +	# with template parameters, like e.g. "foo<int>".
>> +	$result =~ s/_$//g;
>> +
>>  	$result = 'target_debug_print_' . $result;
>>      }
>>
>> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
>> index 14196b4..068495e 100644
>> --- a/gdb/target-debug.h
>> +++ b/gdb/target-debug.h
>> @@ -116,7 +116,7 @@
>>    target_debug_do_print (host_address_to_string (X))
>>  #define target_debug_print_mem_region_vector(X) \
>>    target_debug_do_print (host_address_to_string (X.data ()))
>> -#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
>> +#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
>>    target_debug_do_print (host_address_to_string (X))
>>  #define target_debug_print_const_struct_target_desc_p(X)	\
>>    target_debug_do_print (host_address_to_string (X))
>> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
>> index e0d7a9a..1cbe6f8 100644
>> --- a/gdb/target-delegates.c
>> +++ b/gdb/target-delegates.c
>> @@ -3352,7 +3352,7 @@ debug_static_tracepoint_markers_by_strid (struct
>> target_ops *self, const char *a
>>    fputs_unfiltered (", ", gdb_stdlog);
>>    target_debug_print_const_char_p (arg1);
>>    fputs_unfiltered (") = ", gdb_stdlog);
>> -  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
>> +  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
>>    fputs_unfiltered ("\n", gdb_stdlog);
>>    return result;
>>  }
> 
> LGTM, I'll make a patch to remove the mem_region_vector typedef.
> 
> Simon

Hi Pedro,

Do you mind if I rebase and push this patch?  Additional cleanups I have
in line would benefit from it.

Thanks,

Simon

Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params

[Pedro Alves]

> Do you mind if I rebase and push this patch?  Additional cleanups I have
> in line would benefit from it.

Sure, please go ahead.  Thanks.

Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params

[Simon Marchi]

On 2017-12-03 06:58 AM, Pedro Alves wrote:
>> Do you mind if I rebase and push this patch?  Additional cleanups I have
>> in line would benefit from it.
> 
> Sure, please go ahead.  Thanks.

I rebased, build-tested and pushed.

Thanks,

Simon

Re: [PATCH] Remove mem_region_vector typedef

[Simon Marchi]

On 2017-10-30 01:05 PM, Simon Marchi wrote:
> [Something we can push once that series is in.]
> 
> Now that make-target-delegates understands namespaces and templates,
> this typedef is no longer useful.
> 
> gdb/ChangeLog:
> 
> 	* target.h (mem_region_vector): Remove.
> 	(struct target_ops) <to_memory_map>: Change return type to
> 	std::vector<mem_region>.
> 	* target-debug.h (target_debug_print_mem_region_vector): Rename
> 	to ...
> 	(target_debug_print_std_vector_mem_region): ... this.
> 	* target-delegates.c: Re-generate.
> ---
>  gdb/target-debug.h     |  2 +-
>  gdb/target-delegates.c | 10 +++++-----
>  gdb/target.h           |  7 +------
>  3 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
> index 383d4ea..27e5a55 100644
> --- a/gdb/target-debug.h
> +++ b/gdb/target-debug.h
> @@ -116,7 +116,7 @@
>    target_debug_do_print (host_address_to_string (X))
>  #define target_debug_print_bfd_p(X) \
>    target_debug_do_print (host_address_to_string (X))
> -#define target_debug_print_mem_region_vector(X) \
> +#define target_debug_print_std_vector_mem_region(X) \
>    target_debug_do_print (host_address_to_string (X.data ()))
>  #define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
>    target_debug_do_print (host_address_to_string (X))
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 8702a9e..aaf11d8 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -2146,29 +2146,29 @@ debug_get_memory_xfer_limit (struct target_ops *self)
>    return result;
>  }
>  
> -static mem_region_vector
> +static std::vector<mem_region>
>  delegate_memory_map (struct target_ops *self)
>  {
>    self = self->beneath;
>    return self->to_memory_map (self);
>  }
>  
> -static mem_region_vector
> +static std::vector<mem_region>
>  tdefault_memory_map (struct target_ops *self)
>  {
>    return std::vector<mem_region> ();
>  }
>  
> -static mem_region_vector
> +static std::vector<mem_region>
>  debug_memory_map (struct target_ops *self)
>  {
> -  mem_region_vector result;
> +  std::vector<mem_region> result;
>    fprintf_unfiltered (gdb_stdlog, "-> %s->to_memory_map (...)\n", debug_target.to_shortname);
>    result = debug_target.to_memory_map (&debug_target);
>    fprintf_unfiltered (gdb_stdlog, "<- %s->to_memory_map (", debug_target.to_shortname);
>    target_debug_print_struct_target_ops_p (&debug_target);
>    fputs_unfiltered (") = ", gdb_stdlog);
> -  target_debug_print_mem_region_vector (result);
> +  target_debug_print_std_vector_mem_region (result);
>    fputs_unfiltered ("\n", gdb_stdlog);
>    return result;
>  }
> diff --git a/gdb/target.h b/gdb/target.h
> index 7bcdefb..a4e696f 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -418,11 +418,6 @@ typedef void async_callback_ftype (enum inferior_event_type event_type,
>  #define TARGET_DEFAULT_RETURN(ARG)
>  #define TARGET_DEFAULT_FUNC(ARG)
>  
> -/* Define a typedef, because make-target-delegates doesn't currently handle type
> -   names with templates.  */
> -
> -typedef std::vector<mem_region> mem_region_vector;
> -
>  struct target_ops
>    {
>      struct target_ops *beneath;	/* To the target under this one.  */
> @@ -778,7 +773,7 @@ struct target_ops
>         This method should not cache data; if the memory map could
>         change unexpectedly, it should be invalidated, and higher
>         layers will re-fetch it.  */
> -    mem_region_vector (*to_memory_map) (struct target_ops *)
> +    std::vector<mem_region> (*to_memory_map) (struct target_ops *)
>        TARGET_DEFAULT_RETURN (std::vector<mem_region> ());
>  
>      /* Erases the region of flash memory starting at ADDRESS, of
> 

I pushed this one in too.

Simon

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

[Simon Marchi]

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(-)

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.  */
-- 
2.15.0

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

[John Baldwin]

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!

-- 
John Baldwin

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

[Simon Marchi]

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