* [PATCH 01/12] Handle another edge case for TLS variable lookups.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-23 23:55 ` John Baldwin
2022-03-23 21:00 ` [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET John Baldwin
` (7 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
This is similar to the change in
df22c1e5d53c38f38bce6072bb46de240f9e0e2b but applies to the main
object file. The original change I encountered when testing TLS on
RISC-V for which I was unable to test TLS variables in the main
executable due to issues with compiler-generated debug info, so I only
checked for a backlink before walking the list of shared library
object files.
However, I ran into this issue again (of a separate debug object file
being passed to svr4_fetch_objfile_link_map) when testing TLS
variables on 32-bit arm. To fix, move the check for a backlink
earlier before the check for the main object file.
---
gdb/solib-svr4.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 69f2991f5e6..aba476f6cf2 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1449,15 +1449,15 @@ svr4_fetch_objfile_link_map (struct objfile *objfile)
if (info->main_lm_addr == 0)
solib_add (NULL, 0, auto_solib_add);
- /* svr4_current_sos() will set main_lm_addr for the main executable. */
- if (objfile == current_program_space->symfile_object_file)
- return info->main_lm_addr;
-
/* If OBJFILE is a separate debug object file, look for the
original object file. */
if (objfile->separate_debug_objfile_backlink != NULL)
objfile = objfile->separate_debug_objfile_backlink;
+ /* svr4_current_sos() will set main_lm_addr for the main executable. */
+ if (objfile == current_program_space->symfile_object_file)
+ return info->main_lm_addr;
+
/* The other link map addresses may be found by examining the list
of shared libraries. */
for (struct so_list *so : current_program_space->solibs ())
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 01/12] Handle another edge case for TLS variable lookups.
2022-03-23 21:00 ` [PATCH 01/12] Handle another edge case for TLS variable lookups John Baldwin
@ 2022-03-23 23:55 ` John Baldwin
2022-03-24 0:26 ` John Baldwin
0 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-03-23 23:55 UTC (permalink / raw)
To: binutils, gdb-patches
On 3/23/22 2:00 PM, John Baldwin wrote:
> This is similar to the change in
> df22c1e5d53c38f38bce6072bb46de240f9e0e2b but applies to the main
> object file. The original change I encountered when testing TLS on
> RISC-V for which I was unable to test TLS variables in the main
> executable due to issues with compiler-generated debug info, so I only
> checked for a backlink before walking the list of shared library
> object files.
>
> However, I ran into this issue again (of a separate debug object file
> being passed to svr4_fetch_objfile_link_map) when testing TLS
> variables on 32-bit arm. To fix, move the check for a backlink
> earlier before the check for the main object file.
I now see this when using a separate debug file on FreeBSD/amd64 as
well FWIW. This definitely used to work but now needs this fix for
me. Curiously, 'info address' shows the variable belonging to the
separate debug file, e.g.:
% gdb tls tls.core
...
(gdb) info address id
Symbol "id" is a thread-local variable at offset 0x0 in the thread-local storage for `/usr/home/john/work/johnsvn/test/tls/tls.debug'.
Arguably this is a recent bug somewhere in the DWARF parsing that
is longer following the backlink when choosing the object file
for a TLS variable? I think fixing svr4_fetch_objfile_link_map to
be resilient against this is still ok, but it might be worth
tracking down the other bug as well. I'll see if I can find it.
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/12] Handle another edge case for TLS variable lookups.
2022-03-23 23:55 ` John Baldwin
@ 2022-03-24 0:26 ` John Baldwin
0 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-24 0:26 UTC (permalink / raw)
To: gdb-patches
On 3/23/22 4:55 PM, John Baldwin wrote:
> On 3/23/22 2:00 PM, John Baldwin wrote:
>> This is similar to the change in
>> df22c1e5d53c38f38bce6072bb46de240f9e0e2b but applies to the main
>> object file. The original change I encountered when testing TLS on
>> RISC-V for which I was unable to test TLS variables in the main
>> executable due to issues with compiler-generated debug info, so I only
>> checked for a backlink before walking the list of shared library
>> object files.
>>
>> However, I ran into this issue again (of a separate debug object file
>> being passed to svr4_fetch_objfile_link_map) when testing TLS
>> variables on 32-bit arm. To fix, move the check for a backlink
>> earlier before the check for the main object file.
>
> I now see this when using a separate debug file on FreeBSD/amd64 as
> well FWIW. This definitely used to work but now needs this fix for
> me. Curiously, 'info address' shows the variable belonging to the
> separate debug file, e.g.:
>
> % gdb tls tls.core
> ...
> (gdb) info address id
> Symbol "id" is a thread-local variable at offset 0x0 in the thread-local storage for `/usr/home/john/work/johnsvn/test/tls/tls.debug'.
>
> Arguably this is a recent bug somewhere in the DWARF parsing that
> is longer following the backlink when choosing the object file
> for a TLS variable? I think fixing svr4_fetch_objfile_link_map to
> be resilient against this is still ok, but it might be worth
> tracking down the other bug as well. I'll see if I can find it.
So the change in behavior was introduced by this commit:
commit 9f47c7071654d8cee82ff91ec1e65d57bd78e77f
Author: Simon Marchi <simon.marchi@efficios.com>
Date: Wed May 27 11:14:04 2020 -0400
Remove dwarf2_per_cu_data::objfile ()
Since dwarf2_per_cu_data objects are going to become
objfile-independent, the backlink from dwarf2_per_cu_data to one
particular objfile must be removed. Instead, users of
dwarf2_per_cu_data that need an objfile must know from somewhere else in
the context of which objfile they are using this CU.
This also helps remove a dwarf2_per_cu_data::dwarf2_per_objfile
reference (from where the objfile was obtained).
Note that the dwarf2_per_cu_data::objfile method has a special case to
make sure to return the main objfile, if the objfile associated to the
dwarf2_per_cu_data is a separate debug objfile. I don't really know if
this is necessary: I ignored that, and didn't see any regression when
testing with the various Dejagnu boards with separate debug info, so I
presume it wasn't needed. If it turns out this was needed, then we can
have a helper method on the objfile type for that.
gdb/ChangeLog:
* dwarf2/read.h (struct dwarf2_per_cu_data) <objfile>: Remove.
* dwarf2/read.c (dwarf2_compute_name): Pass per_objfile down.
(read_call_site_scope): Assign per_objfile.
(dwarf2_per_cu_data::objfile): Remove.
* gdbtypes.h (struct call_site) <per_objfile>: New member.
* dwarf2/loc.h (dwarf2_evaluate_loc_desc): Add
dwarf2_per_objfile parameter.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Add
dwarf2_per_objfile parameter.
(dwarf_expr_reg_to_entry_parameter): Add output
dwarf2_per_objfile parameter.
(locexpr_get_frame_base): Update.
(class dwarf_evaluate_loc_desc) <get_tls_address>: Update.
<push_dwarf_reg_entry_value>: Update.
<call_site_to_target_addr>: Update.
(dwarf_entry_parameter_to_value): Add dwarf2_per_objfile
parameter.
(value_of_dwarf_reg_entry): Update.
(rw_pieced_value): Update.
(indirect_synthetic_pointer): Update.
(dwarf2_evaluate_property): Update.
(dwarf2_loc_desc_get_symbol_read_needs): Add dwarf2_per_objfile
parameter.
(locexpr_read_variable): Update.
(locexpr_get_symbol_read_needs): Update.
(loclist_read_variable): Update.
In particular, this appears to be fallout from removing the special case
to return the main objfile described in the last paragraph (and this means
it is also likely a regression in GDB 12?). I did find that 'info address'
also prints the name of the separate debug file in GDB 11, so that behavior
is not new.
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
2022-03-23 21:00 ` [PATCH 01/12] Handle another edge case for TLS variable lookups John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-24 8:51 ` Luis Machado
2022-03-23 21:00 ` [PATCH 03/12] Create pseudo sections for NT_ARM_TLS notes on FreeBSD John Baldwin
` (6 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
FreeBSD's kernel has recently added PT_GETREGSET and PT_SETREGSET
operations to fetch a register set named by an ELF note type. These
helper routines provide helpers to check for a register set's
existence, fetch registers for a register set, and store registers to
a register set.
---
gdb/fbsd-nat.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/fbsd-nat.h | 42 +++++++++++++++++++++++++++++
2 files changed, 114 insertions(+)
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 934fdbad6ef..84abdd9a322 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1772,6 +1772,78 @@ fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
return false;
}
+#ifdef PT_GETREGSET
+/* See fbsd-nat.h. */
+
+bool
+fbsd_nat_target::have_regset (ptid_t ptid, int note)
+{
+ pid_t pid = get_ptrace_pid (ptid);
+ struct iovec iov;
+
+ iov.iov_base = nullptr;
+ iov.iov_len = 0;
+ if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
+ return 0;
+ return iov.iov_len;
+}
+
+/* See fbsd-nat.h. */
+
+bool
+fbsd_nat_target::fetch_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset, void *regs,
+ size_t size)
+{
+ const struct regcache_map_entry *map
+ = (const struct regcache_map_entry *) regset->regmap;
+ pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+ if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
+ size))
+ {
+ struct iovec iov;
+
+ iov.iov_base = regs;
+ iov.iov_len = size;
+ if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
+ perror_with_name (_("Couldn't get registers"));
+
+ regcache->supply_regset (regset, regnum, regs, size);
+ return true;
+ }
+ return false;
+}
+
+bool
+fbsd_nat_target::store_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset, void *regs,
+ size_t size)
+{
+ const struct regcache_map_entry *map
+ = (const struct regcache_map_entry *) regset->regmap;
+ pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+ if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
+ size))
+ {
+ struct iovec iov;
+
+ iov.iov_base = regs;
+ iov.iov_len = size;
+ if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
+ perror_with_name (_("Couldn't get registers"));
+
+ regcache->collect_regset (regset, regnum, regs, size);
+
+ if (ptrace (PT_SETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
+ perror_with_name (_("Couldn't write registers"));
+ return true;
+ }
+ return false;
+}
+#endif
+
/* See fbsd-nat.h. */
bool
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 82f7ee47949..6a4003627e4 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -151,6 +151,19 @@ class fbsd_nat_target : public inf_ptrace_target
bool store_register_set (struct regcache *regcache, int regnum, int fetch_op,
int store_op, const struct regset *regset,
void *regs, size_t size);
+
+#ifdef PT_GETREGSET
+ /* Helper routines which use PT_GETREGSET and PT_SETREGSET for the
+ specified NOTE instead of regset-specific fetch and store
+ ops. */
+
+ bool fetch_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset, void *regs, size_t size);
+
+ bool store_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset, void *regs, size_t size);
+#endif
+
protected:
/* Wrapper versions of the above helpers which accept a register set
type such as 'struct reg' or 'struct fpreg'. */
@@ -172,6 +185,35 @@ class fbsd_nat_target : public inf_ptrace_target
return store_register_set (regcache, regnum, fetch_op, store_op, regset,
®s, sizeof (regs));
}
+
+#ifdef PT_GETREGSET
+ /* Helper routine for use in read_description in subclasses. This
+ routine checks if the register set for the specified NOTE is
+ present for a given PTID. If the register set is present, the
+ the size of the register set is returned. If the register set is
+ not present, zero is returned. */
+
+ bool have_regset (ptid_t ptid, int note);
+
+ /* Wrapper versions of the PT_GETREGSET and PT_REGSET helpers which
+ accept a register set type. */
+
+ template <class Regset>
+ bool fetch_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset)
+ {
+ Regset regs;
+ return fetch_regset (regcache, regnum, note, regset, ®s, sizeof (regs));
+ }
+
+ template <class Regset>
+ bool store_regset (struct regcache *regcache, int regnum, int note,
+ const struct regset *regset)
+ {
+ Regset regs;
+ return store_regset (regcache, regnum, note, regset, ®s, sizeof (regs));
+ }
+#endif
};
/* Fetch the signal information for PTID and store it in *SIGINFO.
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET.
2022-03-23 21:00 ` [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET John Baldwin
@ 2022-03-24 8:51 ` Luis Machado
2022-03-24 17:45 ` John Baldwin
0 siblings, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-03-24 8:51 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 3/23/22 21:00, John Baldwin wrote:
> FreeBSD's kernel has recently added PT_GETREGSET and PT_SETREGSET
> operations to fetch a register set named by an ELF note type. These
> helper routines provide helpers to check for a register set's
> existence, fetch registers for a register set, and store registers to
> a register set.
> ---
> gdb/fbsd-nat.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
> gdb/fbsd-nat.h | 42 +++++++++++++++++++++++++++++
> 2 files changed, 114 insertions(+)
>
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 934fdbad6ef..84abdd9a322 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -1772,6 +1772,78 @@ fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
> return false;
> }
>
> +#ifdef PT_GETREGSET
Instead of ifdef-ing the entire block of code, wouldn't it be better to
conditionally define the constant and have the code do checks at runtime?
If GDB is built on a machine without the proper headers and copied onto
a machine with support for PT_GETREGSET, it won't be able to use this
improvement. On the other hand, if we build GDB on a machine with the
headers, but copy it into a machine without support for PT_GETREGSET,
things will break, no?
> +/* See fbsd-nat.h. */
> +
> +bool
> +fbsd_nat_target::have_regset (ptid_t ptid, int note)
> +{
> + pid_t pid = get_ptrace_pid (ptid);
> + struct iovec iov;
> +
> + iov.iov_base = nullptr;
> + iov.iov_len = 0;
> + if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
> + return 0;
> + return iov.iov_len;
> +}
> +
> +/* See fbsd-nat.h. */
> +
> +bool
> +fbsd_nat_target::fetch_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset, void *regs,
> + size_t size)
> +{
> + const struct regcache_map_entry *map
> + = (const struct regcache_map_entry *) regset->regmap;
> + pid_t pid = get_ptrace_pid (regcache->ptid ());
> +
> + if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
> + size))
> + {
> + struct iovec iov;
> +
> + iov.iov_base = regs;
> + iov.iov_len = size;
> + if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
> + perror_with_name (_("Couldn't get registers"));
> +
> + regcache->supply_regset (regset, regnum, regs, size);
> + return true;
> + }
> + return false;
> +}
> +
> +bool
> +fbsd_nat_target::store_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset, void *regs,
> + size_t size)
> +{
> + const struct regcache_map_entry *map
> + = (const struct regcache_map_entry *) regset->regmap;
> + pid_t pid = get_ptrace_pid (regcache->ptid ());
> +
> + if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
> + size))
> + {
> + struct iovec iov;
> +
> + iov.iov_base = regs;
> + iov.iov_len = size;
> + if (ptrace (PT_GETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
> + perror_with_name (_("Couldn't get registers"));
> +
> + regcache->collect_regset (regset, regnum, regs, size);
> +
> + if (ptrace (PT_SETREGSET, pid, (PTRACE_TYPE_ARG3) &iov, note) == -1)
> + perror_with_name (_("Couldn't write registers"));
> + return true;
> + }
> + return false;
> +}
> +#endif
> +
> /* See fbsd-nat.h. */
>
> bool
> diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
> index 82f7ee47949..6a4003627e4 100644
> --- a/gdb/fbsd-nat.h
> +++ b/gdb/fbsd-nat.h
> @@ -151,6 +151,19 @@ class fbsd_nat_target : public inf_ptrace_target
> bool store_register_set (struct regcache *regcache, int regnum, int fetch_op,
> int store_op, const struct regset *regset,
> void *regs, size_t size);
> +
> +#ifdef PT_GETREGSET
> + /* Helper routines which use PT_GETREGSET and PT_SETREGSET for the
> + specified NOTE instead of regset-specific fetch and store
> + ops. */
> +
> + bool fetch_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset, void *regs, size_t size);
> +
> + bool store_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset, void *regs, size_t size);
> +#endif
> +
> protected:
> /* Wrapper versions of the above helpers which accept a register set
> type such as 'struct reg' or 'struct fpreg'. */
> @@ -172,6 +185,35 @@ class fbsd_nat_target : public inf_ptrace_target
> return store_register_set (regcache, regnum, fetch_op, store_op, regset,
> ®s, sizeof (regs));
> }
> +
> +#ifdef PT_GETREGSET
> + /* Helper routine for use in read_description in subclasses. This
> + routine checks if the register set for the specified NOTE is
> + present for a given PTID. If the register set is present, the
> + the size of the register set is returned. If the register set is
> + not present, zero is returned. */
> +
> + bool have_regset (ptid_t ptid, int note);
> +
> + /* Wrapper versions of the PT_GETREGSET and PT_REGSET helpers which
> + accept a register set type. */
> +
> + template <class Regset>
> + bool fetch_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset)
> + {
> + Regset regs;
> + return fetch_regset (regcache, regnum, note, regset, ®s, sizeof (regs));
> + }
> +
> + template <class Regset>
> + bool store_regset (struct regcache *regcache, int regnum, int note,
> + const struct regset *regset)
> + {
> + Regset regs;
> + return store_regset (regcache, regnum, note, regset, ®s, sizeof (regs));
> + }
> +#endif
> };
>
> /* Fetch the signal information for PTID and store it in *SIGINFO.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET.
2022-03-24 8:51 ` Luis Machado
@ 2022-03-24 17:45 ` John Baldwin
0 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-24 17:45 UTC (permalink / raw)
To: Luis Machado, binutils, gdb-patches
On 3/24/22 1:51 AM, Luis Machado wrote:
> On 3/23/22 21:00, John Baldwin wrote:
>> FreeBSD's kernel has recently added PT_GETREGSET and PT_SETREGSET
>> operations to fetch a register set named by an ELF note type. These
>> helper routines provide helpers to check for a register set's
>> existence, fetch registers for a register set, and store registers to
>> a register set.
>> ---
>> gdb/fbsd-nat.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> gdb/fbsd-nat.h | 42 +++++++++++++++++++++++++++++
>> 2 files changed, 114 insertions(+)
>>
>> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
>> index 934fdbad6ef..84abdd9a322 100644
>> --- a/gdb/fbsd-nat.c
>> +++ b/gdb/fbsd-nat.c
>> @@ -1772,6 +1772,78 @@ fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
>> return false;
>> }
>>
>> +#ifdef PT_GETREGSET
>
> Instead of ifdef-ing the entire block of code, wouldn't it be better to
> conditionally define the constant and have the code do checks at runtime?
>
> If GDB is built on a machine without the proper headers and copied onto
> a machine with support for PT_GETREGSET, it won't be able to use this
> improvement. On the other hand, if we build GDB on a machine with the
> headers, but copy it into a machine without support for PT_GETREGSET,
> things will break, no?
FreeBSD does not generally support forwards compatibility (the second case
of a newer binary on an older kernel), only backwards compatibility.
However, it is true that in the use cases for PT_GETREGSET for the
foreseeable future, they will all be conditioned on a runtime check
via the 'have_regset' function enabling an architecture feature, so I
could add a fallback for the macro and always enable these functions. I
will go ahead and make that change.
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/12] Create pseudo sections for NT_ARM_TLS notes on FreeBSD.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
2022-03-23 21:00 ` [PATCH 01/12] Handle another edge case for TLS variable lookups John Baldwin
2022-03-23 21:00 ` [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-23 21:00 ` [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15 John Baldwin
` (5 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
bfd/ChangeLog:
* elf.c (elfcore_grok_freebsd_note): Handle NT_ARM_TLS notes.
---
bfd/ChangeLog | 4 ++++
bfd/elf.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 86d42b2d639..c35990cf37d 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,7 @@
+2022-03-11 John Baldwin <jhb@FreeBSD.org>
+
+ * elf.c (elfcore_grok_freebsd_note): Handle NT_ARM_TLS notes.
+
2022-03-22 Steiner H Gunderson <steinar+sourceware@gunderson.no>
* dwarf2.c (_bfd_dwarf2_find_nearest_line): if a function name is
diff --git a/bfd/elf.c b/bfd/elf.c
index 82b53be99f9..5d8f5ec33bd 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -11040,6 +11040,9 @@ elfcore_grok_freebsd_note (bfd *abfd, Elf_Internal_Note *note)
return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.lwpinfo",
note);
+ case NT_ARM_TLS:
+ return elfcore_grok_aarch_tls (abfd, note);
+
case NT_ARM_VFP:
return elfcore_grok_arm_vfp (abfd, note);
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (2 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 03/12] Create pseudo sections for NT_ARM_TLS notes on FreeBSD John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-04-04 8:01 ` Luis Machado
2022-03-23 21:00 ` [PATCH 05/12] Read the tpidruro register from NT_ARM_TLS core dump notes on FreeBSD/arm John Baldwin
` (4 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
---
gdb/arch/aarch32.c | 2 ++
gdb/arch/arm.c | 6 +++++-
gdb/arch/arm.h | 7 ++++---
gdb/arm-fbsd-tdep.c | 4 ++--
gdb/arm-linux-nat.c | 6 +++---
gdb/arm-linux-tdep.c | 4 ++--
gdb/arm-netbsd-nat.c | 4 ++--
gdb/arm-tdep.c | 20 +++++++++++++++-----
gdb/arm-tdep.h | 2 +-
gdb/features/Makefile | 1 +
gdb/features/arm/arm-tls.c | 14 ++++++++++++++
gdb/features/arm/arm-tls.xml | 11 +++++++++++
12 files changed, 62 insertions(+), 19 deletions(-)
create mode 100644 gdb/features/arm/arm-tls.c
create mode 100644 gdb/features/arm/arm-tls.xml
diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
index 0c544d381f1..4d6ffb44a15 100644
--- a/gdb/arch/aarch32.c
+++ b/gdb/arch/aarch32.c
@@ -19,6 +19,7 @@
#include "aarch32.h"
#include "../features/arm/arm-core.c"
+#include "../features/arm/arm-tls.c"
#include "../features/arm/arm-vfpv3.c"
/* See aarch32.h. */
@@ -38,6 +39,7 @@ aarch32_create_target_description ()
/* Create a vfpv3 feature, then a blank NEON feature. */
regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
+ regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
return tdesc.release ();
}
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index 126e46a950a..15b600e22f4 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -22,6 +22,7 @@
#include "arm.h"
#include "../features/arm/arm-core.c"
+#include "../features/arm/arm-tls.c"
#include "../features/arm/arm-vfpv2.c"
#include "../features/arm/arm-vfpv3.c"
#include "../features/arm/xscale-iwmmxt.c"
@@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
/* See arch/arm.h. */
target_desc *
-arm_create_target_description (arm_fp_type fp_type)
+arm_create_target_description (arm_fp_type fp_type, bool tls)
{
target_desc_up tdesc = allocate_target_description ();
@@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type fp_type)
error (_("Invalid Arm FP type: %d"), fp_type);
}
+ if (tls)
+ regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
+
return tdesc.release ();
}
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index f75470e7572..32f29b20d33 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -49,6 +49,7 @@ enum gdb_regnum {
ARM_D0_REGNUM, /* VFP double-precision registers. */
ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
ARM_FPSCR_REGNUM,
+ ARM_TPIDRURO_REGNUM,
/* Other useful registers. */
ARM_FP_REGNUM = 11, /* Frame register in ARM code, if used. */
@@ -65,8 +66,8 @@ enum arm_register_counts {
ARM_NUM_ARG_REGS = 4,
/* Number of floating point argument registers. */
ARM_NUM_FP_ARG_REGS = 4,
- /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
- ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
+ /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM + 1. */
+ ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
};
/* Enum describing the different kinds of breakpoints. */
@@ -193,7 +194,7 @@ unsigned long shifted_reg_val (struct regcache *regcache,
/* Create an Arm target description with the given FP hardware type. */
-target_desc *arm_create_target_description (arm_fp_type fp_type);
+target_desc *arm_create_target_description (arm_fp_type fp_type, bool tls);
/* Create an Arm M-profile target description with the given hardware type. */
diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c
index bf337b13f98..06745a36186 100644
--- a/gdb/arm-fbsd-tdep.c
+++ b/gdb/arm-fbsd-tdep.c
@@ -188,9 +188,9 @@ arm_fbsd_read_description_auxv (struct target_ops *target)
return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPD32))
== (HWCAP_VFPv3 | HWCAP_VFPD32))
- return arm_read_description (ARM_FP_TYPE_VFPV3);
+ return arm_read_description (ARM_FP_TYPE_VFPV3, false);
else
- return arm_read_description (ARM_FP_TYPE_VFPV2);
+ return arm_read_description (ARM_FP_TYPE_VFPV2, false);
}
return nullptr;
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index f0f09acf2f9..2abaf5a675d 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -550,7 +550,7 @@ arm_linux_nat_target::read_description ()
}
if (arm_hwcap & HWCAP_IWMMXT)
- return arm_read_description (ARM_FP_TYPE_IWMMXT);
+ return arm_read_description (ARM_FP_TYPE_IWMMXT, false);
if (arm_hwcap & HWCAP_VFP)
{
@@ -567,9 +567,9 @@ arm_linux_nat_target::read_description ()
if (arm_hwcap & HWCAP_NEON)
return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
- return arm_read_description (ARM_FP_TYPE_VFPV3);
+ return arm_read_description (ARM_FP_TYPE_VFPV3, false);
- return arm_read_description (ARM_FP_TYPE_VFPV2);
+ return arm_read_description (ARM_FP_TYPE_VFPV2, false);
}
return this->beneath ()->read_description ();
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 6aac016afb9..805c6ac0459 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -741,9 +741,9 @@ arm_linux_core_read_description (struct gdbarch *gdbarch,
if (arm_hwcap & HWCAP_NEON)
return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
- return arm_read_description (ARM_FP_TYPE_VFPV3);
+ return arm_read_description (ARM_FP_TYPE_VFPV3, false);
- return arm_read_description (ARM_FP_TYPE_VFPV2);
+ return arm_read_description (ARM_FP_TYPE_VFPV2, false);
}
return nullptr;
diff --git a/gdb/arm-netbsd-nat.c b/gdb/arm-netbsd-nat.c
index 591a0ab1d54..764bbe8cd3d 100644
--- a/gdb/arm-netbsd-nat.c
+++ b/gdb/arm-netbsd-nat.c
@@ -346,13 +346,13 @@ arm_netbsd_nat_target::read_description ()
if (sysctlbyname("machdep.fpu_present", &flag, &len, NULL, 0) != 0
|| !flag)
- return arm_read_description (ARM_FP_TYPE_NONE);
+ return arm_read_description (ARM_FP_TYPE_NONE, false);
len = sizeof(flag);
if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag)
return aarch32_read_description ();
- return arm_read_description (ARM_FP_TYPE_VFPV3);
+ return arm_read_description (ARM_FP_TYPE_VFPV3, false);
}
void _initialize_arm_netbsd_nat ();
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 8e245648f23..194eb1f6aa1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -239,7 +239,7 @@ static const char **valid_disassembly_styles;
static const char *disassembly_style;
/* All possible arm target descriptors. */
-static struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID];
+static struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID][2];
static struct target_desc *tdesc_arm_mprofile_list[ARM_M_TYPE_INVALID];
/* This is used to keep the bfd arch_info in sync with the disassembly
@@ -9410,6 +9410,16 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
}
}
+ /* Check for the TLS register feature. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.arm.tls");
+ if (feature != nullptr)
+ {
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ ARM_TPIDRURO_REGNUM, "tpidruro");
+ if (!valid_p)
+ return NULL;
+ }
+
/* Check for MVE after all the checks for GPR's, VFP and Neon.
MVE (Helium) is an M-profile extension. */
if (is_m)
@@ -13725,14 +13735,14 @@ arm_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
/* See arm-tdep.h. */
const target_desc *
-arm_read_description (arm_fp_type fp_type)
+arm_read_description (arm_fp_type fp_type, bool tls)
{
- struct target_desc *tdesc = tdesc_arm_list[fp_type];
+ struct target_desc *tdesc = tdesc_arm_list[fp_type][tls];
if (tdesc == nullptr)
{
- tdesc = arm_create_target_description (fp_type);
- tdesc_arm_list[fp_type] = tdesc;
+ tdesc = arm_create_target_description (fp_type, tls);
+ tdesc_arm_list[fp_type][tls] = tdesc;
}
return tdesc;
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 8a9f618539f..c14ac86a9bd 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -301,7 +301,7 @@ extern void
const struct regcache *regcache);
/* Get the correct Arm target description with given FP hardware type. */
-const target_desc *arm_read_description (arm_fp_type fp_type);
+const target_desc *arm_read_description (arm_fp_type fp_type, bool tls);
/* Get the correct Arm M-Profile target description with given hardware
type. */
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 68e17d0085d..4b09819389a 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -207,6 +207,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
arm/arm-m-profile.xml \
arm/arm-m-profile-mve.xml \
arm/arm-m-profile-with-fpa.xml \
+ arm/arm-tls.xml \
arm/arm-vfpv2.xml \
arm/arm-vfpv3.xml \
arm/xscale-iwmmxt.xml \
diff --git a/gdb/features/arm/arm-tls.c b/gdb/features/arm/arm-tls.c
new file mode 100644
index 00000000000..d1214dda8ec
--- /dev/null
+++ b/gdb/features/arm/arm-tls.c
@@ -0,0 +1,14 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: arm-tls.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_arm_arm_tls (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.arm.tls");
+ tdesc_create_reg (feature, "tpidruro", regnum++, 1, NULL, 32, "data_ptr");
+ return regnum;
+}
diff --git a/gdb/features/arm/arm-tls.xml b/gdb/features/arm/arm-tls.xml
new file mode 100644
index 00000000000..3cdf73e776f
--- /dev/null
+++ b/gdb/features/arm/arm-tls.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2022 Free Software Foundation, Inc.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.arm.tls">
+ <reg name="tpidruro" bitsize="32" type="data_ptr"/>
+</feature>
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-03-23 21:00 ` [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15 John Baldwin
@ 2022-04-04 8:01 ` Luis Machado
2022-04-12 23:36 ` John Baldwin
0 siblings, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-04-04 8:01 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
Hi,
On 3/23/22 21:00, John Baldwin wrote:
> ---
> gdb/arch/aarch32.c | 2 ++
> gdb/arch/arm.c | 6 +++++-
> gdb/arch/arm.h | 7 ++++---
> gdb/arm-fbsd-tdep.c | 4 ++--
> gdb/arm-linux-nat.c | 6 +++---
> gdb/arm-linux-tdep.c | 4 ++--
> gdb/arm-netbsd-nat.c | 4 ++--
> gdb/arm-tdep.c | 20 +++++++++++++++-----
> gdb/arm-tdep.h | 2 +-
> gdb/features/Makefile | 1 +
> gdb/features/arm/arm-tls.c | 14 ++++++++++++++
> gdb/features/arm/arm-tls.xml | 11 +++++++++++
> 12 files changed, 62 insertions(+), 19 deletions(-)
> create mode 100644 gdb/features/arm/arm-tls.c
> create mode 100644 gdb/features/arm/arm-tls.xml
>
> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
> index 0c544d381f1..4d6ffb44a15 100644
> --- a/gdb/arch/aarch32.c
> +++ b/gdb/arch/aarch32.c
> @@ -19,6 +19,7 @@
> #include "aarch32.h"
>
> #include "../features/arm/arm-core.c"
> +#include "../features/arm/arm-tls.c"
> #include "../features/arm/arm-vfpv3.c"
>
> /* See aarch32.h. */
> @@ -38,6 +39,7 @@ aarch32_create_target_description ()
> /* Create a vfpv3 feature, then a blank NEON feature. */
> regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
> tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>
> return tdesc.release ();
> }
> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
> index 126e46a950a..15b600e22f4 100644
> --- a/gdb/arch/arm.c
> +++ b/gdb/arch/arm.c
> @@ -22,6 +22,7 @@
> #include "arm.h"
>
> #include "../features/arm/arm-core.c"
> +#include "../features/arm/arm-tls.c"
> #include "../features/arm/arm-vfpv2.c"
> #include "../features/arm/arm-vfpv3.c"
> #include "../features/arm/xscale-iwmmxt.c"
> @@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
> /* See arch/arm.h. */
>
> target_desc *
> -arm_create_target_description (arm_fp_type fp_type)
> +arm_create_target_description (arm_fp_type fp_type, bool tls)
> {
> target_desc_up tdesc = allocate_target_description ();
>
> @@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type fp_type)
> error (_("Invalid Arm FP type: %d"), fp_type);
> }
>
> + if (tls)
> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
> +
> return tdesc.release ();
> }
>
> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
> index f75470e7572..32f29b20d33 100644
> --- a/gdb/arch/arm.h
> +++ b/gdb/arch/arm.h
> @@ -49,6 +49,7 @@ enum gdb_regnum {
> ARM_D0_REGNUM, /* VFP double-precision registers. */
> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
> ARM_FPSCR_REGNUM,
> + ARM_TPIDRURO_REGNUM,
>
> /* Other useful registers. */
> ARM_FP_REGNUM = 11, /* Frame register in ARM code, if used. */
> @@ -65,8 +66,8 @@ enum arm_register_counts {
> ARM_NUM_ARG_REGS = 4,
> /* Number of floating point argument registers. */
> ARM_NUM_FP_ARG_REGS = 4,
> - /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
> - ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
> + /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM + 1. */
> + ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
> };
I'm attempting to move away from these hardcoded register numbers. If
there are optional features, that means ARM_NUM_REGS won't reflect the
reality anymore, and there may be holes in the register numbering.
For example, a bare metal target may not have ARM_TPIDRURO_REGNUM. The
correct way is to account for it dynamically, similar to what we do with
MVE (and to what we do for pauth, MTE and your TLS handling).
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-04-04 8:01 ` Luis Machado
@ 2022-04-12 23:36 ` John Baldwin
2022-04-14 10:23 ` Luis Machado
0 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-04-12 23:36 UTC (permalink / raw)
To: Luis Machado, binutils, gdb-patches
On 4/4/22 1:01 AM, Luis Machado wrote:
> Hi,
>
> On 3/23/22 21:00, John Baldwin wrote:
>> ---
>> gdb/arch/aarch32.c | 2 ++
>> gdb/arch/arm.c | 6 +++++-
>> gdb/arch/arm.h | 7 ++++---
>> gdb/arm-fbsd-tdep.c | 4 ++--
>> gdb/arm-linux-nat.c | 6 +++---
>> gdb/arm-linux-tdep.c | 4 ++--
>> gdb/arm-netbsd-nat.c | 4 ++--
>> gdb/arm-tdep.c | 20 +++++++++++++++-----
>> gdb/arm-tdep.h | 2 +-
>> gdb/features/Makefile | 1 +
>> gdb/features/arm/arm-tls.c | 14 ++++++++++++++
>> gdb/features/arm/arm-tls.xml | 11 +++++++++++
>> 12 files changed, 62 insertions(+), 19 deletions(-)
>> create mode 100644 gdb/features/arm/arm-tls.c
>> create mode 100644 gdb/features/arm/arm-tls.xml
>>
>> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
>> index 0c544d381f1..4d6ffb44a15 100644
>> --- a/gdb/arch/aarch32.c
>> +++ b/gdb/arch/aarch32.c
>> @@ -19,6 +19,7 @@
>> #include "aarch32.h"
>>
>> #include "../features/arm/arm-core.c"
>> +#include "../features/arm/arm-tls.c"
>> #include "../features/arm/arm-vfpv3.c"
>>
>> /* See aarch32.h. */
>> @@ -38,6 +39,7 @@ aarch32_create_target_description ()
>> /* Create a vfpv3 feature, then a blank NEON feature. */
>> regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
>> tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>
>> return tdesc.release ();
>> }
>> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
>> index 126e46a950a..15b600e22f4 100644
>> --- a/gdb/arch/arm.c
>> +++ b/gdb/arch/arm.c
>> @@ -22,6 +22,7 @@
>> #include "arm.h"
>>
>> #include "../features/arm/arm-core.c"
>> +#include "../features/arm/arm-tls.c"
>> #include "../features/arm/arm-vfpv2.c"
>> #include "../features/arm/arm-vfpv3.c"
>> #include "../features/arm/xscale-iwmmxt.c"
>> @@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
>> /* See arch/arm.h. */
>>
>> target_desc *
>> -arm_create_target_description (arm_fp_type fp_type)
>> +arm_create_target_description (arm_fp_type fp_type, bool tls)
>> {
>> target_desc_up tdesc = allocate_target_description ();
>>
>> @@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type fp_type)
>> error (_("Invalid Arm FP type: %d"), fp_type);
>> }
>>
>> + if (tls)
>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>> +
>> return tdesc.release ();
>> }
>>
>> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
>> index f75470e7572..32f29b20d33 100644
>> --- a/gdb/arch/arm.h
>> +++ b/gdb/arch/arm.h
>> @@ -49,6 +49,7 @@ enum gdb_regnum {
>> ARM_D0_REGNUM, /* VFP double-precision registers. */
>> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
>> ARM_FPSCR_REGNUM,
>> + ARM_TPIDRURO_REGNUM,
>>
>> /* Other useful registers. */
>> ARM_FP_REGNUM = 11, /* Frame register in ARM code, if used. */
>> @@ -65,8 +66,8 @@ enum arm_register_counts {
>> ARM_NUM_ARG_REGS = 4,
>> /* Number of floating point argument registers. */
>> ARM_NUM_FP_ARG_REGS = 4,
>> - /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
>> - ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
>> + /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM + 1. */
>> + ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
>> };
>
> I'm attempting to move away from these hardcoded register numbers. If
> there are optional features, that means ARM_NUM_REGS won't reflect the
> reality anymore, and there may be holes in the register numbering.
>
> For example, a bare metal target may not have ARM_TPIDRURO_REGNUM. The
> correct way is to account for it dynamically, similar to what we do with
> MVE (and to what we do for pauth, MTE and your TLS handling).
Note that these constants aren't required for the remote protocol however as
GDB's remote target figures out its own mapping between remote register
numbers and the internal numbers used in target descriptions. Having fixed
values means one can use constant register_map_entry structures that can
be reused (e.g. I often reuse the structures from <foo>-fbsd-tdep.c files
in the <foo>-fbsd-nat.c file to handle a register set in a native target).
Other arches work fine with holes in the register number space (e.g.
x86 uses fixed constants for various optional register sets like the
different sets of vector registers).
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-04-12 23:36 ` John Baldwin
@ 2022-04-14 10:23 ` Luis Machado
2022-04-19 16:18 ` John Baldwin
0 siblings, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-04-14 10:23 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 4/13/22 00:36, John Baldwin wrote:
> On 4/4/22 1:01 AM, Luis Machado wrote:
>> Hi,
>>
>> On 3/23/22 21:00, John Baldwin wrote:
>>> ---
>>> gdb/arch/aarch32.c | 2 ++
>>> gdb/arch/arm.c | 6 +++++-
>>> gdb/arch/arm.h | 7 ++++---
>>> gdb/arm-fbsd-tdep.c | 4 ++--
>>> gdb/arm-linux-nat.c | 6 +++---
>>> gdb/arm-linux-tdep.c | 4 ++--
>>> gdb/arm-netbsd-nat.c | 4 ++--
>>> gdb/arm-tdep.c | 20 +++++++++++++++-----
>>> gdb/arm-tdep.h | 2 +-
>>> gdb/features/Makefile | 1 +
>>> gdb/features/arm/arm-tls.c | 14 ++++++++++++++
>>> gdb/features/arm/arm-tls.xml | 11 +++++++++++
>>> 12 files changed, 62 insertions(+), 19 deletions(-)
>>> create mode 100644 gdb/features/arm/arm-tls.c
>>> create mode 100644 gdb/features/arm/arm-tls.xml
>>>
>>> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
>>> index 0c544d381f1..4d6ffb44a15 100644
>>> --- a/gdb/arch/aarch32.c
>>> +++ b/gdb/arch/aarch32.c
>>> @@ -19,6 +19,7 @@
>>> #include "aarch32.h"
>>> #include "../features/arm/arm-core.c"
>>> +#include "../features/arm/arm-tls.c"
>>> #include "../features/arm/arm-vfpv3.c"
>>> /* See aarch32.h. */
>>> @@ -38,6 +39,7 @@ aarch32_create_target_description ()
>>> /* Create a vfpv3 feature, then a blank NEON feature. */
>>> regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
>>> tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>> return tdesc.release ();
>>> }
>>> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
>>> index 126e46a950a..15b600e22f4 100644
>>> --- a/gdb/arch/arm.c
>>> +++ b/gdb/arch/arm.c
>>> @@ -22,6 +22,7 @@
>>> #include "arm.h"
>>> #include "../features/arm/arm-core.c"
>>> +#include "../features/arm/arm-tls.c"
>>> #include "../features/arm/arm-vfpv2.c"
>>> #include "../features/arm/arm-vfpv3.c"
>>> #include "../features/arm/xscale-iwmmxt.c"
>>> @@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache,
>>> unsigned long inst,
>>> /* See arch/arm.h. */
>>> target_desc *
>>> -arm_create_target_description (arm_fp_type fp_type)
>>> +arm_create_target_description (arm_fp_type fp_type, bool tls)
>>> {
>>> target_desc_up tdesc = allocate_target_description ();
>>> @@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type fp_type)
>>> error (_("Invalid Arm FP type: %d"), fp_type);
>>> }
>>> + if (tls)
>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>> +
>>> return tdesc.release ();
>>> }
>>> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
>>> index f75470e7572..32f29b20d33 100644
>>> --- a/gdb/arch/arm.h
>>> +++ b/gdb/arch/arm.h
>>> @@ -49,6 +49,7 @@ enum gdb_regnum {
>>> ARM_D0_REGNUM, /* VFP double-precision registers. */
>>> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
>>> ARM_FPSCR_REGNUM,
>>> + ARM_TPIDRURO_REGNUM,
>>> /* Other useful registers. */
>>> ARM_FP_REGNUM = 11, /* Frame register in ARM code, if
>>> used. */
>>> @@ -65,8 +66,8 @@ enum arm_register_counts {
>>> ARM_NUM_ARG_REGS = 4,
>>> /* Number of floating point argument registers. */
>>> ARM_NUM_FP_ARG_REGS = 4,
>>> - /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
>>> - ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
>>> + /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM + 1. */
>>> + ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
>>> };
>>
>> I'm attempting to move away from these hardcoded register numbers. If
>> there are optional features, that means ARM_NUM_REGS won't reflect the
>> reality anymore, and there may be holes in the register numbering.
>>
>> For example, a bare metal target may not have ARM_TPIDRURO_REGNUM. The
>> correct way is to account for it dynamically, similar to what we do with
>> MVE (and to what we do for pauth, MTE and your TLS handling).
>
> Note that these constants aren't required for the remote protocol
> however as
> GDB's remote target figures out its own mapping between remote register
> numbers and the internal numbers used in target descriptions. Having fixed
> values means one can use constant register_map_entry structures that can
> be reused (e.g. I often reuse the structures from <foo>-fbsd-tdep.c files
> in the <foo>-fbsd-nat.c file to handle a register set in a native target).
>
> Other arches work fine with holes in the register number space (e.g.
> x86 uses fixed constants for various optional register sets like the
> different sets of vector registers).
>
Indeed. It is true that these holes have no negative impact other than
"maint print registers" showing empty entries and the number of
registers being slightly misleading.
The register_map_entry structures work nicely, but they don't provide a
good way to track pseudo registers alongside the real feature registers.
Having more clear boundaries between each feature and the registers and
pseudo-registers in them looks cleaner to me.
Some time ago I disentangled the handling of pseudo registers for 32-bit
arm, as it started to get a bit chaotic.
Most of the feature handling tends to happen in generic arm-tdep, so
that only needs to change once.
Unfortunately the linux and fbsd layers for 32-bit arm work in slightly
different ways. Ideally they would share more code and we'd unify some
register handling. But we're not there yet.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-04-14 10:23 ` Luis Machado
@ 2022-04-19 16:18 ` John Baldwin
2022-04-20 6:59 ` Luis Machado
0 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-04-19 16:18 UTC (permalink / raw)
To: Luis Machado, binutils, gdb-patches
On 4/14/22 3:23 AM, Luis Machado wrote:
> On 4/13/22 00:36, John Baldwin wrote:
>> On 4/4/22 1:01 AM, Luis Machado wrote:
>>> Hi,
>>>
>>> On 3/23/22 21:00, John Baldwin wrote:
>>>> ---
>>>> gdb/arch/aarch32.c | 2 ++
>>>> gdb/arch/arm.c | 6 +++++-
>>>> gdb/arch/arm.h | 7 ++++---
>>>> gdb/arm-fbsd-tdep.c | 4 ++--
>>>> gdb/arm-linux-nat.c | 6 +++---
>>>> gdb/arm-linux-tdep.c | 4 ++--
>>>> gdb/arm-netbsd-nat.c | 4 ++--
>>>> gdb/arm-tdep.c | 20 +++++++++++++++-----
>>>> gdb/arm-tdep.h | 2 +-
>>>> gdb/features/Makefile | 1 +
>>>> gdb/features/arm/arm-tls.c | 14 ++++++++++++++
>>>> gdb/features/arm/arm-tls.xml | 11 +++++++++++
>>>> 12 files changed, 62 insertions(+), 19 deletions(-)
>>>> create mode 100644 gdb/features/arm/arm-tls.c
>>>> create mode 100644 gdb/features/arm/arm-tls.xml
>>>>
>>>> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
>>>> index 0c544d381f1..4d6ffb44a15 100644
>>>> --- a/gdb/arch/aarch32.c
>>>> +++ b/gdb/arch/aarch32.c
>>>> @@ -19,6 +19,7 @@
>>>> #include "aarch32.h"
>>>> #include "../features/arm/arm-core.c"
>>>> +#include "../features/arm/arm-tls.c"
>>>> #include "../features/arm/arm-vfpv3.c"
>>>> /* See aarch32.h. */
>>>> @@ -38,6 +39,7 @@ aarch32_create_target_description ()
>>>> /* Create a vfpv3 feature, then a blank NEON feature. */
>>>> regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
>>>> tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
>>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>>> return tdesc.release ();
>>>> }
>>>> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
>>>> index 126e46a950a..15b600e22f4 100644
>>>> --- a/gdb/arch/arm.c
>>>> +++ b/gdb/arch/arm.c
>>>> @@ -22,6 +22,7 @@
>>>> #include "arm.h"
>>>> #include "../features/arm/arm-core.c"
>>>> +#include "../features/arm/arm-tls.c"
>>>> #include "../features/arm/arm-vfpv2.c"
>>>> #include "../features/arm/arm-vfpv3.c"
>>>> #include "../features/arm/xscale-iwmmxt.c"
>>>> @@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache,
>>>> unsigned long inst,
>>>> /* See arch/arm.h. */
>>>> target_desc *
>>>> -arm_create_target_description (arm_fp_type fp_type)
>>>> +arm_create_target_description (arm_fp_type fp_type, bool tls)
>>>> {
>>>> target_desc_up tdesc = allocate_target_description ();
>>>> @@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type fp_type)
>>>> error (_("Invalid Arm FP type: %d"), fp_type);
>>>> }
>>>> + if (tls)
>>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>>> +
>>>> return tdesc.release ();
>>>> }
>>>> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
>>>> index f75470e7572..32f29b20d33 100644
>>>> --- a/gdb/arch/arm.h
>>>> +++ b/gdb/arch/arm.h
>>>> @@ -49,6 +49,7 @@ enum gdb_regnum {
>>>> ARM_D0_REGNUM, /* VFP double-precision registers. */
>>>> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
>>>> ARM_FPSCR_REGNUM,
>>>> + ARM_TPIDRURO_REGNUM,
>>>> /* Other useful registers. */
>>>> ARM_FP_REGNUM = 11, /* Frame register in ARM code, if
>>>> used. */
>>>> @@ -65,8 +66,8 @@ enum arm_register_counts {
>>>> ARM_NUM_ARG_REGS = 4,
>>>> /* Number of floating point argument registers. */
>>>> ARM_NUM_FP_ARG_REGS = 4,
>>>> - /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
>>>> - ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
>>>> + /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM + 1. */
>>>> + ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
>>>> };
>>>
>>> I'm attempting to move away from these hardcoded register numbers. If
>>> there are optional features, that means ARM_NUM_REGS won't reflect the
>>> reality anymore, and there may be holes in the register numbering.
>>>
>>> For example, a bare metal target may not have ARM_TPIDRURO_REGNUM. The
>>> correct way is to account for it dynamically, similar to what we do with
>>> MVE (and to what we do for pauth, MTE and your TLS handling).
>>
>> Note that these constants aren't required for the remote protocol
>> however as
>> GDB's remote target figures out its own mapping between remote register
>> numbers and the internal numbers used in target descriptions. Having fixed
>> values means one can use constant register_map_entry structures that can
>> be reused (e.g. I often reuse the structures from <foo>-fbsd-tdep.c files
>> in the <foo>-fbsd-nat.c file to handle a register set in a native target).
>>
>> Other arches work fine with holes in the register number space (e.g.
>> x86 uses fixed constants for various optional register sets like the
>> different sets of vector registers).
>>
>
> Indeed. It is true that these holes have no negative impact other than
> "maint print registers" showing empty entries and the number of
> registers being slightly misleading.
>
> The register_map_entry structures work nicely, but they don't provide a
> good way to track pseudo registers alongside the real feature registers.
> Having more clear boundaries between each feature and the registers and
> pseudo-registers in them looks cleaner to me.
>
> Some time ago I disentangled the handling of pseudo registers for 32-bit
> arm, as it started to get a bit chaotic.
>
> Most of the feature handling tends to happen in generic arm-tdep, so
> that only needs to change once.
>
> Unfortunately the linux and fbsd layers for 32-bit arm work in slightly
> different ways. Ideally they would share more code and we'd unify some
> register handling. But we're not there yet.
So I'm not quite sure how to parse your reply in terms of would you rather
me make the TLS register numbers dynamic on both arm and aarch64 with a
field in the tdep structure holding the number, or fixed values as the V2
patch series currently does? Also, a question I have in the V2 series is
if NT_ARM_TLS is old enough to be present on all supported Linux aarch64
kernels or if it needs some runtime checks to decide if the register is
present or not for a Linux kernel.
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15.
2022-04-19 16:18 ` John Baldwin
@ 2022-04-20 6:59 ` Luis Machado
0 siblings, 0 replies; 26+ messages in thread
From: Luis Machado @ 2022-04-20 6:59 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 4/19/22 17:18, John Baldwin wrote:
> On 4/14/22 3:23 AM, Luis Machado wrote:
>> On 4/13/22 00:36, John Baldwin wrote:
>>> On 4/4/22 1:01 AM, Luis Machado wrote:
>>>> Hi,
>>>>
>>>> On 3/23/22 21:00, John Baldwin wrote:
>>>>> ---
>>>>> gdb/arch/aarch32.c | 2 ++
>>>>> gdb/arch/arm.c | 6 +++++-
>>>>> gdb/arch/arm.h | 7 ++++---
>>>>> gdb/arm-fbsd-tdep.c | 4 ++--
>>>>> gdb/arm-linux-nat.c | 6 +++---
>>>>> gdb/arm-linux-tdep.c | 4 ++--
>>>>> gdb/arm-netbsd-nat.c | 4 ++--
>>>>> gdb/arm-tdep.c | 20 +++++++++++++++-----
>>>>> gdb/arm-tdep.h | 2 +-
>>>>> gdb/features/Makefile | 1 +
>>>>> gdb/features/arm/arm-tls.c | 14 ++++++++++++++
>>>>> gdb/features/arm/arm-tls.xml | 11 +++++++++++
>>>>> 12 files changed, 62 insertions(+), 19 deletions(-)
>>>>> create mode 100644 gdb/features/arm/arm-tls.c
>>>>> create mode 100644 gdb/features/arm/arm-tls.xml
>>>>>
>>>>> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
>>>>> index 0c544d381f1..4d6ffb44a15 100644
>>>>> --- a/gdb/arch/aarch32.c
>>>>> +++ b/gdb/arch/aarch32.c
>>>>> @@ -19,6 +19,7 @@
>>>>> #include "aarch32.h"
>>>>> #include "../features/arm/arm-core.c"
>>>>> +#include "../features/arm/arm-tls.c"
>>>>> #include "../features/arm/arm-vfpv3.c"
>>>>> /* See aarch32.h. */
>>>>> @@ -38,6 +39,7 @@ aarch32_create_target_description ()
>>>>> /* Create a vfpv3 feature, then a blank NEON feature. */
>>>>> regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum);
>>>>> tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon");
>>>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>>>> return tdesc.release ();
>>>>> }
>>>>> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
>>>>> index 126e46a950a..15b600e22f4 100644
>>>>> --- a/gdb/arch/arm.c
>>>>> +++ b/gdb/arch/arm.c
>>>>> @@ -22,6 +22,7 @@
>>>>> #include "arm.h"
>>>>> #include "../features/arm/arm-core.c"
>>>>> +#include "../features/arm/arm-tls.c"
>>>>> #include "../features/arm/arm-vfpv2.c"
>>>>> #include "../features/arm/arm-vfpv3.c"
>>>>> #include "../features/arm/xscale-iwmmxt.c"
>>>>> @@ -373,7 +374,7 @@ shifted_reg_val (struct regcache *regcache,
>>>>> unsigned long inst,
>>>>> /* See arch/arm.h. */
>>>>> target_desc *
>>>>> -arm_create_target_description (arm_fp_type fp_type)
>>>>> +arm_create_target_description (arm_fp_type fp_type, bool tls)
>>>>> {
>>>>> target_desc_up tdesc = allocate_target_description ();
>>>>> @@ -409,6 +410,9 @@ arm_create_target_description (arm_fp_type
>>>>> fp_type)
>>>>> error (_("Invalid Arm FP type: %d"), fp_type);
>>>>> }
>>>>> + if (tls)
>>>>> + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum);
>>>>> +
>>>>> return tdesc.release ();
>>>>> }
>>>>> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
>>>>> index f75470e7572..32f29b20d33 100644
>>>>> --- a/gdb/arch/arm.h
>>>>> +++ b/gdb/arch/arm.h
>>>>> @@ -49,6 +49,7 @@ enum gdb_regnum {
>>>>> ARM_D0_REGNUM, /* VFP double-precision registers. */
>>>>> ARM_D31_REGNUM = ARM_D0_REGNUM + 31,
>>>>> ARM_FPSCR_REGNUM,
>>>>> + ARM_TPIDRURO_REGNUM,
>>>>> /* Other useful registers. */
>>>>> ARM_FP_REGNUM = 11, /* Frame register in ARM code, if
>>>>> used. */
>>>>> @@ -65,8 +66,8 @@ enum arm_register_counts {
>>>>> ARM_NUM_ARG_REGS = 4,
>>>>> /* Number of floating point argument registers. */
>>>>> ARM_NUM_FP_ARG_REGS = 4,
>>>>> - /* Number of registers (old, defined as ARM_FPSCR_REGNUM + 1. */
>>>>> - ARM_NUM_REGS = ARM_FPSCR_REGNUM + 1
>>>>> + /* Number of registers (old, defined as ARM_TPIDRURO_REGNUM +
>>>>> 1. */
>>>>> + ARM_NUM_REGS = ARM_TPIDRURO_REGNUM + 1
>>>>> };
>>>>
>>>> I'm attempting to move away from these hardcoded register numbers. If
>>>> there are optional features, that means ARM_NUM_REGS won't reflect the
>>>> reality anymore, and there may be holes in the register numbering.
>>>>
>>>> For example, a bare metal target may not have ARM_TPIDRURO_REGNUM. The
>>>> correct way is to account for it dynamically, similar to what we do
>>>> with
>>>> MVE (and to what we do for pauth, MTE and your TLS handling).
>>>
>>> Note that these constants aren't required for the remote protocol
>>> however as
>>> GDB's remote target figures out its own mapping between remote register
>>> numbers and the internal numbers used in target descriptions. Having
>>> fixed
>>> values means one can use constant register_map_entry structures that can
>>> be reused (e.g. I often reuse the structures from <foo>-fbsd-tdep.c
>>> files
>>> in the <foo>-fbsd-nat.c file to handle a register set in a native
>>> target).
>>>
>>> Other arches work fine with holes in the register number space (e.g.
>>> x86 uses fixed constants for various optional register sets like the
>>> different sets of vector registers).
>>>
>>
>> Indeed. It is true that these holes have no negative impact other than
>> "maint print registers" showing empty entries and the number of
>> registers being slightly misleading.
>>
>> The register_map_entry structures work nicely, but they don't provide a
>> good way to track pseudo registers alongside the real feature registers.
>> Having more clear boundaries between each feature and the registers and
>> pseudo-registers in them looks cleaner to me.
>>
>> Some time ago I disentangled the handling of pseudo registers for 32-bit
>> arm, as it started to get a bit chaotic.
>>
>> Most of the feature handling tends to happen in generic arm-tdep, so
>> that only needs to change once.
>>
>> Unfortunately the linux and fbsd layers for 32-bit arm work in slightly
>> different ways. Ideally they would share more code and we'd unify some
>> register handling. But we're not there yet.
>
> So I'm not quite sure how to parse your reply in terms of would you rather
> me make the TLS register numbers dynamic on both arm and aarch64 with a
> field in the tdep structure holding the number, or fixed values as the V2
> patch series currently does? Also, a question I have in the V2 series is
> if NT_ARM_TLS is old enough to be present on all supported Linux aarch64
> kernels or if it needs some runtime checks to decide if the register is
> present or not for a Linux kernel.
>
I'd rather have the dynamic register numbering in the generic files for
now. In the future I'd like us to converge on a unified form of handling
these as to avoid having the linux and fbsd files using different
mechanisms for no good reason.
As for NT_ARM_TLS, it's been in the Linux kernel since 2012:
commit 478fcb2cdb2351dcfc3fb23f42d76f4436ee4149
Author: Will Deacon <will.deacon@arm.com>
Date: Mon Mar 5 11:49:33 2012 +0000
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/12] Read the tpidruro register from NT_ARM_TLS core dump notes on FreeBSD/arm.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (3 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15 John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-23 21:00 ` [PATCH 06/12] Support TLS variables " John Baldwin
` (3 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
---
gdb/arm-fbsd-nat.c | 2 +-
gdb/arm-fbsd-tdep.c | 28 ++++++++++++++++++++++------
gdb/arm-fbsd-tdep.h | 6 +++++-
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
index 3106d73cc3a..c32924de735 100644
--- a/gdb/arm-fbsd-nat.c
+++ b/gdb/arm-fbsd-nat.c
@@ -72,7 +72,7 @@ arm_fbsd_nat_target::read_description ()
{
const struct target_desc *desc;
- desc = arm_fbsd_read_description_auxv (this);
+ desc = arm_fbsd_read_description_auxv (this, false);
if (desc == NULL)
desc = this->beneath ()->read_description ();
return desc;
diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c
index 06745a36186..e11b17664eb 100644
--- a/gdb/arm-fbsd-tdep.c
+++ b/gdb/arm-fbsd-tdep.c
@@ -44,6 +44,12 @@ static const struct regcache_map_entry arm_fbsd_gregmap[] =
{ 0 }
};
+static const struct regcache_map_entry arm_fbsd_tlsregmap[] =
+ {
+ { 1, ARM_TPIDRURO_REGNUM, 4 },
+ { 0 }
+ };
+
static const struct regcache_map_entry arm_fbsd_vfpregmap[] =
{
{ 32, ARM_D0_REGNUM, 8 }, /* d0 ... d31 */
@@ -144,6 +150,12 @@ const struct regset arm_fbsd_gregset =
regcache_supply_regset, regcache_collect_regset
};
+const struct regset arm_fbsd_tlsregset =
+ {
+ arm_fbsd_tlsregmap,
+ regcache_supply_regset, regcache_collect_regset
+ };
+
const struct regset arm_fbsd_vfpregset =
{
arm_fbsd_vfpregmap,
@@ -162,6 +174,8 @@ arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
cb (".reg", ARM_FBSD_SIZEOF_GREGSET, ARM_FBSD_SIZEOF_GREGSET,
&arm_fbsd_gregset, NULL, cb_data);
+ cb (".reg-aarch-tls", ARM_FBSD_SIZEOF_TLSREGSET, ARM_FBSD_SIZEOF_TLSREGSET,
+ &arm_fbsd_tlsregset, NULL, cb_data);
/* While FreeBSD/arm cores do contain a NT_FPREGSET / ".reg2"
register set, it is not populated with register values by the
@@ -175,12 +189,12 @@ arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
vector. */
const struct target_desc *
-arm_fbsd_read_description_auxv (struct target_ops *target)
+arm_fbsd_read_description_auxv (struct target_ops *target, bool tls)
{
CORE_ADDR arm_hwcap = 0;
if (target_auxv_search (target, AT_FREEBSD_HWCAP, &arm_hwcap) != 1)
- return nullptr;
+ return arm_read_description (ARM_FP_TYPE_NONE, tls);
if (arm_hwcap & HWCAP_VFP)
{
@@ -188,12 +202,12 @@ arm_fbsd_read_description_auxv (struct target_ops *target)
return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPD32))
== (HWCAP_VFPv3 | HWCAP_VFPD32))
- return arm_read_description (ARM_FP_TYPE_VFPV3, false);
+ return arm_read_description (ARM_FP_TYPE_VFPV3, tls);
else
- return arm_read_description (ARM_FP_TYPE_VFPV2, false);
+ return arm_read_description (ARM_FP_TYPE_VFPV2, tls);
}
- return nullptr;
+ return arm_read_description (ARM_FP_TYPE_NONE, tls);
}
/* Implement the "core_read_description" gdbarch method. */
@@ -203,7 +217,9 @@ arm_fbsd_core_read_description (struct gdbarch *gdbarch,
struct target_ops *target,
bfd *abfd)
{
- return arm_fbsd_read_description_auxv (target);
+ asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
+
+ return arm_fbsd_read_description_auxv (target, tls != nullptr);
}
/* Implement the 'init_osabi' method of struct gdb_osabi_handler. */
diff --git a/gdb/arm-fbsd-tdep.h b/gdb/arm-fbsd-tdep.h
index 633dafad75d..00b55cf5bd0 100644
--- a/gdb/arm-fbsd-tdep.h
+++ b/gdb/arm-fbsd-tdep.h
@@ -26,11 +26,15 @@
PC, and CPSR registers. */
#define ARM_FBSD_SIZEOF_GREGSET (17 * 4)
+/* The TLS regset consists of a single register. */
+#define ARM_FBSD_SIZEOF_TLSREGSET (4)
+
/* The VFP regset consists of 32 D registers plus FPSCR, and the whole
structure is padded to 64-bit alignment. */
#define ARM_FBSD_SIZEOF_VFPREGSET (33 * 8)
extern const struct regset arm_fbsd_gregset;
+extern const struct regset arm_fbsd_tlsregset;
extern const struct regset arm_fbsd_vfpregset;
/* Flags passed in AT_HWCAP. */
@@ -40,6 +44,6 @@ extern const struct regset arm_fbsd_vfpregset;
#define HWCAP_VFPD32 0x00080000
extern const struct target_desc *
-arm_fbsd_read_description_auxv (struct target_ops *target);
+arm_fbsd_read_description_auxv (struct target_ops *target, bool tls);
#endif /* ARM_FBSD_TDEP_H */
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 06/12] Support TLS variables on FreeBSD/arm.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (4 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 05/12] Read the tpidruro register from NT_ARM_TLS core dump notes on FreeBSD/arm John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-23 21:00 ` [PATCH 07/12] Fetch the NT_ARM_TLS register set for native FreeBSD/arm processes John Baldwin
` (2 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
Derive the pointer to the DTV array from the tpidruro register.
---
gdb/arm-fbsd-tdep.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c
index e11b17664eb..97d41a77f40 100644
--- a/gdb/arm-fbsd-tdep.c
+++ b/gdb/arm-fbsd-tdep.c
@@ -27,6 +27,7 @@
#include "auxv.h"
#include "fbsd-tdep.h"
#include "gdbcore.h"
+#include "inferior.h"
#include "osabi.h"
#include "solib-svr4.h"
#include "trad-frame.h"
@@ -222,6 +223,29 @@ arm_fbsd_core_read_description (struct gdbarch *gdbarch,
return arm_fbsd_read_description_auxv (target, tls != nullptr);
}
+/* Implement the get_thread_local_address gdbarch method. */
+
+static CORE_ADDR
+arm_fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
+ CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+ struct regcache *regcache;
+
+ regcache = get_thread_arch_regcache (current_inferior ()->process_target (),
+ ptid, gdbarch);
+
+ target_fetch_registers (regcache, ARM_TPIDRURO_REGNUM);
+
+ ULONGEST tpidruro;
+ if (regcache->cooked_read (ARM_TPIDRURO_REGNUM, &tpidruro) != REG_VALID)
+ error (_("Unable to fetch %%tpidruro"));
+
+ /* %tpidruro points to the TCB whose first member is the dtv
+ pointer. */
+ CORE_ADDR dtv_addr = tpidruro;
+ return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
+}
+
/* Implement the 'init_osabi' method of struct gdb_osabi_handler. */
static void
@@ -247,6 +271,11 @@ arm_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
(gdbarch, arm_fbsd_iterate_over_regset_sections);
set_gdbarch_core_read_description (gdbarch, arm_fbsd_core_read_description);
+ set_gdbarch_fetch_tls_load_module_address (gdbarch,
+ svr4_fetch_objfile_link_map);
+ set_gdbarch_get_thread_local_address (gdbarch,
+ arm_fbsd_get_thread_local_address);
+
/* Single stepping. */
set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
}
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 07/12] Fetch the NT_ARM_TLS register set for native FreeBSD/arm processes.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (5 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 06/12] Support TLS variables " John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-23 21:00 ` [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register John Baldwin
2022-03-23 21:00 ` [PATCH 09/12] Read the tpidr register from NT_ARM_TLS core dump notes on FreeBSD/Aarch64 John Baldwin
8 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
This permits resolving TLS variables.
---
gdb/arm-fbsd-nat.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
index c32924de735..6b5befc3a05 100644
--- a/gdb/arm-fbsd-nat.c
+++ b/gdb/arm-fbsd-nat.c
@@ -18,8 +18,11 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "inferior.h"
#include "target.h"
+#include "elf/common.h"
+
#include <sys/types.h>
#include <sys/ptrace.h>
#include <machine/reg.h>
@@ -49,6 +52,9 @@ arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
fetch_register_set<struct vfpreg> (regcache, regnum, PT_GETVFPREGS,
&arm_fbsd_vfpregset);
#endif
+#ifdef PT_GETREGSET
+ fetch_regset<uint32_t> (regcache, regnum, NT_ARM_TLS, &arm_fbsd_tlsregset);
+#endif
}
/* Store register REGNUM back into the inferior. If REGNUM is -1, do
@@ -63,6 +69,9 @@ arm_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
store_register_set<struct vfpreg> (regcache, regnum, PT_GETVFPREGS,
PT_SETVFPREGS, &arm_fbsd_vfpregset);
#endif
+#ifdef PT_GETREGSET
+ store_regset<uint32_t> (regcache, regnum, NT_ARM_TLS, &arm_fbsd_tlsregset);
+#endif
}
/* Implement the to_read_description method. */
@@ -71,8 +80,12 @@ const struct target_desc *
arm_fbsd_nat_target::read_description ()
{
const struct target_desc *desc;
+ bool tls = false;
- desc = arm_fbsd_read_description_auxv (this, false);
+#ifdef PT_GETREGSET
+ tls = have_regset (inferior_ptid, NT_ARM_TLS) != 0;
+#endif
+ desc = arm_fbsd_read_description_auxv (this, tls);
if (desc == NULL)
desc = this->beneath ()->read_description ();
return desc;
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (6 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 07/12] Fetch the NT_ARM_TLS register set for native FreeBSD/arm processes John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
2022-03-28 10:16 ` Luis Machado
2022-05-03 21:14 ` Luis Machado
2022-03-23 21:00 ` [PATCH 09/12] Read the tpidr register from NT_ARM_TLS core dump notes on FreeBSD/Aarch64 John Baldwin
8 siblings, 2 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
---
gdb/aarch64-linux-nat.c | 3 ++-
gdb/aarch64-linux-tdep.c | 2 +-
gdb/aarch64-tdep.c | 42 ++++++++++++++++++++++++++------
gdb/aarch64-tdep.h | 10 +++++++-
gdb/arch/aarch64.c | 7 +++++-
gdb/arch/aarch64.h | 9 +++++--
gdb/features/Makefile | 1 +
gdb/features/aarch64-tls.c | 14 +++++++++++
gdb/features/aarch64-tls.xml | 12 +++++++++
gdbserver/linux-aarch64-tdesc.cc | 2 +-
gdbserver/netbsd-aarch64-low.cc | 2 +-
11 files changed, 88 insertions(+), 16 deletions(-)
create mode 100644 gdb/features/aarch64-tls.c
create mode 100644 gdb/features/aarch64-tls.xml
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 7bb82d17cc8..4da274c285a 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
bool mte_p = hwcap2 & HWCAP2_MTE;
- return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p);
+ return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p,
+ false);
}
/* Convert a native/host siginfo object, into/from the siginfo in the
diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index cb132d5a540..f5aac7bc0b4 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
bool mte_p = hwcap2 & HWCAP2_MTE;
return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
- pauth_p, mte_p);
+ pauth_p, mte_p, false);
}
/* Implementation of `gdbarch_stap_is_single_operand', as defined in
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index b714f6194b6..38c5c9e0e00 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -58,7 +58,7 @@
#define HA_MAX_NUM_FLDS 4
/* All possible aarch64 target descriptors. */
-static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */];
+static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */][2 /* tls */];
/* The standard register names, and all the valid aliases for them. */
static const struct
@@ -178,6 +178,12 @@ static const char *const aarch64_mte_register_names[] =
"tag_ctl"
};
+static const char *const aarch64_tls_register_names[] =
+{
+ /* Thread/Process ID Register. */
+ "tpidr"
+};
+
/* AArch64 prologue cache structure. */
struct aarch64_prologue_cache
{
@@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch)
If VQ is zero then it is assumed SVE is not supported.
(It is not possible to set VQ to zero on an SVE system).
- MTE_P indicates the presence of the Memory Tagging Extension feature. */
+ MTE_P indicates the presence of the Memory Tagging Extension feature.
+
+ TLS_P indicates the presence of the Thread Local Storage feature. */
const target_desc *
-aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
+aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p, bool tls_p)
{
if (vq > AARCH64_MAX_SVE_VQ)
error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
AARCH64_MAX_SVE_VQ);
- struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
+ struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
if (tdesc == NULL)
{
- tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
- tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
+ tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, tls_p);
+ tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
}
return tdesc;
@@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
bool valid_p = true;
int i, num_regs = 0, num_pseudo_regs = 0;
int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
- int first_mte_regnum = -1;
+ int first_mte_regnum = -1, first_tls_regnum = -1;
/* Use the vector length passed via the target info. Here -1 is used for no
SVE, and 0 is unset. If unset then use the vector length from the existing
@@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
value. */
const struct target_desc *tdesc = info.target_desc;
if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq (tdesc))
- tdesc = aarch64_read_description (vq, false, false);
+ tdesc = aarch64_read_description (vq, false, false, false);
gdb_assert (tdesc);
feature_core = tdesc_find_feature (tdesc,"org.gnu.gdb.aarch64.core");
@@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
const struct tdesc_feature *feature_mte
= tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
+ const struct tdesc_feature *feature_tls
+ = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
if (feature_core == nullptr)
return nullptr;
@@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
num_regs += i;
}
+ /* Add the TLS register. */
+ if (feature_tls != nullptr)
+ {
+ first_tls_regnum = num_regs;
+
+ /* Validate the descriptor provides the mandatory MTE registers and
+ allocate their numbers. */
+ for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
+ valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
+ first_tls_regnum + i,
+ aarch64_tls_register_names[i]);
+
+ num_regs += i;
+ }
+
if (!valid_p)
return nullptr;
@@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
: pauth_ra_state_offset + num_regs;
tdep->mte_reg_base = first_mte_regnum;
+ tdep->tls_reg_base = first_tls_regnum;
set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index 60a9d5a29c2..092586d4ee3 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
{
return mte_reg_base != -1;
}
+
+ /* TLS register. This is -1 if the TLS register is not available. */
+ int tls_reg_base = 0;
+
+ bool has_tls() const
+ {
+ return tls_reg_base != -1;
+ }
};
const target_desc *aarch64_read_description (uint64_t vq, bool pauth_p,
- bool mte_p);
+ bool mte_p, bool tls_p);
extern int aarch64_process_record (struct gdbarch *gdbarch,
struct regcache *regcache, CORE_ADDR addr);
diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
index 485d667ccde..733a3fd6d2a 100644
--- a/gdb/arch/aarch64.c
+++ b/gdb/arch/aarch64.c
@@ -24,11 +24,13 @@
#include "../features/aarch64-sve.c"
#include "../features/aarch64-pauth.c"
#include "../features/aarch64-mte.c"
+#include "../features/aarch64-tls.c"
/* See arch/aarch64.h. */
target_desc *
-aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
+aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p,
+ bool tls_p)
{
target_desc_up tdesc = allocate_target_description ();
@@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
if (mte_p)
regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
+ if (tls_p)
+ regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
+
return tdesc.release ();
}
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index e416e346e9a..79821666ce6 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -29,6 +29,7 @@ struct aarch64_features
bool sve = false;
bool pauth = false;
bool mte = false;
+ bool tls = false;
};
/* Create the aarch64 target description. A non zero VQ value indicates both
@@ -36,10 +37,12 @@ struct aarch64_features
an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH
feature.
- MTE_P indicates the presence of the Memory Tagging Extension feature. */
+ MTE_P indicates the presence of the Memory Tagging Extension feature.
+
+ TLS_P indicates the presence of the Thread Local Storage feature. */
target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p,
- bool mte_p);
+ bool mte_p, bool tls_p);
/* Register numbers of various important registers.
Note that on SVE, the Z registers reuse the V register numbers and the V
@@ -84,6 +87,8 @@ enum aarch64_regnum
#define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
#define AARCH64_PAUTH_REGS_SIZE (16)
+#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
+
#define AARCH64_X_REGS_NUM 31
#define AARCH64_V_REGS_NUM 32
#define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 4b09819389a..946ec983df5 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
aarch64-fpu.xml \
aarch64-pauth.xml \
aarch64-mte.xml \
+ aarch64-tls.xml \
arc/v1-core.xml \
arc/v1-aux.xml \
arc/v2-core.xml \
diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
new file mode 100644
index 00000000000..30d730dffba
--- /dev/null
+++ b/gdb/features/aarch64-tls.c
@@ -0,0 +1,14 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: aarch64-tls.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_aarch64_tls (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
+ tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64, "data_ptr");
+ return regnum;
+}
diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
new file mode 100644
index 00000000000..48e0e9a6030
--- /dev/null
+++ b/gdb/features/aarch64-tls.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
+ Contributed by ARM Ltd.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.aarch64.tls">
+ <reg name="tpidr" bitsize="64" type="data_ptr"/>
+</feature>
diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
index e982ab85067..14d6a4f80eb 100644
--- a/gdbserver/linux-aarch64-tdesc.cc
+++ b/gdbserver/linux-aarch64-tdesc.cc
@@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool pauth_p, bool mte_p)
if (tdesc == NULL)
{
- tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
+ tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, false);
static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
static const char *expedite_regs_aarch64_sve[] = { "x29", "sp", "pc",
diff --git a/gdbserver/netbsd-aarch64-low.cc b/gdbserver/netbsd-aarch64-low.cc
index 202bf1cdac6..b371e599232 100644
--- a/gdbserver/netbsd-aarch64-low.cc
+++ b/gdbserver/netbsd-aarch64-low.cc
@@ -96,7 +96,7 @@ void
netbsd_aarch64_target::low_arch_setup ()
{
target_desc *tdesc
- = aarch64_create_target_description (0, false);
+ = aarch64_create_target_description (0, false, false, false);
static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
init_target_desc (tdesc, expedite_regs_aarch64);
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-03-23 21:00 ` [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register John Baldwin
@ 2022-03-28 10:16 ` Luis Machado
2022-04-01 23:30 ` John Baldwin
2022-05-03 21:14 ` Luis Machado
1 sibling, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-03-28 10:16 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
Hi John,
On 3/23/22 21:00, John Baldwin wrote:
> ---
> gdb/aarch64-linux-nat.c | 3 ++-
> gdb/aarch64-linux-tdep.c | 2 +-
> gdb/aarch64-tdep.c | 42 ++++++++++++++++++++++++++------
> gdb/aarch64-tdep.h | 10 +++++++-
> gdb/arch/aarch64.c | 7 +++++-
> gdb/arch/aarch64.h | 9 +++++--
> gdb/features/Makefile | 1 +
> gdb/features/aarch64-tls.c | 14 +++++++++++
> gdb/features/aarch64-tls.xml | 12 +++++++++
> gdbserver/linux-aarch64-tdesc.cc | 2 +-
> gdbserver/netbsd-aarch64-low.cc | 2 +-
> 11 files changed, 88 insertions(+), 16 deletions(-)
> create mode 100644 gdb/features/aarch64-tls.c
> create mode 100644 gdb/features/aarch64-tls.xml
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 7bb82d17cc8..4da274c285a 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
> bool mte_p = hwcap2 & HWCAP2_MTE;
>
> - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p);
> + return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p,
> + false);
This is getting a bit ugly. We should pass a single struct that contains
all available features eventually. But it should be OK right now.
> }
>
> /* Convert a native/host siginfo object, into/from the siginfo in the
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index cb132d5a540..f5aac7bc0b4 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
> bool mte_p = hwcap2 & HWCAP2_MTE;
> return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
> - pauth_p, mte_p);
> + pauth_p, mte_p, false);
> }
>
> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b714f6194b6..38c5c9e0e00 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -58,7 +58,7 @@
> #define HA_MAX_NUM_FLDS 4
>
> /* All possible aarch64 target descriptors. */
> -static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */];
> +static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */][2 /* tls */];
>
> /* The standard register names, and all the valid aliases for them. */
> static const struct
> @@ -178,6 +178,12 @@ static const char *const aarch64_mte_register_names[] =
> "tag_ctl"
> };
>
> +static const char *const aarch64_tls_register_names[] =
> +{
> + /* Thread/Process ID Register. */
> + "tpidr"
> +};
> +
> /* AArch64 prologue cache structure. */
> struct aarch64_prologue_cache
> {
> @@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch)
> If VQ is zero then it is assumed SVE is not supported.
> (It is not possible to set VQ to zero on an SVE system).
>
> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
> + MTE_P indicates the presence of the Memory Tagging Extension feature.
> +
> + TLS_P indicates the presence of the Thread Local Storage feature. */
>
> const target_desc *
> -aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
> +aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p, bool tls_p)
> {
> if (vq > AARCH64_MAX_SVE_VQ)
> error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
> AARCH64_MAX_SVE_VQ);
>
> - struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
> + struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
>
> if (tdesc == NULL)
> {
> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
> - tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, tls_p);
> + tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
> }
>
> return tdesc;
> @@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> bool valid_p = true;
> int i, num_regs = 0, num_pseudo_regs = 0;
> int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
> - int first_mte_regnum = -1;
> + int first_mte_regnum = -1, first_tls_regnum = -1;
>
> /* Use the vector length passed via the target info. Here -1 is used for no
> SVE, and 0 is unset. If unset then use the vector length from the existing
> @@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> value. */
> const struct target_desc *tdesc = info.target_desc;
> if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq (tdesc))
> - tdesc = aarch64_read_description (vq, false, false);
> + tdesc = aarch64_read_description (vq, false, false, false);
> gdb_assert (tdesc);
>
> feature_core = tdesc_find_feature (tdesc,"org.gnu.gdb.aarch64.core");
> @@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
> const struct tdesc_feature *feature_mte
> = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
> + const struct tdesc_feature *feature_tls
> + = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
>
> if (feature_core == nullptr)
> return nullptr;
> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> num_regs += i;
> }
>
> + /* Add the TLS register. */
> + if (feature_tls != nullptr)
> + {
> + first_tls_regnum = num_regs;
> +
> + /* Validate the descriptor provides the mandatory MTE registers and
> + allocate their numbers. */
It should say TLS instead of MTE. And also adjust it to mention it is a
single register (at least for now?).
> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
> + first_tls_regnum + i,
> + aarch64_tls_register_names[i]);
> +
> + num_regs += i;
> + }
> +
> if (!valid_p)
> return nullptr;
We should probably error/warn loudly about the fact this feature lacks a
mandatory register.
>
> @@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
> : pauth_ra_state_offset + num_regs;
> tdep->mte_reg_base = first_mte_regnum;
> + tdep->tls_reg_base = first_tls_regnum;
>
> set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
> set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 60a9d5a29c2..092586d4ee3 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
> {
> return mte_reg_base != -1;
> }
> +
> + /* TLS register. This is -1 if the TLS register is not available. */
> + int tls_reg_base = 0;
> +
> + bool has_tls() const
> + {
> + return tls_reg_base != -1;
> + }
> };
>
> const target_desc *aarch64_read_description (uint64_t vq, bool pauth_p,
> - bool mte_p);
> + bool mte_p, bool tls_p);
>
> extern int aarch64_process_record (struct gdbarch *gdbarch,
> struct regcache *regcache, CORE_ADDR addr);
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index 485d667ccde..733a3fd6d2a 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -24,11 +24,13 @@
> #include "../features/aarch64-sve.c"
> #include "../features/aarch64-pauth.c"
> #include "../features/aarch64-mte.c"
> +#include "../features/aarch64-tls.c"
>
> /* See arch/aarch64.h. */
>
> target_desc *
> -aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
> +aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p,
> + bool tls_p)
> {
> target_desc_up tdesc = allocate_target_description ();
>
> @@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
> if (mte_p)
> regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
>
> + if (tls_p)
> + regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
> +
> return tdesc.release ();
> }
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index e416e346e9a..79821666ce6 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -29,6 +29,7 @@ struct aarch64_features
> bool sve = false;
> bool pauth = false;
> bool mte = false;
> + bool tls = false;
> };
The tls field doesn't seem to be set/used anywhere. I think it should be
set when we find the tls feature, and used during gdbserver linux/bsd
target description selection.
Also, the patch adjusts the gdb/aarch64-linux-nat.c file, but doesn't
touch similar code in gdbserver's gdbserver/linux-aarch64-low.cc. We
also support NT_ARM_TLS there, but we don't export this particular
register through a feature.
I think it makes sense to do so, though I understand that is outside the
boundary of bsd work. Let me know if you'd like me to enable that and
test before the patch goes in.
>
> /* Create the aarch64 target description. A non zero VQ value indicates both
> @@ -36,10 +37,12 @@ struct aarch64_features
> an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH
> feature.
>
> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
> + MTE_P indicates the presence of the Memory Tagging Extension feature.
> +
> + TLS_P indicates the presence of the Thread Local Storage feature. */
>
> target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p,
> - bool mte_p);
> + bool mte_p, bool tls_p);
>
> /* Register numbers of various important registers.
> Note that on SVE, the Z registers reuse the V register numbers and the V
> @@ -84,6 +87,8 @@ enum aarch64_regnum
> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
> #define AARCH64_PAUTH_REGS_SIZE (16)
>
> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
> +
The purpose of similar macros is to index a register given a base
number. Given the TLS set is just a single register, I don't think this
macro should exist. We should use the value recorded in the tdep struct
instead.
Also, I couldn't find any uses of AARCH64_TPIDR_REGNUM. Maybe I missed a
patch?
> #define AARCH64_X_REGS_NUM 31
> #define AARCH64_V_REGS_NUM 32
> #define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 4b09819389a..946ec983df5 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> aarch64-fpu.xml \
> aarch64-pauth.xml \
> aarch64-mte.xml \
> + aarch64-tls.xml \
> arc/v1-core.xml \
> arc/v1-aux.xml \
> arc/v2-core.xml \
> diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
> new file mode 100644
> index 00000000000..30d730dffba
> --- /dev/null
> +++ b/gdb/features/aarch64-tls.c
> @@ -0,0 +1,14 @@
> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> + Original: aarch64-tls.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_aarch64_tls (struct target_desc *result, long regnum)
> +{
> + struct tdesc_feature *feature;
> +
> + feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
> + tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64, "data_ptr");
> + return regnum;
> +}
> diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
> new file mode 100644
> index 00000000000..48e0e9a6030
> --- /dev/null
> +++ b/gdb/features/aarch64-tls.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
> + Contributed by ARM Ltd.
I think the above Copyright should say 2022 and you should drop the
"Contributed by ARM".
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.aarch64.tls">
> + <reg name="tpidr" bitsize="64" type="data_ptr"/>
> +</feature>
> diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
> index e982ab85067..14d6a4f80eb 100644
> --- a/gdbserver/linux-aarch64-tdesc.cc
> +++ b/gdbserver/linux-aarch64-tdesc.cc
> @@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>
> if (tdesc == NULL)
> {
> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, false);
>
> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
> static const char *expedite_regs_aarch64_sve[] = { "x29", "sp", "pc",
> diff --git a/gdbserver/netbsd-aarch64-low.cc b/gdbserver/netbsd-aarch64-low.cc
> index 202bf1cdac6..b371e599232 100644
> --- a/gdbserver/netbsd-aarch64-low.cc
> +++ b/gdbserver/netbsd-aarch64-low.cc
> @@ -96,7 +96,7 @@ void
> netbsd_aarch64_target::low_arch_setup ()
> {
> target_desc *tdesc
> - = aarch64_create_target_description (0, false);
> + = aarch64_create_target_description (0, false, false, false);
>
> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
> init_target_desc (tdesc, expedite_regs_aarch64);
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-03-28 10:16 ` Luis Machado
@ 2022-04-01 23:30 ` John Baldwin
2022-04-04 8:06 ` Luis Machado
0 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-04-01 23:30 UTC (permalink / raw)
To: Luis Machado, binutils, gdb-patches
On 3/28/22 3:16 AM, Luis Machado wrote:
>> - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p);
>> + return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p,
>> + false);
>
> This is getting a bit ugly. We should pass a single struct that contains
> all available features eventually. But it should be OK right now.
Mmm, a struct would be nice, yes. Maybe I will do that as a followup change.
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index b714f6194b6..38c5c9e0e00 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> num_regs += i;
>> }
>>
>> + /* Add the TLS register. */
>> + if (feature_tls != nullptr)
>> + {
>> + first_tls_regnum = num_regs;
>> +
>> + /* Validate the descriptor provides the mandatory MTE registers and
>> + allocate their numbers. */
>
> It should say TLS instead of MTE. And also adjust it to mention it is a
> single register (at least for now?).
Oops, did miss a MTE rename. Note that the MTE register set also contains a
single register but still uses the plural (but I will fix this). If you think
it's clearer I could remove the aarch64_tls_register_names and just call
tdesc_numbered_register() once without the loop with the specific register name.
>> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
>> + first_tls_regnum + i,
>> + aarch64_tls_register_names[i]);
>> +
>> + num_regs += i;
>> + }
>> +
>> if (!valid_p)
>> return nullptr;
>
> We should probably error/warn loudly about the fact this feature lacks a
> mandatory register.
It requires the single "tpidr" register I think?
>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>> index e416e346e9a..79821666ce6 100644
>> --- a/gdb/arch/aarch64.h
>> +++ b/gdb/arch/aarch64.h
>> @@ -29,6 +29,7 @@ struct aarch64_features
>> bool sve = false;
>> bool pauth = false;
>> bool mte = false;
>> + bool tls = false;
>> };
>
> The tls field doesn't seem to be set/used anywhere. I think it should be
> set when we find the tls feature, and used during gdbserver linux/bsd
> target description selection.
Mm, yes. Arguably this structure should be the thing that replaces the
parameters passed to aarch64_read_description(), though I think it would
need the vq value and not just a bool for sve to serve that purpose.
> Also, the patch adjusts the gdb/aarch64-linux-nat.c file, but doesn't
> touch similar code in gdbserver's gdbserver/linux-aarch64-low.cc. We
> also support NT_ARM_TLS there, but we don't export this particular
> register through a feature.
So for this particular patch, I'm just trying to add the arch feature
itself. The later patches in the series (some of which don't seem to
have made it to the mailing list actually, so I will resend), make use
of it in aarch64-fbsd-tdep.c and aarch64-fbsd-nat.c. The changes to
aarch64-linux-nat.c in this patch is just to support the new argument
to aarch64_read_description().
> I think it makes sense to do so, though I understand that is outside the
> boundary of bsd work. Let me know if you'd like me to enable that and
> test before the patch goes in.
I can probably take a stab at this based on the FreeBSD patches in the
series (they should be fairly similar, at least for the tdep.c file).
>>
>> /* Create the aarch64 target description. A non zero VQ value indicates both
>> @@ -36,10 +37,12 @@ struct aarch64_features
>> an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH
>> feature.
>>
>> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
>> + MTE_P indicates the presence of the Memory Tagging Extension feature.
>> +
>> + TLS_P indicates the presence of the Thread Local Storage feature. */
>>
>> target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p,
>> - bool mte_p);
>> + bool mte_p, bool tls_p);
>>
>> /* Register numbers of various important registers.
>> Note that on SVE, the Z registers reuse the V register numbers and the V
>> @@ -84,6 +87,8 @@ enum aarch64_regnum
>> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
>> #define AARCH64_PAUTH_REGS_SIZE (16)
>>
>> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
>> +
>
> The purpose of similar macros is to index a register given a base
> number. Given the TLS set is just a single register, I don't think this
> macro should exist. We should use the value recorded in the tdep struct
> instead.
Ok, I was just following what had been done for MTE. I'm fine with just
making it a single value. Alternatively, it might be nice to allocate a
fixed value for the register in enum aarch64_regnum. I only picked a
variable base due to copying what was done for the PAUTH registers. If
adding a new AARCH64_TPIDR_REGNUM after AARCH64_SVE_VG_REGNUM is ok, it
would be simpler (e.g. I could use a static regcache_map instead of having
to construct one dynamically).
> Also, I couldn't find any uses of AARCH64_TPIDR_REGNUM. Maybe I missed a
> patch?
It is used in one of the FreeBSD patches that didn't make it to the list
(but was included in the cover letter).
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-04-01 23:30 ` John Baldwin
@ 2022-04-04 8:06 ` Luis Machado
2022-04-04 12:18 ` Luis Machado
0 siblings, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-04-04 8:06 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 4/2/22 00:30, John Baldwin wrote:
> On 3/28/22 3:16 AM, Luis Machado wrote:
>>> - return aarch64_read_description (aarch64_sve_get_vq (tid),
>>> pauth_p, mte_p);
>>> + return aarch64_read_description (aarch64_sve_get_vq (tid),
>>> pauth_p, mte_p,
>>> + false);
>>
>> This is getting a bit ugly. We should pass a single struct that contains
>> all available features eventually. But it should be OK right now.
>
> Mmm, a struct would be nice, yes. Maybe I will do that as a followup
> change.
My idea is to have a shared struct between gdb and gdbserver. Then do
feature checks based on that. I did do some of that for gdbserver, but
it didn't make its way to gdb yet unfortunately.
>
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index b714f6194b6..38c5c9e0e00 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info
>>> info, struct gdbarch_list *arches)
>>> num_regs += i;
>>> }
>>> + /* Add the TLS register. */
>>> + if (feature_tls != nullptr)
>>> + {
>>> + first_tls_regnum = num_regs;
>>> +
>>> + /* Validate the descriptor provides the mandatory MTE
>>> registers and
>>> + allocate their numbers. */
>>
>> It should say TLS instead of MTE. And also adjust it to mention it is a
>> single register (at least for now?).
>
> Oops, did miss a MTE rename. Note that the MTE register set also
> contains a
> single register but still uses the plural (but I will fix this). If you
> think
> it's clearer I could remove the aarch64_tls_register_names and just call
> tdesc_numbered_register() once without the loop with the specific
> register name.
>
Probably copy/pasting. I think it should be fine the way it is then, for
the sake of keeping it consistent.
>>> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>>> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
>>> + first_tls_regnum + i,
>>> + aarch64_tls_register_names[i]);
>>> +
>>> + num_regs += i;
>>> + }
>>> +
>>> if (!valid_p)
>>> return nullptr;
>>
>> We should probably error/warn loudly about the fact this feature lacks a
>> mandatory register.
>
> It requires the single "tpidr" register I think?
>
Right. Is your intent to make tpidr a mandatory register for FreeBSD
going forward? So if it is missing, should it be considered an error?
For Linux it shouldn't make the feature selection fail, for example.
>>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>>> index e416e346e9a..79821666ce6 100644
>>> --- a/gdb/arch/aarch64.h
>>> +++ b/gdb/arch/aarch64.h
>>> @@ -29,6 +29,7 @@ struct aarch64_features
>>> bool sve = false;
>>> bool pauth = false;
>>> bool mte = false;
>>> + bool tls = false;
>>> };
>>
>> The tls field doesn't seem to be set/used anywhere. I think it should be
>> set when we find the tls feature, and used during gdbserver linux/bsd
>> target description selection.
>
> Mm, yes. Arguably this structure should be the thing that replaces the
> parameters passed to aarch64_read_description(), though I think it would
> need the vq value and not just a bool for sve to serve that purpose.
>
That's true.
>> Also, the patch adjusts the gdb/aarch64-linux-nat.c file, but doesn't
>> touch similar code in gdbserver's gdbserver/linux-aarch64-low.cc. We
>> also support NT_ARM_TLS there, but we don't export this particular
>> register through a feature.
>
> So for this particular patch, I'm just trying to add the arch feature
> itself. The later patches in the series (some of which don't seem to
> have made it to the mailing list actually, so I will resend), make use
> of it in aarch64-fbsd-tdep.c and aarch64-fbsd-nat.c. The changes to
> aarch64-linux-nat.c in this patch is just to support the new argument
> to aarch64_read_description().
>
>> I think it makes sense to do so, though I understand that is outside the
>> boundary of bsd work. Let me know if you'd like me to enable that and
>> test before the patch goes in.
>
> I can probably take a stab at this based on the FreeBSD patches in the
> series (they should be fairly similar, at least for the tdep.c file).
I have to teach binutils about this particular register set
(NT_ARM_TLS). I see readelf currently doesn't recognize the proper name
in a core dump. I'll send a patch soon.
>>> /* Create the aarch64 target description. A non zero VQ value
>>> indicates both
>>> @@ -36,10 +37,12 @@ struct aarch64_features
>>> an SVE Z register. HAS_PAUTH_P indicates the presence of the
>>> PAUTH
>>> feature.
>>> - MTE_P indicates the presence of the Memory Tagging Extension
>>> feature. */
>>> + MTE_P indicates the presence of the Memory Tagging Extension
>>> feature.
>>> +
>>> + TLS_P indicates the presence of the Thread Local Storage
>>> feature. */
>>> target_desc *aarch64_create_target_description (uint64_t vq, bool
>>> has_pauth_p,
>>> - bool mte_p);
>>> + bool mte_p, bool tls_p);
>>> /* Register numbers of various important registers.
>>> Note that on SVE, the Z registers reuse the V register numbers
>>> and the V
>>> @@ -84,6 +87,8 @@ enum aarch64_regnum
>>> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base
>>> + 1)
>>> #define AARCH64_PAUTH_REGS_SIZE (16)
>>> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
>>> +
>>
>> The purpose of similar macros is to index a register given a base
>> number. Given the TLS set is just a single register, I don't think this
>> macro should exist. We should use the value recorded in the tdep struct
>> instead.
>
> Ok, I was just following what had been done for MTE. I'm fine with just
> making it a single value. Alternatively, it might be nice to allocate a
> fixed value for the register in enum aarch64_regnum. I only picked a
> variable base due to copying what was done for the PAUTH registers. If
> adding a new AARCH64_TPIDR_REGNUM after AARCH64_SVE_VG_REGNUM is ok, it
> would be simpler (e.g. I could use a static regcache_map instead of having
> to construct one dynamically).
Right. The idea of recording things in the tdep struct is to better
organize the register sets in the presence of optional features. You
might have some of the features, but not others, and we can't leave a
hole in the register numbering.
At the moment there is a mix of macros and structs. My plan is to
eventually convert everything to a single mechanism. A regcache_map
might work nicely.
>
>> Also, I couldn't find any uses of AARCH64_TPIDR_REGNUM. Maybe I missed a
>> patch?
>
> It is used in one of the FreeBSD patches that didn't make it to the list
> (but was included in the cover letter).
>
Ah, I see.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-04-04 8:06 ` Luis Machado
@ 2022-04-04 12:18 ` Luis Machado
0 siblings, 0 replies; 26+ messages in thread
From: Luis Machado @ 2022-04-04 12:18 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 4/4/22 09:06, Luis Machado wrote:
> On 4/2/22 00:30, John Baldwin wrote:
>> On 3/28/22 3:16 AM, Luis Machado wrote:
>>>> - return aarch64_read_description (aarch64_sve_get_vq (tid),
>>>> pauth_p, mte_p);
>>>> + return aarch64_read_description (aarch64_sve_get_vq (tid),
>>>> pauth_p, mte_p,
>>>> + false);
>>>
>>> This is getting a bit ugly. We should pass a single struct that contains
>>> all available features eventually. But it should be OK right now.
>>
>> Mmm, a struct would be nice, yes. Maybe I will do that as a followup
>> change.
>
> My idea is to have a shared struct between gdb and gdbserver. Then do
> feature checks based on that. I did do some of that for gdbserver, but
> it didn't make its way to gdb yet unfortunately.
>
>>
>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>>> index b714f6194b6..38c5c9e0e00 100644
>>>> --- a/gdb/aarch64-tdep.c
>>>> +++ b/gdb/aarch64-tdep.c
>>>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info
>>>> info, struct gdbarch_list *arches)
>>>> num_regs += i;
>>>> }
>>>> + /* Add the TLS register. */
>>>> + if (feature_tls != nullptr)
>>>> + {
>>>> + first_tls_regnum = num_regs;
>>>> +
>>>> + /* Validate the descriptor provides the mandatory MTE
>>>> registers and
>>>> + allocate their numbers. */
>>>
>>> It should say TLS instead of MTE. And also adjust it to mention it is a
>>> single register (at least for now?).
>>
>> Oops, did miss a MTE rename. Note that the MTE register set also
>> contains a
>> single register but still uses the plural (but I will fix this). If
>> you think
>> it's clearer I could remove the aarch64_tls_register_names and just call
>> tdesc_numbered_register() once without the loop with the specific
>> register name.
>>
>
> Probably copy/pasting. I think it should be fine the way it is then, for
> the sake of keeping it consistent.
>
>>>> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>>>> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get
>>>> (),
>>>> + first_tls_regnum + i,
>>>> + aarch64_tls_register_names[i]);
>>>> +
>>>> + num_regs += i;
>>>> + }
>>>> +
>>>> if (!valid_p)
>>>> return nullptr;
>>>
>>> We should probably error/warn loudly about the fact this feature lacks a
>>> mandatory register.
>>
>> It requires the single "tpidr" register I think?
>>
>
> Right. Is your intent to make tpidr a mandatory register for FreeBSD
> going forward? So if it is missing, should it be considered an error?
>
> For Linux it shouldn't make the feature selection fail, for example.
>
>>>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>>>> index e416e346e9a..79821666ce6 100644
>>>> --- a/gdb/arch/aarch64.h
>>>> +++ b/gdb/arch/aarch64.h
>>>> @@ -29,6 +29,7 @@ struct aarch64_features
>>>> bool sve = false;
>>>> bool pauth = false;
>>>> bool mte = false;
>>>> + bool tls = false;
>>>> };
>>>
>>> The tls field doesn't seem to be set/used anywhere. I think it should be
>>> set when we find the tls feature, and used during gdbserver linux/bsd
>>> target description selection.
>>
>> Mm, yes. Arguably this structure should be the thing that replaces the
>> parameters passed to aarch64_read_description(), though I think it would
>> need the vq value and not just a bool for sve to serve that purpose.
>>
>
> That's true.
>
>>> Also, the patch adjusts the gdb/aarch64-linux-nat.c file, but doesn't
>>> touch similar code in gdbserver's gdbserver/linux-aarch64-low.cc. We
>>> also support NT_ARM_TLS there, but we don't export this particular
>>> register through a feature.
>>
>> So for this particular patch, I'm just trying to add the arch feature
>> itself. The later patches in the series (some of which don't seem to
>> have made it to the mailing list actually, so I will resend), make use
>> of it in aarch64-fbsd-tdep.c and aarch64-fbsd-nat.c. The changes to
>> aarch64-linux-nat.c in this patch is just to support the new argument
>> to aarch64_read_description().
>>
>>> I think it makes sense to do so, though I understand that is outside the
>>> boundary of bsd work. Let me know if you'd like me to enable that and
>>> test before the patch goes in.
>>
>> I can probably take a stab at this based on the FreeBSD patches in the
>> series (they should be fairly similar, at least for the tdep.c file).
>
> I have to teach binutils about this particular register set
> (NT_ARM_TLS). I see readelf currently doesn't recognize the proper name
> in a core dump. I'll send a patch soon.
>
Sorry, scratch that. The set that isn't handled is the system call number.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-03-23 21:00 ` [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register John Baldwin
2022-03-28 10:16 ` Luis Machado
@ 2022-05-03 21:14 ` Luis Machado
2022-05-03 21:30 ` John Baldwin
1 sibling, 1 reply; 26+ messages in thread
From: Luis Machado @ 2022-05-03 21:14 UTC (permalink / raw)
To: John Baldwin, binutils, gdb-patches
On 3/23/22 21:00, John Baldwin wrote:
> ---
> gdb/aarch64-linux-nat.c | 3 ++-
> gdb/aarch64-linux-tdep.c | 2 +-
> gdb/aarch64-tdep.c | 42 ++++++++++++++++++++++++++------
> gdb/aarch64-tdep.h | 10 +++++++-
> gdb/arch/aarch64.c | 7 +++++-
> gdb/arch/aarch64.h | 9 +++++--
> gdb/features/Makefile | 1 +
> gdb/features/aarch64-tls.c | 14 +++++++++++
> gdb/features/aarch64-tls.xml | 12 +++++++++
> gdbserver/linux-aarch64-tdesc.cc | 2 +-
> gdbserver/netbsd-aarch64-low.cc | 2 +-
> 11 files changed, 88 insertions(+), 16 deletions(-)
> create mode 100644 gdb/features/aarch64-tls.c
> create mode 100644 gdb/features/aarch64-tls.xml
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 7bb82d17cc8..4da274c285a 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
> bool mte_p = hwcap2 & HWCAP2_MTE;
>
> - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p);
> + return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p,
> + false);
> }
>
> /* Convert a native/host siginfo object, into/from the siginfo in the
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index cb132d5a540..f5aac7bc0b4 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
> bool mte_p = hwcap2 & HWCAP2_MTE;
> return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
> - pauth_p, mte_p);
> + pauth_p, mte_p, false);
> }
>
> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b714f6194b6..38c5c9e0e00 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -58,7 +58,7 @@
> #define HA_MAX_NUM_FLDS 4
>
> /* All possible aarch64 target descriptors. */
> -static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */];
> +static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */][2 /* tls */];
>
> /* The standard register names, and all the valid aliases for them. */
> static const struct
> @@ -178,6 +178,12 @@ static const char *const aarch64_mte_register_names[] =
> "tag_ctl"
> };
>
> +static const char *const aarch64_tls_register_names[] =
> +{
> + /* Thread/Process ID Register. */
> + "tpidr"
> +};
> +
> /* AArch64 prologue cache structure. */
> struct aarch64_prologue_cache
> {
> @@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch)
> If VQ is zero then it is assumed SVE is not supported.
> (It is not possible to set VQ to zero on an SVE system).
>
> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
> + MTE_P indicates the presence of the Memory Tagging Extension feature.
> +
> + TLS_P indicates the presence of the Thread Local Storage feature. */
>
> const target_desc *
> -aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
> +aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p, bool tls_p)
> {
> if (vq > AARCH64_MAX_SVE_VQ)
> error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
> AARCH64_MAX_SVE_VQ);
>
> - struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
> + struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
>
> if (tdesc == NULL)
> {
> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
> - tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, tls_p);
> + tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
> }
>
> return tdesc;
> @@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> bool valid_p = true;
> int i, num_regs = 0, num_pseudo_regs = 0;
> int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
> - int first_mte_regnum = -1;
> + int first_mte_regnum = -1, first_tls_regnum = -1;
>
> /* Use the vector length passed via the target info. Here -1 is used for no
> SVE, and 0 is unset. If unset then use the vector length from the existing
> @@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> value. */
> const struct target_desc *tdesc = info.target_desc;
> if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq (tdesc))
> - tdesc = aarch64_read_description (vq, false, false);
> + tdesc = aarch64_read_description (vq, false, false, false);
> gdb_assert (tdesc);
>
> feature_core = tdesc_find_feature (tdesc,"org.gnu.gdb.aarch64.core");
> @@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
> const struct tdesc_feature *feature_mte
> = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
> + const struct tdesc_feature *feature_tls
> + = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
>
> if (feature_core == nullptr)
> return nullptr;
> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> num_regs += i;
> }
>
> + /* Add the TLS register. */
> + if (feature_tls != nullptr)
> + {
> + first_tls_regnum = num_regs;
> +
> + /* Validate the descriptor provides the mandatory MTE registers and
> + allocate their numbers. */
> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
> + first_tls_regnum + i,
> + aarch64_tls_register_names[i]);
> +
> + num_regs += i;
> + }
> +
> if (!valid_p)
> return nullptr;
>
> @@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
> : pauth_ra_state_offset + num_regs;
> tdep->mte_reg_base = first_mte_regnum;
> + tdep->tls_reg_base = first_tls_regnum;
>
> set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
> set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 60a9d5a29c2..092586d4ee3 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
> {
> return mte_reg_base != -1;
> }
> +
> + /* TLS register. This is -1 if the TLS register is not available. */
> + int tls_reg_base = 0;
> +
> + bool has_tls() const
> + {
> + return tls_reg_base != -1;
> + }
> };
>
> const target_desc *aarch64_read_description (uint64_t vq, bool pauth_p,
> - bool mte_p);
> + bool mte_p, bool tls_p);
>
> extern int aarch64_process_record (struct gdbarch *gdbarch,
> struct regcache *regcache, CORE_ADDR addr);
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index 485d667ccde..733a3fd6d2a 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -24,11 +24,13 @@
> #include "../features/aarch64-sve.c"
> #include "../features/aarch64-pauth.c"
> #include "../features/aarch64-mte.c"
> +#include "../features/aarch64-tls.c"
>
> /* See arch/aarch64.h. */
>
> target_desc *
> -aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
> +aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p,
> + bool tls_p)
> {
> target_desc_up tdesc = allocate_target_description ();
>
> @@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
> if (mte_p)
> regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
>
> + if (tls_p)
> + regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
> +
> return tdesc.release ();
> }
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index e416e346e9a..79821666ce6 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -29,6 +29,7 @@ struct aarch64_features
> bool sve = false;
> bool pauth = false;
> bool mte = false;
> + bool tls = false;
> };
>
> /* Create the aarch64 target description. A non zero VQ value indicates both
> @@ -36,10 +37,12 @@ struct aarch64_features
> an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH
> feature.
>
> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
> + MTE_P indicates the presence of the Memory Tagging Extension feature.
> +
> + TLS_P indicates the presence of the Thread Local Storage feature. */
>
> target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p,
> - bool mte_p);
> + bool mte_p, bool tls_p);
>
> /* Register numbers of various important registers.
> Note that on SVE, the Z registers reuse the V register numbers and the V
> @@ -84,6 +87,8 @@ enum aarch64_regnum
> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
> #define AARCH64_PAUTH_REGS_SIZE (16)
>
> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
> +
> #define AARCH64_X_REGS_NUM 31
> #define AARCH64_V_REGS_NUM 32
> #define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 4b09819389a..946ec983df5 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> aarch64-fpu.xml \
> aarch64-pauth.xml \
> aarch64-mte.xml \
> + aarch64-tls.xml \
> arc/v1-core.xml \
> arc/v1-aux.xml \
> arc/v2-core.xml \
> diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
> new file mode 100644
> index 00000000000..30d730dffba
> --- /dev/null
> +++ b/gdb/features/aarch64-tls.c
> @@ -0,0 +1,14 @@
> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> + Original: aarch64-tls.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_aarch64_tls (struct target_desc *result, long regnum)
> +{
> + struct tdesc_feature *feature;
> +
> + feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
> + tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64, "data_ptr");
> + return regnum;
> +}
> diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
> new file mode 100644
> index 00000000000..48e0e9a6030
> --- /dev/null
> +++ b/gdb/features/aarch64-tls.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
> + Contributed by ARM Ltd.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.aarch64.tls">
> + <reg name="tpidr" bitsize="64" type="data_ptr"/>
> +</feature>
> diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
> index e982ab85067..14d6a4f80eb 100644
> --- a/gdbserver/linux-aarch64-tdesc.cc
> +++ b/gdbserver/linux-aarch64-tdesc.cc
> @@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>
> if (tdesc == NULL)
> {
> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, false);
>
> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
> static const char *expedite_regs_aarch64_sve[] = { "x29", "sp", "pc",
> diff --git a/gdbserver/netbsd-aarch64-low.cc b/gdbserver/netbsd-aarch64-low.cc
> index 202bf1cdac6..b371e599232 100644
> --- a/gdbserver/netbsd-aarch64-low.cc
> +++ b/gdbserver/netbsd-aarch64-low.cc
> @@ -96,7 +96,7 @@ void
> netbsd_aarch64_target::low_arch_setup ()
> {
> target_desc *tdesc
> - = aarch64_create_target_description (0, false);
> + = aarch64_create_target_description (0, false, false, false);
>
> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
> init_target_desc (tdesc, expedite_regs_aarch64);
LGTM from aarch64's side.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-05-03 21:14 ` Luis Machado
@ 2022-05-03 21:30 ` John Baldwin
2022-05-03 21:34 ` Luis Machado
0 siblings, 1 reply; 26+ messages in thread
From: John Baldwin @ 2022-05-03 21:30 UTC (permalink / raw)
To: Luis Machado, gdb-patches
On 5/3/22 2:14 PM, Luis Machado wrote:
> On 3/23/22 21:00, John Baldwin wrote:
>> ---
>> gdb/aarch64-linux-nat.c | 3 ++-
>> gdb/aarch64-linux-tdep.c | 2 +-
>> gdb/aarch64-tdep.c | 42 ++++++++++++++++++++++++++------
>> gdb/aarch64-tdep.h | 10 +++++++-
>> gdb/arch/aarch64.c | 7 +++++-
>> gdb/arch/aarch64.h | 9 +++++--
>> gdb/features/Makefile | 1 +
>> gdb/features/aarch64-tls.c | 14 +++++++++++
>> gdb/features/aarch64-tls.xml | 12 +++++++++
>> gdbserver/linux-aarch64-tdesc.cc | 2 +-
>> gdbserver/netbsd-aarch64-low.cc | 2 +-
>> 11 files changed, 88 insertions(+), 16 deletions(-)
>> create mode 100644 gdb/features/aarch64-tls.c
>> create mode 100644 gdb/features/aarch64-tls.xml
>>
>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>> index 7bb82d17cc8..4da274c285a 100644
>> --- a/gdb/aarch64-linux-nat.c
>> +++ b/gdb/aarch64-linux-nat.c
>> @@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
>> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>> bool mte_p = hwcap2 & HWCAP2_MTE;
>>
>> - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p);
>> + return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p,
>> + false);
>> }
>>
>> /* Convert a native/host siginfo object, into/from the siginfo in the
>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>> index cb132d5a540..f5aac7bc0b4 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>> bool mte_p = hwcap2 & HWCAP2_MTE;
>> return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
>> - pauth_p, mte_p);
>> + pauth_p, mte_p, false);
>> }
>>
>> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index b714f6194b6..38c5c9e0e00 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -58,7 +58,7 @@
>> #define HA_MAX_NUM_FLDS 4
>>
>> /* All possible aarch64 target descriptors. */
>> -static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */];
>> +static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1][2/*pauth*/][2 /* mte */][2 /* tls */];
>>
>> /* The standard register names, and all the valid aliases for them. */
>> static const struct
>> @@ -178,6 +178,12 @@ static const char *const aarch64_mte_register_names[] =
>> "tag_ctl"
>> };
>>
>> +static const char *const aarch64_tls_register_names[] =
>> +{
>> + /* Thread/Process ID Register. */
>> + "tpidr"
>> +};
>> +
>> /* AArch64 prologue cache structure. */
>> struct aarch64_prologue_cache
>> {
>> @@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch)
>> If VQ is zero then it is assumed SVE is not supported.
>> (It is not possible to set VQ to zero on an SVE system).
>>
>> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
>> + MTE_P indicates the presence of the Memory Tagging Extension feature.
>> +
>> + TLS_P indicates the presence of the Thread Local Storage feature. */
>>
>> const target_desc *
>> -aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>> +aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p, bool tls_p)
>> {
>> if (vq > AARCH64_MAX_SVE_VQ)
>> error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
>> AARCH64_MAX_SVE_VQ);
>>
>> - struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
>> + struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
>>
>> if (tdesc == NULL)
>> {
>> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>> - tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
>> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, tls_p);
>> + tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
>> }
>>
>> return tdesc;
>> @@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> bool valid_p = true;
>> int i, num_regs = 0, num_pseudo_regs = 0;
>> int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
>> - int first_mte_regnum = -1;
>> + int first_mte_regnum = -1, first_tls_regnum = -1;
>>
>> /* Use the vector length passed via the target info. Here -1 is used for no
>> SVE, and 0 is unset. If unset then use the vector length from the existing
>> @@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> value. */
>> const struct target_desc *tdesc = info.target_desc;
>> if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq (tdesc))
>> - tdesc = aarch64_read_description (vq, false, false);
>> + tdesc = aarch64_read_description (vq, false, false, false);
>> gdb_assert (tdesc);
>>
>> feature_core = tdesc_find_feature (tdesc,"org.gnu.gdb.aarch64.core");
>> @@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
>> const struct tdesc_feature *feature_mte
>> = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
>> + const struct tdesc_feature *feature_tls
>> + = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
>>
>> if (feature_core == nullptr)
>> return nullptr;
>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> num_regs += i;
>> }
>>
>> + /* Add the TLS register. */
>> + if (feature_tls != nullptr)
>> + {
>> + first_tls_regnum = num_regs;
>> +
>> + /* Validate the descriptor provides the mandatory MTE registers and
>> + allocate their numbers. */
>> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
>> + first_tls_regnum + i,
>> + aarch64_tls_register_names[i]);
>> +
>> + num_regs += i;
>> + }
>> +
>> if (!valid_p)
>> return nullptr;
>>
>> @@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>> tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
>> : pauth_ra_state_offset + num_regs;
>> tdep->mte_reg_base = first_mte_regnum;
>> + tdep->tls_reg_base = first_tls_regnum;
>>
>> set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
>> set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
>> index 60a9d5a29c2..092586d4ee3 100644
>> --- a/gdb/aarch64-tdep.h
>> +++ b/gdb/aarch64-tdep.h
>> @@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
>> {
>> return mte_reg_base != -1;
>> }
>> +
>> + /* TLS register. This is -1 if the TLS register is not available. */
>> + int tls_reg_base = 0;
>> +
>> + bool has_tls() const
>> + {
>> + return tls_reg_base != -1;
>> + }
>> };
>>
>> const target_desc *aarch64_read_description (uint64_t vq, bool pauth_p,
>> - bool mte_p);
>> + bool mte_p, bool tls_p);
>>
>> extern int aarch64_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache, CORE_ADDR addr);
>> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
>> index 485d667ccde..733a3fd6d2a 100644
>> --- a/gdb/arch/aarch64.c
>> +++ b/gdb/arch/aarch64.c
>> @@ -24,11 +24,13 @@
>> #include "../features/aarch64-sve.c"
>> #include "../features/aarch64-pauth.c"
>> #include "../features/aarch64-mte.c"
>> +#include "../features/aarch64-tls.c"
>>
>> /* See arch/aarch64.h. */
>>
>> target_desc *
>> -aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
>> +aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p,
>> + bool tls_p)
>> {
>> target_desc_up tdesc = allocate_target_description ();
>>
>> @@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq, bool pauth_p, bool mte_p)
>> if (mte_p)
>> regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
>>
>> + if (tls_p)
>> + regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
>> +
>> return tdesc.release ();
>> }
>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>> index e416e346e9a..79821666ce6 100644
>> --- a/gdb/arch/aarch64.h
>> +++ b/gdb/arch/aarch64.h
>> @@ -29,6 +29,7 @@ struct aarch64_features
>> bool sve = false;
>> bool pauth = false;
>> bool mte = false;
>> + bool tls = false;
>> };
>>
>> /* Create the aarch64 target description. A non zero VQ value indicates both
>> @@ -36,10 +37,12 @@ struct aarch64_features
>> an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH
>> feature.
>>
>> - MTE_P indicates the presence of the Memory Tagging Extension feature. */
>> + MTE_P indicates the presence of the Memory Tagging Extension feature.
>> +
>> + TLS_P indicates the presence of the Thread Local Storage feature. */
>>
>> target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p,
>> - bool mte_p);
>> + bool mte_p, bool tls_p);
>>
>> /* Register numbers of various important registers.
>> Note that on SVE, the Z registers reuse the V register numbers and the V
>> @@ -84,6 +87,8 @@ enum aarch64_regnum
>> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
>> #define AARCH64_PAUTH_REGS_SIZE (16)
>>
>> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
>> +
>> #define AARCH64_X_REGS_NUM 31
>> #define AARCH64_V_REGS_NUM 32
>> #define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
>> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
>> index 4b09819389a..946ec983df5 100644
>> --- a/gdb/features/Makefile
>> +++ b/gdb/features/Makefile
>> @@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
>> aarch64-fpu.xml \
>> aarch64-pauth.xml \
>> aarch64-mte.xml \
>> + aarch64-tls.xml \
>> arc/v1-core.xml \
>> arc/v1-aux.xml \
>> arc/v2-core.xml \
>> diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
>> new file mode 100644
>> index 00000000000..30d730dffba
>> --- /dev/null
>> +++ b/gdb/features/aarch64-tls.c
>> @@ -0,0 +1,14 @@
>> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
>> + Original: aarch64-tls.xml */
>> +
>> +#include "gdbsupport/tdesc.h"
>> +
>> +static int
>> +create_feature_aarch64_tls (struct target_desc *result, long regnum)
>> +{
>> + struct tdesc_feature *feature;
>> +
>> + feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
>> + tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64, "data_ptr");
>> + return regnum;
>> +}
>> diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
>> new file mode 100644
>> index 00000000000..48e0e9a6030
>> --- /dev/null
>> +++ b/gdb/features/aarch64-tls.xml
>> @@ -0,0 +1,12 @@
>> +<?xml version="1.0"?>
>> +<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
>> + Contributed by ARM Ltd.
>> +
>> + Copying and distribution of this file, with or without modification,
>> + are permitted in any medium without royalty provided the copyright
>> + notice and this notice are preserved. -->
>> +
>> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
>> +<feature name="org.gnu.gdb.aarch64.tls">
>> + <reg name="tpidr" bitsize="64" type="data_ptr"/>
>> +</feature>
>> diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
>> index e982ab85067..14d6a4f80eb 100644
>> --- a/gdbserver/linux-aarch64-tdesc.cc
>> +++ b/gdbserver/linux-aarch64-tdesc.cc
>> @@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>>
>> if (tdesc == NULL)
>> {
>> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, false);
>>
>> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
>> static const char *expedite_regs_aarch64_sve[] = { "x29", "sp", "pc",
>> diff --git a/gdbserver/netbsd-aarch64-low.cc b/gdbserver/netbsd-aarch64-low.cc
>> index 202bf1cdac6..b371e599232 100644
>> --- a/gdbserver/netbsd-aarch64-low.cc
>> +++ b/gdbserver/netbsd-aarch64-low.cc
>> @@ -96,7 +96,7 @@ void
>> netbsd_aarch64_target::low_arch_setup ()
>> {
>> target_desc *tdesc
>> - = aarch64_create_target_description (0, false);
>> + = aarch64_create_target_description (0, false, false, false);
>>
>> static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
>> init_target_desc (tdesc, expedite_regs_aarch64);
>
> LGTM from aarch64's side.
Hmm, did you mean to send this in response to the V3 series posted last week?
--
John Baldwin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
2022-05-03 21:30 ` John Baldwin
@ 2022-05-03 21:34 ` Luis Machado
0 siblings, 0 replies; 26+ messages in thread
From: Luis Machado @ 2022-05-03 21:34 UTC (permalink / raw)
To: John Baldwin, gdb-patches
On 5/3/22 22:30, John Baldwin wrote:
> On 5/3/22 2:14 PM, Luis Machado wrote:
>> On 3/23/22 21:00, John Baldwin wrote:
>>> ---
>>> gdb/aarch64-linux-nat.c | 3 ++-
>>> gdb/aarch64-linux-tdep.c | 2 +-
>>> gdb/aarch64-tdep.c | 42
>>> ++++++++++++++++++++++++++------
>>> gdb/aarch64-tdep.h | 10 +++++++-
>>> gdb/arch/aarch64.c | 7 +++++-
>>> gdb/arch/aarch64.h | 9 +++++--
>>> gdb/features/Makefile | 1 +
>>> gdb/features/aarch64-tls.c | 14 +++++++++++
>>> gdb/features/aarch64-tls.xml | 12 +++++++++
>>> gdbserver/linux-aarch64-tdesc.cc | 2 +-
>>> gdbserver/netbsd-aarch64-low.cc | 2 +-
>>> 11 files changed, 88 insertions(+), 16 deletions(-)
>>> create mode 100644 gdb/features/aarch64-tls.c
>>> create mode 100644 gdb/features/aarch64-tls.xml
>>>
>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>>> index 7bb82d17cc8..4da274c285a 100644
>>> --- a/gdb/aarch64-linux-nat.c
>>> +++ b/gdb/aarch64-linux-nat.c
>>> @@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
>>> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>>> bool mte_p = hwcap2 & HWCAP2_MTE;
>>> - return aarch64_read_description (aarch64_sve_get_vq (tid),
>>> pauth_p, mte_p);
>>> + return aarch64_read_description (aarch64_sve_get_vq (tid),
>>> pauth_p, mte_p,
>>> + false);
>>> }
>>> /* Convert a native/host siginfo object, into/from the siginfo in the
>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>>> index cb132d5a540..f5aac7bc0b4 100644
>>> --- a/gdb/aarch64-linux-tdep.c
>>> +++ b/gdb/aarch64-linux-tdep.c
>>> @@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct
>>> gdbarch *gdbarch,
>>> bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>>> bool mte_p = hwcap2 & HWCAP2_MTE;
>>> return aarch64_read_description (aarch64_linux_core_read_vq
>>> (gdbarch, abfd),
>>> - pauth_p, mte_p);
>>> + pauth_p, mte_p, false);
>>> }
>>> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index b714f6194b6..38c5c9e0e00 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -58,7 +58,7 @@
>>> #define HA_MAX_NUM_FLDS 4
>>> /* All possible aarch64 target descriptors. */
>>> -static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ +
>>> 1][2/*pauth*/][2 /* mte */];
>>> +static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ +
>>> 1][2/*pauth*/][2 /* mte */][2 /* tls */];
>>> /* The standard register names, and all the valid aliases for
>>> them. */
>>> static const struct
>>> @@ -178,6 +178,12 @@ static const char *const
>>> aarch64_mte_register_names[] =
>>> "tag_ctl"
>>> };
>>> +static const char *const aarch64_tls_register_names[] =
>>> +{
>>> + /* Thread/Process ID Register. */
>>> + "tpidr"
>>> +};
>>> +
>>> /* AArch64 prologue cache structure. */
>>> struct aarch64_prologue_cache
>>> {
>>> @@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct
>>> gdbarch *gdbarch)
>>> If VQ is zero then it is assumed SVE is not supported.
>>> (It is not possible to set VQ to zero on an SVE system).
>>> - MTE_P indicates the presence of the Memory Tagging Extension
>>> feature. */
>>> + MTE_P indicates the presence of the Memory Tagging Extension
>>> feature.
>>> +
>>> + TLS_P indicates the presence of the Thread Local Storage
>>> feature. */
>>> const target_desc *
>>> -aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>>> +aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p,
>>> bool tls_p)
>>> {
>>> if (vq > AARCH64_MAX_SVE_VQ)
>>> error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
>>> AARCH64_MAX_SVE_VQ);
>>> - struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
>>> + struct target_desc *tdesc =
>>> tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
>>> if (tdesc == NULL)
>>> {
>>> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>>> - tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
>>> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p,
>>> tls_p);
>>> + tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
>>> }
>>> return tdesc;
>>> @@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info,
>>> struct gdbarch_list *arches)
>>> bool valid_p = true;
>>> int i, num_regs = 0, num_pseudo_regs = 0;
>>> int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
>>> - int first_mte_regnum = -1;
>>> + int first_mte_regnum = -1, first_tls_regnum = -1;
>>> /* Use the vector length passed via the target info. Here -1 is
>>> used for no
>>> SVE, and 0 is unset. If unset then use the vector length
>>> from the existing
>>> @@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info,
>>> struct gdbarch_list *arches)
>>> value. */
>>> const struct target_desc *tdesc = info.target_desc;
>>> if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq
>>> (tdesc))
>>> - tdesc = aarch64_read_description (vq, false, false);
>>> + tdesc = aarch64_read_description (vq, false, false, false);
>>> gdb_assert (tdesc);
>>> feature_core = tdesc_find_feature
>>> (tdesc,"org.gnu.gdb.aarch64.core");
>>> @@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info,
>>> struct gdbarch_list *arches)
>>> feature_pauth = tdesc_find_feature (tdesc,
>>> "org.gnu.gdb.aarch64.pauth");
>>> const struct tdesc_feature *feature_mte
>>> = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
>>> + const struct tdesc_feature *feature_tls
>>> + = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
>>> if (feature_core == nullptr)
>>> return nullptr;
>>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info
>>> info, struct gdbarch_list *arches)
>>> num_regs += i;
>>> }
>>> + /* Add the TLS register. */
>>> + if (feature_tls != nullptr)
>>> + {
>>> + first_tls_regnum = num_regs;
>>> +
>>> + /* Validate the descriptor provides the mandatory MTE
>>> registers and
>>> + allocate their numbers. */
>>> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>>> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
>>> + first_tls_regnum + i,
>>> + aarch64_tls_register_names[i]);
>>> +
>>> + num_regs += i;
>>> + }
>>> +
>>> if (!valid_p)
>>> return nullptr;
>>> @@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info,
>>> struct gdbarch_list *arches)
>>> tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
>>> : pauth_ra_state_offset + num_regs;
>>> tdep->mte_reg_base = first_mte_regnum;
>>> + tdep->tls_reg_base = first_tls_regnum;
>>> set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
>>> set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
>>> index 60a9d5a29c2..092586d4ee3 100644
>>> --- a/gdb/aarch64-tdep.h
>>> +++ b/gdb/aarch64-tdep.h
>>> @@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
>>> {
>>> return mte_reg_base != -1;
>>> }
>>> +
>>> + /* TLS register. This is -1 if the TLS register is not
>>> available. */
>>> + int tls_reg_base = 0;
>>> +
>>> + bool has_tls() const
>>> + {
>>> + return tls_reg_base != -1;
>>> + }
>>> };
>>> const target_desc *aarch64_read_description (uint64_t vq, bool
>>> pauth_p,
>>> - bool mte_p);
>>> + bool mte_p, bool tls_p);
>>> extern int aarch64_process_record (struct gdbarch *gdbarch,
>>> struct regcache *regcache, CORE_ADDR addr);
>>> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
>>> index 485d667ccde..733a3fd6d2a 100644
>>> --- a/gdb/arch/aarch64.c
>>> +++ b/gdb/arch/aarch64.c
>>> @@ -24,11 +24,13 @@
>>> #include "../features/aarch64-sve.c"
>>> #include "../features/aarch64-pauth.c"
>>> #include "../features/aarch64-mte.c"
>>> +#include "../features/aarch64-tls.c"
>>> /* See arch/aarch64.h. */
>>> target_desc *
>>> -aarch64_create_target_description (uint64_t vq, bool pauth_p, bool
>>> mte_p)
>>> +aarch64_create_target_description (uint64_t vq, bool pauth_p, bool
>>> mte_p,
>>> + bool tls_p)
>>> {
>>> target_desc_up tdesc = allocate_target_description ();
>>> @@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq,
>>> bool pauth_p, bool mte_p)
>>> if (mte_p)
>>> regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
>>> + if (tls_p)
>>> + regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
>>> +
>>> return tdesc.release ();
>>> }
>>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>>> index e416e346e9a..79821666ce6 100644
>>> --- a/gdb/arch/aarch64.h
>>> +++ b/gdb/arch/aarch64.h
>>> @@ -29,6 +29,7 @@ struct aarch64_features
>>> bool sve = false;
>>> bool pauth = false;
>>> bool mte = false;
>>> + bool tls = false;
>>> };
>>> /* Create the aarch64 target description. A non zero VQ value
>>> indicates both
>>> @@ -36,10 +37,12 @@ struct aarch64_features
>>> an SVE Z register. HAS_PAUTH_P indicates the presence of the
>>> PAUTH
>>> feature.
>>> - MTE_P indicates the presence of the Memory Tagging Extension
>>> feature. */
>>> + MTE_P indicates the presence of the Memory Tagging Extension
>>> feature.
>>> +
>>> + TLS_P indicates the presence of the Thread Local Storage
>>> feature. */
>>> target_desc *aarch64_create_target_description (uint64_t vq, bool
>>> has_pauth_p,
>>> - bool mte_p);
>>> + bool mte_p, bool tls_p);
>>> /* Register numbers of various important registers.
>>> Note that on SVE, the Z registers reuse the V register numbers
>>> and the V
>>> @@ -84,6 +87,8 @@ enum aarch64_regnum
>>> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base
>>> + 1)
>>> #define AARCH64_PAUTH_REGS_SIZE (16)
>>> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
>>> +
>>> #define AARCH64_X_REGS_NUM 31
>>> #define AARCH64_V_REGS_NUM 32
>>> #define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
>>> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
>>> index 4b09819389a..946ec983df5 100644
>>> --- a/gdb/features/Makefile
>>> +++ b/gdb/features/Makefile
>>> @@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
>>> aarch64-fpu.xml \
>>> aarch64-pauth.xml \
>>> aarch64-mte.xml \
>>> + aarch64-tls.xml \
>>> arc/v1-core.xml \
>>> arc/v1-aux.xml \
>>> arc/v2-core.xml \
>>> diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
>>> new file mode 100644
>>> index 00000000000..30d730dffba
>>> --- /dev/null
>>> +++ b/gdb/features/aarch64-tls.c
>>> @@ -0,0 +1,14 @@
>>> +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
>>> + Original: aarch64-tls.xml */
>>> +
>>> +#include "gdbsupport/tdesc.h"
>>> +
>>> +static int
>>> +create_feature_aarch64_tls (struct target_desc *result, long regnum)
>>> +{
>>> + struct tdesc_feature *feature;
>>> +
>>> + feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
>>> + tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64,
>>> "data_ptr");
>>> + return regnum;
>>> +}
>>> diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
>>> new file mode 100644
>>> index 00000000000..48e0e9a6030
>>> --- /dev/null
>>> +++ b/gdb/features/aarch64-tls.xml
>>> @@ -0,0 +1,12 @@
>>> +<?xml version="1.0"?>
>>> +<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
>>> + Contributed by ARM Ltd.
>>> +
>>> + Copying and distribution of this file, with or without
>>> modification,
>>> + are permitted in any medium without royalty provided the copyright
>>> + notice and this notice are preserved. -->
>>> +
>>> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
>>> +<feature name="org.gnu.gdb.aarch64.tls">
>>> + <reg name="tpidr" bitsize="64" type="data_ptr"/>
>>> +</feature>
>>> diff --git a/gdbserver/linux-aarch64-tdesc.cc
>>> b/gdbserver/linux-aarch64-tdesc.cc
>>> index e982ab85067..14d6a4f80eb 100644
>>> --- a/gdbserver/linux-aarch64-tdesc.cc
>>> +++ b/gdbserver/linux-aarch64-tdesc.cc
>>> @@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool
>>> pauth_p, bool mte_p)
>>> if (tdesc == NULL)
>>> {
>>> - tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>>> + tdesc = aarch64_create_target_description (vq, pauth_p, mte_p,
>>> false);
>>> static const char *expedite_regs_aarch64[] = { "x29", "sp",
>>> "pc", NULL };
>>> static const char *expedite_regs_aarch64_sve[] = { "x29",
>>> "sp", "pc",
>>> diff --git a/gdbserver/netbsd-aarch64-low.cc
>>> b/gdbserver/netbsd-aarch64-low.cc
>>> index 202bf1cdac6..b371e599232 100644
>>> --- a/gdbserver/netbsd-aarch64-low.cc
>>> +++ b/gdbserver/netbsd-aarch64-low.cc
>>> @@ -96,7 +96,7 @@ void
>>> netbsd_aarch64_target::low_arch_setup ()
>>> {
>>> target_desc *tdesc
>>> - = aarch64_create_target_description (0, false);
>>> + = aarch64_create_target_description (0, false, false, false);
>>> static const char *expedite_regs_aarch64[] = { "x29", "sp",
>>> "pc", NULL };
>>> init_target_desc (tdesc, expedite_regs_aarch64);
>>
>> LGTM from aarch64's side.
>
> Hmm, did you mean to send this in response to the V3 series posted last
> week?
>
Indeed. I went through v3 before, but when I went to reply to it, my
filters had sent it to a separate folder. So yeah, this was for v3.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/12] Read the tpidr register from NT_ARM_TLS core dump notes on FreeBSD/Aarch64.
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
` (7 preceding siblings ...)
2022-03-23 21:00 ` [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register John Baldwin
@ 2022-03-23 21:00 ` John Baldwin
8 siblings, 0 replies; 26+ messages in thread
From: John Baldwin @ 2022-03-23 21:00 UTC (permalink / raw)
To: binutils, gdb-patches
---
gdb/aarch64-fbsd-tdep.c | 34 ++++++++++++++++++++++++++++++++++
gdb/aarch64-fbsd-tdep.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/gdb/aarch64-fbsd-tdep.c b/gdb/aarch64-fbsd-tdep.c
index 32f441892a8..3fb1010d43e 100644
--- a/gdb/aarch64-fbsd-tdep.c
+++ b/gdb/aarch64-fbsd-tdep.c
@@ -142,10 +142,42 @@ aarch64_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
void *cb_data,
const struct regcache *regcache)
{
+ aarch64_gdbarch_tdep *tdep = (aarch64_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+
cb (".reg", AARCH64_FBSD_SIZEOF_GREGSET, AARCH64_FBSD_SIZEOF_GREGSET,
&aarch64_fbsd_gregset, NULL, cb_data);
cb (".reg2", AARCH64_FBSD_SIZEOF_FPREGSET, AARCH64_FBSD_SIZEOF_FPREGSET,
&aarch64_fbsd_fpregset, NULL, cb_data);
+
+ if (tdep->has_tls ())
+ {
+ /* Create this on the fly in order to handle the variable location. */
+ const struct regcache_map_entry tls_regmap[] =
+ {
+ { 1, AARCH64_TPIDR_REGNUM (tdep->tls_reg_base), 8},
+ { 0 }
+ };
+
+ const struct regset aarch64_fbsd_tls_regset =
+ {
+ tls_regmap, regcache_supply_regset, regcache_collect_regset
+ };
+
+ cb (".reg-aarch-tls", AARCH64_FBSD_SIZEOF_TLSREGSET,
+ AARCH64_FBSD_SIZEOF_TLSREGSET, &aarch64_fbsd_tls_regset,
+ "TLS register", cb_data);
+ }
+}
+
+/* Implement the "core_read_description" gdbarch method. */
+
+static const struct target_desc *
+aarch64_fbsd_core_read_description (struct gdbarch *gdbarch,
+ struct target_ops *target, bfd *abfd)
+{
+ asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
+
+ return aarch64_read_description (0, false, false, tls != nullptr);
}
/* Implement the 'init_osabi' method of struct gdb_osabi_handler. */
@@ -168,6 +200,8 @@ aarch64_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_iterate_over_regset_sections
(gdbarch, aarch64_fbsd_iterate_over_regset_sections);
+ set_gdbarch_core_read_description (gdbarch,
+ aarch64_fbsd_core_read_description);
}
void _initialize_aarch64_fbsd_tdep ();
diff --git a/gdb/aarch64-fbsd-tdep.h b/gdb/aarch64-fbsd-tdep.h
index fc8fbee8843..7419ea6be03 100644
--- a/gdb/aarch64-fbsd-tdep.h
+++ b/gdb/aarch64-fbsd-tdep.h
@@ -32,6 +32,9 @@
alignment. */
#define AARCH64_FBSD_SIZEOF_FPREGSET (33 * V_REGISTER_SIZE)
+/* The TLS regset consists of a single register. */
+#define AARCH64_FBSD_SIZEOF_TLSREGSET (X_REGISTER_SIZE)
+
extern const struct regset aarch64_fbsd_gregset;
extern const struct regset aarch64_fbsd_fpregset;
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread