* [PATCH 0/2] Make make-target-delegates grok C++ type names better @ 2017-10-30 15:32 Pedro Alves 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Pedro Alves @ 2017-10-30 15:32 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-10-30 15:32 [PATCH 0/2] Make make-target-delegates grok C++ type names better Pedro Alves @ 2017-10-30 15:32 ` Pedro Alves 2017-10-30 15:59 ` Simon Marchi 2017-10-30 15:59 ` John Baldwin 2017-10-30 15:32 ` [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params Pedro Alves 2017-10-30 17:05 ` [PATCH] Remove mem_region_vector typedef Simon Marchi 2 siblings, 2 replies; 14+ messages in thread From: Pedro Alves @ 2017-10-30 15:32 UTC (permalink / raw) To: gdb-patches 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) (¤t_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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves @ 2017-10-30 15:59 ` Simon Marchi 2017-10-30 15:59 ` John Baldwin 1 sibling, 0 replies; 14+ messages in thread From: Simon Marchi @ 2017-10-30 15:59 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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) (¤t_target, \ > pid, needed, any_count, \ > - table_size, table) > + table) The doc talks about SYSCALL_COUNTS, but the parameter is named table. Simon ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves 2017-10-30 15:59 ` Simon Marchi @ 2017-10-30 15:59 ` John Baldwin 2017-12-03 18:18 ` Simon Marchi 1 sibling, 1 reply; 14+ messages in thread From: John Baldwin @ 2017-10-30 15:59 UTC (permalink / raw) To: Pedro Alves, gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-10-30 15:59 ` John Baldwin @ 2017-12-03 18:18 ` Simon Marchi 2017-12-06 22:38 ` John Baldwin 0 siblings, 1 reply; 14+ messages in thread From: Simon Marchi @ 2017-12-03 18:18 UTC (permalink / raw) To: John Baldwin, Pedro Alves, gdb-patches 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) (¤t_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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-12-03 18:18 ` Simon Marchi @ 2017-12-06 22:38 ` John Baldwin 2017-12-06 22:50 ` Simon Marchi 0 siblings, 1 reply; 14+ messages in thread From: John Baldwin @ 2017-12-06 22:38 UTC (permalink / raw) To: Simon Marchi, Pedro Alves, gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool 2017-12-06 22:38 ` John Baldwin @ 2017-12-06 22:50 ` Simon Marchi 0 siblings, 0 replies; 14+ messages in thread From: Simon Marchi @ 2017-12-06 22:50 UTC (permalink / raw) To: John Baldwin, Simon Marchi, Pedro Alves, gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params 2017-10-30 15:32 [PATCH 0/2] Make make-target-delegates grok C++ type names better Pedro Alves 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves @ 2017-10-30 15:32 ` Pedro Alves 2017-10-30 16:02 ` Simon Marchi 2017-10-30 17:05 ` [PATCH] Remove mem_region_vector typedef Simon Marchi 2 siblings, 1 reply; 14+ messages in thread From: Pedro Alves @ 2017-10-30 15:32 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params 2017-10-30 15:32 ` [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params Pedro Alves @ 2017-10-30 16:02 ` Simon Marchi 2017-12-03 0:51 ` Simon Marchi 0 siblings, 1 reply; 14+ messages in thread From: Simon Marchi @ 2017-10-30 16:02 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params 2017-10-30 16:02 ` Simon Marchi @ 2017-12-03 0:51 ` Simon Marchi 2017-12-03 11:58 ` Pedro Alves 0 siblings, 1 reply; 14+ messages in thread From: Simon Marchi @ 2017-12-03 0:51 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params 2017-12-03 0:51 ` Simon Marchi @ 2017-12-03 11:58 ` Pedro Alves 2017-12-03 18:00 ` Simon Marchi 0 siblings, 1 reply; 14+ messages in thread From: Pedro Alves @ 2017-12-03 11:58 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params 2017-12-03 11:58 ` Pedro Alves @ 2017-12-03 18:00 ` Simon Marchi 0 siblings, 0 replies; 14+ messages in thread From: Simon Marchi @ 2017-12-03 18:00 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Remove mem_region_vector typedef 2017-10-30 15:32 [PATCH 0/2] Make make-target-delegates grok C++ type names better Pedro Alves 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves 2017-10-30 15:32 ` [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params Pedro Alves @ 2017-10-30 17:05 ` Simon Marchi 2017-12-03 18:06 ` Simon Marchi 2 siblings, 1 reply; 14+ messages in thread From: Simon Marchi @ 2017-10-30 17:05 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Remove mem_region_vector typedef 2017-10-30 17:05 ` [PATCH] Remove mem_region_vector typedef Simon Marchi @ 2017-12-03 18:06 ` Simon Marchi 0 siblings, 0 replies; 14+ messages in thread From: Simon Marchi @ 2017-12-03 18:06 UTC (permalink / raw) To: Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-12-06 22:50 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-30 15:32 [PATCH 0/2] Make make-target-delegates grok C++ type names better Pedro Alves 2017-10-30 15:32 ` [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool Pedro Alves 2017-10-30 15:59 ` Simon Marchi 2017-10-30 15:59 ` John Baldwin 2017-12-03 18:18 ` Simon Marchi 2017-12-06 22:38 ` John Baldwin 2017-12-06 22:50 ` Simon Marchi 2017-10-30 15:32 ` [PATCH 1/2] Make make-target-delegates grok namespace scope op and template params Pedro Alves 2017-10-30 16:02 ` Simon Marchi 2017-12-03 0:51 ` Simon Marchi 2017-12-03 11:58 ` Pedro Alves 2017-12-03 18:00 ` Simon Marchi 2017-10-30 17:05 ` [PATCH] Remove mem_region_vector typedef Simon Marchi 2017-12-03 18:06 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).