[v2,2/3] Add an optional "alias" attribute to syscall entries.
Commit Message
When setting a syscall catchpoint by name, catch syscalls whose name
or alias matches the requested string.
When the ABI of a system call is changed in the FreeBSD kernel, this
is implemented by leaving a compatibility system call using the old
ABI at the existing "slot" and allocating a new system call for the
version using the new ABI. For example, new fields were added to the
'struct kevent' used by the kevent() system call in FreeBSD 12. The
previous kevent() system call in FreeBSD 12 kernels is now called
freebsd11_kevent() and is still used by older binaries compiled
against the older ABI. The freebsd11_kevent() system call can be
tagged with an "alias" attribute of "kevent" permitting 'catch syscall
kevent' to catch both system calls and providing the expected user
behavior for both old and new binaries. It also provides the expected
behavior if GDB is compiled on an older host (such as a FreeBSD 11
host).
gdb/ChangeLog:
* break-catch-syscall.c (catch_syscall_split_args): Update for
get_syscalls_by_name returning a vector.
* gdbarch.sh (UNKNOWN_SYSCALL): Remove.
* gdbarch.h: Regenerate.
* syscalls/gdb-syscalls.dtd (syscall): Add alias attribute.
* xml-syscall.c [!HAVE_LIBEXPAT] (get_syscalls_by_name): Rename
from get_syscall_by_name. Now returns a vector of integers.
[HAVE_LIBEXPAT] (struct syscall_desc): Add alias member.
(syscall_create_syscall_desc): Add alias parameter and pass it to
syscall_desc constructor.
(syscall_start_syscall): Handle alias attribute.
(syscall_attr): Add alias attribute.
(xml_get_syscalls_by_name): Rename from xml_get_syscall_number.
Now returns a vector of integers. Add syscalls whose alias or
name matches the requested name.
(get_syscalls_by_name): Rename from get_syscall_by_name. Now
returns a vector of integers.
* xml-syscall.h (get_syscalls_by_name): Likewise.
---
gdb/ChangeLog | 21 +++++++++++++
gdb/break-catch-syscall.c | 11 +++----
gdb/gdbarch.h | 3 --
gdb/gdbarch.sh | 3 --
gdb/syscalls/gdb-syscalls.dtd | 1 +
gdb/xml-syscall.c | 55 ++++++++++++++++++-----------------
gdb/xml-syscall.h | 9 +++---
7 files changed, 62 insertions(+), 41 deletions(-)
Comments
On Tue, 6 Nov 2018 09:54:30 -0800
John Baldwin <jhb@FreeBSD.org> wrote:
> When setting a syscall catchpoint by name, catch syscalls whose name
> or alias matches the requested string.
>
> When the ABI of a system call is changed in the FreeBSD kernel, this
> is implemented by leaving a compatibility system call using the old
> ABI at the existing "slot" and allocating a new system call for the
> version using the new ABI. For example, new fields were added to the
> 'struct kevent' used by the kevent() system call in FreeBSD 12. The
> previous kevent() system call in FreeBSD 12 kernels is now called
> freebsd11_kevent() and is still used by older binaries compiled
> against the older ABI. The freebsd11_kevent() system call can be
> tagged with an "alias" attribute of "kevent" permitting 'catch syscall
> kevent' to catch both system calls and providing the expected user
> behavior for both old and new binaries. It also provides the expected
> behavior if GDB is compiled on an older host (such as a FreeBSD 11
> host).
>
> gdb/ChangeLog:
>
> * break-catch-syscall.c (catch_syscall_split_args): Update for
> get_syscalls_by_name returning a vector.
> * gdbarch.sh (UNKNOWN_SYSCALL): Remove.
> * gdbarch.h: Regenerate.
> * syscalls/gdb-syscalls.dtd (syscall): Add alias attribute.
> * xml-syscall.c [!HAVE_LIBEXPAT] (get_syscalls_by_name): Rename
> from get_syscall_by_name. Now returns a vector of integers.
> [HAVE_LIBEXPAT] (struct syscall_desc): Add alias member.
> (syscall_create_syscall_desc): Add alias parameter and pass it to
> syscall_desc constructor.
> (syscall_start_syscall): Handle alias attribute.
> (syscall_attr): Add alias attribute.
> (xml_get_syscalls_by_name): Rename from xml_get_syscall_number.
> Now returns a vector of integers. Add syscalls whose alias or
> name matches the requested name.
> (get_syscalls_by_name): Rename from get_syscall_by_name. Now
> returns a vector of integers.
> * xml-syscall.h (get_syscalls_by_name): Likewise.
Okay.
Kevin
On 2018-11-06 12:54, John Baldwin wrote:
> When setting a syscall catchpoint by name, catch syscalls whose name
> or alias matches the requested string.
>
> When the ABI of a system call is changed in the FreeBSD kernel, this
> is implemented by leaving a compatibility system call using the old
> ABI at the existing "slot" and allocating a new system call for the
> version using the new ABI. For example, new fields were added to the
> 'struct kevent' used by the kevent() system call in FreeBSD 12. The
> previous kevent() system call in FreeBSD 12 kernels is now called
> freebsd11_kevent() and is still used by older binaries compiled
> against the older ABI. The freebsd11_kevent() system call can be
> tagged with an "alias" attribute of "kevent" permitting 'catch syscall
> kevent' to catch both system calls and providing the expected user
> behavior for both old and new binaries. It also provides the expected
> behavior if GDB is compiled on an older host (such as a FreeBSD 11
> host).
Will we ever need to provide more than one alias to a syscall?
Simon
On 2018-11-09 10:28, Simon Marchi wrote:
> On 2018-11-06 12:54, John Baldwin wrote:
>> When setting a syscall catchpoint by name, catch syscalls whose name
>> or alias matches the requested string.
>>
>> When the ABI of a system call is changed in the FreeBSD kernel, this
>> is implemented by leaving a compatibility system call using the old
>> ABI at the existing "slot" and allocating a new system call for the
>> version using the new ABI. For example, new fields were added to the
>> 'struct kevent' used by the kevent() system call in FreeBSD 12. The
>> previous kevent() system call in FreeBSD 12 kernels is now called
>> freebsd11_kevent() and is still used by older binaries compiled
>> against the older ABI. The freebsd11_kevent() system call can be
>> tagged with an "alias" attribute of "kevent" permitting 'catch syscall
>> kevent' to catch both system calls and providing the expected user
>> behavior for both old and new binaries. It also provides the expected
>> behavior if GDB is compiled on an older host (such as a FreeBSD 11
>> host).
>
> Will we ever need to provide more than one alias to a syscall?
Sorry, I just saw you have already answered this question, here:
https://sourceware.org/ml/gdb-patches/2018-10/msg00385.html
Simon
Hi,
On 11/06/2018 05:54 PM, John Baldwin wrote:
> --- a/gdb/syscalls/gdb-syscalls.dtd
> +++ b/gdb/syscalls/gdb-syscalls.dtd
> @@ -12,4 +12,5 @@
> <!ATTLIST syscall
> name CDATA #REQUIRED
> number CDATA #REQUIRED
> + alias CDATA #IMPLIED
> groups CDATA #IMPLIED>
I think there should be a NEWS entry for this.
I'd think there should be a docs update as well, but it
looks like this dtd isn't documented in the manual. :-/
> + /* We have a name. Let's check if it's valid and fetch a
> + list of matching numbers. */
> + std::vector<int> numbers = get_syscalls_by_name (gdbarch, cur_name);
>
> - if (s.number == UNKNOWN_SYSCALL)
> + if (numbers.empty ())
> /* Here we have to issue an error instead of a warning,
> because GDB cannot do anything useful if there's no
> syscall number to be caught. */
> error (_("Unknown syscall name '%s'."), cur_name);
>
> /* Ok, it's valid. */
> - result.push_back (s.number);
> + for (int number : numbers)
> + result.push_back (number);
Nit: this return-vector interface seems like leads to a bit of unnecessary
work, and double the number of necessary vector/heap allocations -- if
get_syscalls_by_name were passed RESULT instead of returning a new vector,
it could add to RESULT directly? The code above would like something like this
instead with get_syscalls_by_name returning true/false to indicate
whether something was added:
/* We have a name. Let's check if it's valid and fetch a
list of matching numbers. */
if (!get_syscalls_by_name (gdbarch, cur_name, result))
/* Here we have to issue an error instead of a warning,
because GDB cannot do anything useful if there's no
syscall number to be caught. */
error (_("Unknown syscall name '%s'."), cur_name);
That means there's no need to copy numbers between the vectors
(since there's only one vector).
Same for get_syscalls_by_group.
Note that instead of:
> + for (int number : numbers)
> + result.push_back (number);
You could write:
result.insert (result.end (), numbers.begin (), numbers.end ());
which may be more efficient assuming insert is smart enough
to call std::vector::reserve to allocate space in one go.
Thanks,
Pedro Alves
On 11/9/18 8:31 AM, Pedro Alves wrote:
> Hi,
>
> On 11/06/2018 05:54 PM, John Baldwin wrote:
>
>> --- a/gdb/syscalls/gdb-syscalls.dtd
>> +++ b/gdb/syscalls/gdb-syscalls.dtd
>> @@ -12,4 +12,5 @@
>> <!ATTLIST syscall
>> name CDATA #REQUIRED
>> number CDATA #REQUIRED
>> + alias CDATA #IMPLIED
>> groups CDATA #IMPLIED>
>
> I think there should be a NEWS entry for this.
Ok.
> I'd think there should be a docs update as well, but it
> looks like this dtd isn't documented in the manual. :-/
>
>
>> + /* We have a name. Let's check if it's valid and fetch a
>> + list of matching numbers. */
>> + std::vector<int> numbers = get_syscalls_by_name (gdbarch, cur_name);
>>
>> - if (s.number == UNKNOWN_SYSCALL)
>> + if (numbers.empty ())
>> /* Here we have to issue an error instead of a warning,
>> because GDB cannot do anything useful if there's no
>> syscall number to be caught. */
>> error (_("Unknown syscall name '%s'."), cur_name);
>>
>> /* Ok, it's valid. */
>> - result.push_back (s.number);
>> + for (int number : numbers)
>> + result.push_back (number);
>
> Nit: this return-vector interface seems like leads to a bit of unnecessary
> work, and double the number of necessary vector/heap allocations -- if
> get_syscalls_by_name were passed RESULT instead of returning a new vector,
> it could add to RESULT directly? The code above would like something like this
> instead with get_syscalls_by_name returning true/false to indicate
> whether something was added:
>
> /* We have a name. Let's check if it's valid and fetch a
> list of matching numbers. */
> if (!get_syscalls_by_name (gdbarch, cur_name, result))
> /* Here we have to issue an error instead of a warning,
> because GDB cannot do anything useful if there's no
> syscall number to be caught. */
> error (_("Unknown syscall name '%s'."), cur_name);
>
> That means there's no need to copy numbers between the vectors
> (since there's only one vector).
>
> Same for get_syscalls_by_group.
Oh, yes, that is a much better idea. I will adopt that change in both
patches.
> Note that instead of:
>
>> + for (int number : numbers)
>> + result.push_back (number);
>
> You could write:
>
> result.insert (result.end (), numbers.begin (), numbers.end ());
>
> which may be more efficient assuming insert is smart enough
> to call std::vector::reserve to allocate space in one go.
Ok. I wasn't quite sure if there was a more efficient idiom to use. It's
a bit of a shame std::vector<> doesn't have an explicit append/join method
similar to list.append() in Python.
@@ -1,3 +1,24 @@
+2018-11-06 John Baldwin <jhb@FreeBSD.org>
+
+ * break-catch-syscall.c (catch_syscall_split_args): Update for
+ get_syscalls_by_name returning a vector.
+ * gdbarch.sh (UNKNOWN_SYSCALL): Remove.
+ * gdbarch.h: Regenerate.
+ * syscalls/gdb-syscalls.dtd (syscall): Add alias attribute.
+ * xml-syscall.c [!HAVE_LIBEXPAT] (get_syscalls_by_name): Rename
+ from get_syscall_by_name. Now returns a vector of integers.
+ [HAVE_LIBEXPAT] (struct syscall_desc): Add alias member.
+ (syscall_create_syscall_desc): Add alias parameter and pass it to
+ syscall_desc constructor.
+ (syscall_start_syscall): Handle alias attribute.
+ (syscall_attr): Add alias attribute.
+ (xml_get_syscalls_by_name): Rename from xml_get_syscall_number.
+ Now returns a vector of integers. Add syscalls whose alias or
+ name matches the requested name.
+ (get_syscalls_by_name): Rename from get_syscall_by_name. Now
+ returns a vector of integers.
+ * xml-syscall.h (get_syscalls_by_name): Likewise.
+
2018-11-06 John Baldwin <jhb@FreeBSD.org>
* break-catch-syscall.c (catch_syscall_split_args): Update for
@@ -425,18 +425,19 @@ catch_syscall_split_args (const char *arg)
}
else
{
- /* We have a name. Let's check if it's valid and convert it
- to a number. */
- get_syscall_by_name (gdbarch, cur_name, &s);
+ /* We have a name. Let's check if it's valid and fetch a
+ list of matching numbers. */
+ std::vector<int> numbers = get_syscalls_by_name (gdbarch, cur_name);
- if (s.number == UNKNOWN_SYSCALL)
+ if (numbers.empty ())
/* Here we have to issue an error instead of a warning,
because GDB cannot do anything useful if there's no
syscall number to be caught. */
error (_("Unknown syscall name '%s'."), cur_name);
/* Ok, it's valid. */
- result.push_back (s.number);
+ for (int number : numbers)
+ result.push_back (number);
}
}
@@ -1569,9 +1569,6 @@ typedef ULONGEST (gdbarch_type_align_ftype) (struct gdbarch *gdbarch, struct typ
extern ULONGEST gdbarch_type_align (struct gdbarch *gdbarch, struct type *type);
extern void set_gdbarch_type_align (struct gdbarch *gdbarch, gdbarch_type_align_ftype *type_align);
-/* Definition for an unknown syscall, used basically in error-cases. */
-#define UNKNOWN_SYSCALL (-1)
-
extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
@@ -1393,9 +1393,6 @@ done
# close it off
cat <<EOF
-/* Definition for an unknown syscall, used basically in error-cases. */
-#define UNKNOWN_SYSCALL (-1)
-
extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
@@ -12,4 +12,5 @@
<!ATTLIST syscall
name CDATA #REQUIRED
number CDATA #REQUIRED
+ alias CDATA #IMPLIED
groups CDATA #IMPLIED>
@@ -61,13 +61,11 @@ get_syscall_by_number (struct gdbarch *gdbarch,
s->name = NULL;
}
-void
-get_syscall_by_name (struct gdbarch *gdbarch, const char *syscall_name,
- struct syscall *s)
+std::vector<int>
+get_syscalls_by_name (struct gdbarch *gdbarch, const char *syscall_name)
{
syscall_warn_user ();
- s->number = UNKNOWN_SYSCALL;
- s->name = syscall_name;
+ return std::vector<int> ();
}
const char **
@@ -96,8 +94,8 @@ get_syscall_group_names (struct gdbarch *gdbarch)
/* Structure which describes a syscall. */
struct syscall_desc
{
- syscall_desc (int number_, std::string name_)
- : number (number_), name (name_)
+ syscall_desc (int number_, std::string name_, std::string alias_)
+ : number (number_), name (name_), alias (alias_)
{}
/* The syscall number. */
@@ -107,6 +105,10 @@ struct syscall_desc
/* The syscall name. */
std::string name;
+
+ /* An optional alias. */
+
+ std::string alias;
};
typedef std::unique_ptr<syscall_desc> syscall_desc_up;
@@ -206,10 +208,11 @@ syscall_group_add_syscall (struct syscalls_info *syscalls_info,
static void
syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
- const char *name, int number,
+ const char *name, int number, const char *alias,
char *groups)
{
- syscall_desc *sysdesc = new syscall_desc (number, name);
+ syscall_desc *sysdesc = new syscall_desc (number, name,
+ alias != NULL ? alias : "");
syscalls_info->syscalls.emplace_back (sysdesc);
@@ -234,6 +237,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
/* syscall info. */
char *name = NULL;
int number = 0;
+ char *alias = NULL;
char *groups = NULL;
for (const gdb_xml_value &attr : attributes)
@@ -242,6 +246,8 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
name = (char *) attr.value.get ();
else if (strcmp (attr.name, "number") == 0)
number = * (ULONGEST *) attr.value.get ();
+ else if (strcmp (attr.name, "alias") == 0)
+ alias = (char *) attr.value.get ();
else if (strcmp (attr.name, "groups") == 0)
groups = (char *) attr.value.get ();
else
@@ -250,7 +256,8 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
}
gdb_assert (name);
- syscall_create_syscall_desc (data->syscalls_info, name, number, groups);
+ syscall_create_syscall_desc (data->syscalls_info, name, number, alias,
+ groups);
}
@@ -258,6 +265,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
static const struct gdb_xml_attribute syscall_attr[] = {
{ "number", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "name", GDB_XML_AF_NONE, NULL, NULL },
+ { "alias", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ "groups", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
};
@@ -389,21 +397,18 @@ syscall_group_get_group_by_name (const struct syscalls_info *syscalls_info,
return NULL;
}
-static int
-xml_get_syscall_number (struct gdbarch *gdbarch,
- const char *syscall_name)
+static std::vector<int>
+xml_get_syscalls_by_name (struct gdbarch *gdbarch, const char *syscall_name)
{
struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
+ std::vector<int> syscalls;
- if (syscalls_info == NULL
- || syscall_name == NULL)
- return UNKNOWN_SYSCALL;
+ if (syscalls_info != NULL && syscall_name != NULL)
+ for (const syscall_desc_up &sysdesc : syscalls_info->syscalls)
+ if (sysdesc->name == syscall_name || sysdesc->alias == syscall_name)
+ syscalls.push_back (sysdesc->number);
- for (const syscall_desc_up &sysdesc : syscalls_info->syscalls)
- if (sysdesc->name == syscall_name)
- return sysdesc->number;
-
- return UNKNOWN_SYSCALL;
+ return syscalls;
}
static const char *
@@ -506,14 +511,12 @@ get_syscall_by_number (struct gdbarch *gdbarch,
s->name = xml_get_syscall_name (gdbarch, syscall_number);
}
-void
-get_syscall_by_name (struct gdbarch *gdbarch,
- const char *syscall_name, struct syscall *s)
+std::vector<int>
+get_syscalls_by_name (struct gdbarch *gdbarch, const char *syscall_name)
{
init_syscalls_info (gdbarch);
- s->number = xml_get_syscall_number (gdbarch, syscall_name);
- s->name = syscall_name;
+ return xml_get_syscalls_by_name (gdbarch, syscall_name);
}
const char **
@@ -38,11 +38,12 @@ void set_xml_syscall_file_name (struct gdbarch *gdbarch,
void get_syscall_by_number (struct gdbarch *gdbarch,
int syscall_number, struct syscall *s);
-/* Function that retrieves the syscall number corresponding to the given
- name. It puts the requested information inside 'struct syscall'. */
+/* Function that retrieves the syscall numbers corresponding to the
+ given name. The numbers of all syscalls with either a name or
+ alias equal to SYSCALL_NAME are returned in a vector. */
-void get_syscall_by_name (struct gdbarch *gdbarch,
- const char *syscall_name, struct syscall *s);
+std::vector<int> get_syscalls_by_name (struct gdbarch *gdbarch,
+ const char *syscall_name);
/* Function used to retrieve the list of syscalls in the system. This list
is returned as an array of strings. Returns the list of syscalls in the