[v2,2/6] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap
Commit Message
This patch doesn't change gdbserver behaviour, but after later changes are
made it avoids a null pointer dereference when HWCAP needs to be obtained
for a specific process while current_thread is nullptr.
Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
parameter seems more correct than setting current_thread in one particular
code path.
Changes are propagated to allow passing the new parameter through the call
chain.
---
gdbserver/linux-aarch64-low.cc | 7 ++++---
gdbserver/linux-arm-low.cc | 2 +-
gdbserver/linux-low.cc | 18 +++++++++---------
gdbserver/linux-low.h | 9 ++++-----
gdbserver/linux-ppc-low.cc | 6 +++---
gdbserver/linux-s390-low.cc | 2 +-
gdbserver/netbsd-low.cc | 4 +---
gdbserver/netbsd-low.h | 2 +-
gdbserver/server.cc | 2 +-
gdbserver/target.cc | 4 ++--
gdbserver/target.h | 2 +-
11 files changed, 28 insertions(+), 30 deletions(-)
Comments
Hi,
On 11/26/22 02:04, Thiago Jung Bauermann wrote:
> This patch doesn't change gdbserver behaviour, but after later changes are
> made it avoids a null pointer dereference when HWCAP needs to be obtained
> for a specific process while current_thread is nullptr.
>
> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
> parameter seems more correct than setting current_thread in one particular
> code path.
I'm wondering if passing the pid is needed at all in gdbserver for the purposes of fetching the hwcap entries from auxv.
While current thread can be nullptr, I suppose current_process will never be nullptr if we have a valid inferior. And the auxv entries are per-process rather than per-thread.
There is a bit of a corner case when we have extended-remote without a live process and we need to fetch the hwcap bits to determine a particular feature to report via qSupported. But I suppose this is not what this change is trying to address, is it?
If not, it may be the case that we don't need these changes, and we can just use current_process.
Does that make sense?
>
> Changes are propagated to allow passing the new parameter through the call
> chain.
> ---
> gdbserver/linux-aarch64-low.cc | 7 ++++---
> gdbserver/linux-arm-low.cc | 2 +-
> gdbserver/linux-low.cc | 18 +++++++++---------
> gdbserver/linux-low.h | 9 ++++-----
> gdbserver/linux-ppc-low.cc | 6 +++---
> gdbserver/linux-s390-low.cc | 2 +-
> gdbserver/netbsd-low.cc | 4 +---
> gdbserver/netbsd-low.h | 2 +-
> gdbserver/server.cc | 2 +-
> gdbserver/target.cc | 4 ++--
> gdbserver/target.h | 2 +-
> 11 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index db5086962612..a6ed68f93029 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -823,12 +823,13 @@ aarch64_target::low_arch_setup ()
> if (is_elf64)
> {
> struct aarch64_features features;
> + int pid = pid_of (current_thread);
>
> features.vq = aarch64_sve_get_vq (tid);
> /* A-profile PAC is 64-bit only. */
> - features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA;
> + features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
> /* A-profile MTE is 64-bit only. */
> - features.mte = linux_get_hwcap2 (8) & HWCAP2_MTE;
> + features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE;
> features.tls = true;
>
> current_process ()->tdesc = aarch64_linux_read_description (features);
> @@ -3299,7 +3300,7 @@ aarch64_target::supports_memory_tagging ()
> #endif
> }
>
> - return (linux_get_hwcap2 (8) & HWCAP2_MTE) != 0;
> + return (linux_get_hwcap2 (pid_of (current_thread), 8) & HWCAP2_MTE) != 0;
> }
>
> bool
> diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc
> index a458b0f14a68..1420909626e8 100644
> --- a/gdbserver/linux-arm-low.cc
> +++ b/gdbserver/linux-arm-low.cc
> @@ -958,7 +958,7 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self)
> static const struct target_desc *
> arm_read_description (void)
> {
> - unsigned long arm_hwcap = linux_get_hwcap (4);
> + unsigned long arm_hwcap = linux_get_hwcap (pid_of (current_thread), 4);
>
> if (arm_hwcap & HWCAP_IWMMXT)
> return arm_linux_read_description (ARM_FP_TYPE_IWMMXT);
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index a896b37528b6..fc496275d6a3 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -5480,12 +5480,11 @@ linux_process_target::supports_read_auxv ()
> to debugger memory starting at MYADDR. */
>
> int
> -linux_process_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> - unsigned int len)
> +linux_process_target::read_auxv (int pid, CORE_ADDR offset,
> + unsigned char *myaddr, unsigned int len)
> {
> char filename[PATH_MAX];
> int fd, n;
> - int pid = lwpid_of (current_thread);
>
> xsnprintf (filename, sizeof filename, "/proc/%d/auxv", pid);
>
> @@ -6979,14 +6978,15 @@ linux_get_pc_64bit (struct regcache *regcache)
> /* See linux-low.h. */
>
> int
> -linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)
> +linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp)
> {
> gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
> int offset = 0;
>
> gdb_assert (wordsize == 4 || wordsize == 8);
>
> - while (the_target->read_auxv (offset, data, 2 * wordsize) == 2 * wordsize)
> + while (the_target->read_auxv (pid, offset, data, 2 * wordsize)
> + == 2 * wordsize)
> {
> if (wordsize == 4)
> {
> @@ -7016,20 +7016,20 @@ linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)
> /* See linux-low.h. */
>
> CORE_ADDR
> -linux_get_hwcap (int wordsize)
> +linux_get_hwcap (int pid, int wordsize)
> {
> CORE_ADDR hwcap = 0;
> - linux_get_auxv (wordsize, AT_HWCAP, &hwcap);
> + linux_get_auxv (pid, wordsize, AT_HWCAP, &hwcap);
> return hwcap;
> }
>
> /* See linux-low.h. */
>
> CORE_ADDR
> -linux_get_hwcap2 (int wordsize)
> +linux_get_hwcap2 (int pid, int wordsize)
> {
> CORE_ADDR hwcap2 = 0;
> - linux_get_auxv (wordsize, AT_HWCAP2, &hwcap2);
> + linux_get_auxv (pid, wordsize, AT_HWCAP2, &hwcap2);
> return hwcap2;
> }
>
> diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
> index 1594f063f47c..182a540f3bb3 100644
> --- a/gdbserver/linux-low.h
> +++ b/gdbserver/linux-low.h
> @@ -178,7 +178,7 @@ class linux_process_target : public process_stratum_target
>
> bool supports_read_auxv () override;
>
> - int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> + int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
> unsigned int len) override;
>
> int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
> @@ -946,17 +946,16 @@ extern int have_ptrace_getregset;
> *VALP and return 1. If not found or if there is an error, return
> 0. */
>
> -int linux_get_auxv (int wordsize, CORE_ADDR match,
> - CORE_ADDR *valp);
> +int linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp);
>
> /* Fetch the AT_HWCAP entry from the auxv vector, where entries are length
> WORDSIZE. If no entry was found, return zero. */
>
> -CORE_ADDR linux_get_hwcap (int wordsize);
> +CORE_ADDR linux_get_hwcap (int pid, int wordsize);
>
> /* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length
> WORDSIZE. If no entry was found, return zero. */
>
> -CORE_ADDR linux_get_hwcap2 (int wordsize);
> +CORE_ADDR linux_get_hwcap2 (int pid, int wordsize);
>
> #endif /* GDBSERVER_LINUX_LOW_H */
> diff --git a/gdbserver/linux-ppc-low.cc b/gdbserver/linux-ppc-low.cc
> index 08824887003b..1db20d998ffd 100644
> --- a/gdbserver/linux-ppc-low.cc
> +++ b/gdbserver/linux-ppc-low.cc
> @@ -894,8 +894,8 @@ ppc_target::low_arch_setup ()
>
> /* The value of current_process ()->tdesc needs to be set for this
> call. */
> - ppc_hwcap = linux_get_hwcap (features.wordsize);
> - ppc_hwcap2 = linux_get_hwcap2 (features.wordsize);
> + ppc_hwcap = linux_get_hwcap (pid_of (current_thread), features.wordsize);
> + ppc_hwcap2 = linux_get_hwcap2 (pid_of (current_thread), features.wordsize);
>
> features.isa205 = ppc_linux_has_isa205 (ppc_hwcap);
>
> @@ -1097,7 +1097,7 @@ is_elfv2_inferior (void)
> const struct target_desc *tdesc = current_process ()->tdesc;
> int wordsize = register_size (tdesc, 0);
>
> - if (!linux_get_auxv (wordsize, AT_PHDR, &phdr))
> + if (!linux_get_auxv (pid_of (current_thread), wordsize, AT_PHDR, &phdr))
> return def_res;
>
> /* Assume ELF header is at the beginning of the page where program headers
> diff --git a/gdbserver/linux-s390-low.cc b/gdbserver/linux-s390-low.cc
> index 5adc28070574..2d030a806350 100644
> --- a/gdbserver/linux-s390-low.cc
> +++ b/gdbserver/linux-s390-low.cc
> @@ -592,7 +592,7 @@ s390_target::low_arch_setup ()
> /* Determine word size and HWCAP. */
> int pid = pid_of (current_thread);
> int wordsize = s390_get_wordsize (pid);
> - unsigned long hwcap = linux_get_hwcap (wordsize);
> + unsigned long hwcap = linux_get_hwcap (pid, wordsize);
>
> /* Check whether the kernel supports extra register sets. */
> int have_regset_last_break
> diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
> index f05bcd4e173f..03af80049457 100644
> --- a/gdbserver/netbsd-low.cc
> +++ b/gdbserver/netbsd-low.cc
> @@ -581,11 +581,9 @@ netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
> to debugger memory starting at MYADDR. */
>
> int
> -netbsd_process_target::read_auxv (CORE_ADDR offset,
> +netbsd_process_target::read_auxv (int pid, CORE_ADDR offset,
> unsigned char *myaddr, unsigned int len)
> {
> - pid_t pid = pid_of (current_thread);
> -
> return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
> }
>
> diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
> index 06b4fd613571..eced7b3dd860 100644
> --- a/gdbserver/netbsd-low.h
> +++ b/gdbserver/netbsd-low.h
> @@ -77,7 +77,7 @@ class netbsd_process_target : public process_stratum_target
>
> bool supports_read_auxv () override;
>
> - int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> + int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
> unsigned int len) override;
>
> bool supports_hardware_single_step () override;
> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
> index aaef38e00622..63c323071670 100644
> --- a/gdbserver/server.cc
> +++ b/gdbserver/server.cc
> @@ -1443,7 +1443,7 @@ handle_qxfer_auxv (const char *annex,
> if (annex[0] != '\0' || current_thread == NULL)
> return -1;
>
> - return the_target->read_auxv (offset, readbuf, len);
> + return the_target->read_auxv (pid_of (current_thread), offset, readbuf, len);
> }
>
> /* Handle qXfer:exec-file:read. */
> diff --git a/gdbserver/target.cc b/gdbserver/target.cc
> index c06a67600b1f..3651dbb9e862 100644
> --- a/gdbserver/target.cc
> +++ b/gdbserver/target.cc
> @@ -344,8 +344,8 @@ process_stratum_target::supports_read_auxv ()
> }
>
> int
> -process_stratum_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> - unsigned int len)
> +process_stratum_target::read_auxv (int pid, CORE_ADDR offset,
> + unsigned char *myaddr, unsigned int len)
> {
> gdb_assert_not_reached ("target op read_auxv not supported");
> }
> diff --git a/gdbserver/target.h b/gdbserver/target.h
> index 18ab969dda70..fe7a80b645bc 100644
> --- a/gdbserver/target.h
> +++ b/gdbserver/target.h
> @@ -175,7 +175,7 @@ class process_stratum_target
> /* Read auxiliary vector data from the inferior process.
>
> Read LEN bytes at OFFSET into a buffer at MYADDR. */
> - virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> + virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
> unsigned int len);
>
> /* Returns true if GDB Z breakpoint type TYPE is supported, false
On 11/28/22 06:50, Luis Machado via Gdb-patches wrote:
> Hi,
>
> On 11/26/22 02:04, Thiago Jung Bauermann wrote:
>> This patch doesn't change gdbserver behaviour, but after later changes are
>> made it avoids a null pointer dereference when HWCAP needs to be obtained
>> for a specific process while current_thread is nullptr.
>>
>> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
>> parameter seems more correct than setting current_thread in one particular
>> code path.
>
> I'm wondering if passing the pid is needed at all in gdbserver for the purposes of fetching the hwcap entries from auxv.
>
> While current thread can be nullptr, I suppose current_process will never be nullptr if we have a valid inferior. And the auxv entries are per-process rather than per-thread.
>
> There is a bit of a corner case when we have extended-remote without a live process and we need to fetch the hwcap bits to determine a particular feature to report via qSupported. But I suppose this is not what this change is trying to address, is it?
>
> If not, it may be the case that we don't need these changes, and we can just use current_process.
Even if we could just use current_process, this patch is going in the
general direction I like, which is to reduce referring to the global
state everywhere, try to fetch it only at the entry points and then pass
the necessary context down by parameters.
Simon
On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
> This patch doesn't change gdbserver behaviour, but after later changes are
> made it avoids a null pointer dereference when HWCAP needs to be obtained
> for a specific process while current_thread is nullptr.
>
> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
> parameter seems more correct than setting current_thread in one particular
> code path.
>
> Changes are propagated to allow passing the new parameter through the call
> chain.
Thanks, this is ok, with the following nits fixed:
Approved-By: Simon Marchi <simon.marchi@efficios.com>
... unless Luis is strongly against it.
> ---
> gdbserver/linux-aarch64-low.cc | 7 ++++---
> gdbserver/linux-arm-low.cc | 2 +-
> gdbserver/linux-low.cc | 18 +++++++++---------
> gdbserver/linux-low.h | 9 ++++-----
> gdbserver/linux-ppc-low.cc | 6 +++---
> gdbserver/linux-s390-low.cc | 2 +-
> gdbserver/netbsd-low.cc | 4 +---
> gdbserver/netbsd-low.h | 2 +-
> gdbserver/server.cc | 2 +-
> gdbserver/target.cc | 4 ++--
> gdbserver/target.h | 2 +-
> 11 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index db5086962612..a6ed68f93029 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -823,12 +823,13 @@ aarch64_target::low_arch_setup ()
> if (is_elf64)
> {
> struct aarch64_features features;
> + int pid = pid_of (current_thread);
>
> features.vq = aarch64_sve_get_vq (tid);
> /* A-profile PAC is 64-bit only. */
> - features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA;
> + features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
Avoid adding the outer parenthesis.
> diff --git a/gdbserver/target.h b/gdbserver/target.h
> index 18ab969dda70..fe7a80b645bc 100644
> --- a/gdbserver/target.h
> +++ b/gdbserver/target.h
> @@ -175,7 +175,7 @@ class process_stratum_target
> /* Read auxiliary vector data from the inferior process.
>
> Read LEN bytes at OFFSET into a buffer at MYADDR. */
> - virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> + virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
Please update the doc of the method to replace "from the inferior
process" to from "process with pid PID".
Simon
On 11/28/22 15:07, Simon Marchi wrote:
>
>
> On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
>> This patch doesn't change gdbserver behaviour, but after later changes are
>> made it avoids a null pointer dereference when HWCAP needs to be obtained
>> for a specific process while current_thread is nullptr.
>>
>> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
>> parameter seems more correct than setting current_thread in one particular
>> code path.
>>
>> Changes are propagated to allow passing the new parameter through the call
>> chain.
>
> Thanks, this is ok, with the following nits fixed:
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>
> ... unless Luis is strongly against it.
Not at all. Given gdbserver's knowledge of the current ptid is implicit (set by remote packets), I thought this would simplify the series a bit.
>
>> ---
>> gdbserver/linux-aarch64-low.cc | 7 ++++---
>> gdbserver/linux-arm-low.cc | 2 +-
>> gdbserver/linux-low.cc | 18 +++++++++---------
>> gdbserver/linux-low.h | 9 ++++-----
>> gdbserver/linux-ppc-low.cc | 6 +++---
>> gdbserver/linux-s390-low.cc | 2 +-
>> gdbserver/netbsd-low.cc | 4 +---
>> gdbserver/netbsd-low.h | 2 +-
>> gdbserver/server.cc | 2 +-
>> gdbserver/target.cc | 4 ++--
>> gdbserver/target.h | 2 +-
>> 11 files changed, 28 insertions(+), 30 deletions(-)
>>
>> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
>> index db5086962612..a6ed68f93029 100644
>> --- a/gdbserver/linux-aarch64-low.cc
>> +++ b/gdbserver/linux-aarch64-low.cc
>> @@ -823,12 +823,13 @@ aarch64_target::low_arch_setup ()
>> if (is_elf64)
>> {
>> struct aarch64_features features;
>> + int pid = pid_of (current_thread);
>>
>> features.vq = aarch64_sve_get_vq (tid);
>> /* A-profile PAC is 64-bit only. */
>> - features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA;
>> + features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
>
> Avoid adding the outer parenthesis.
>
>> diff --git a/gdbserver/target.h b/gdbserver/target.h
>> index 18ab969dda70..fe7a80b645bc 100644
>> --- a/gdbserver/target.h
>> +++ b/gdbserver/target.h
>> @@ -175,7 +175,7 @@ class process_stratum_target
>> /* Read auxiliary vector data from the inferior process.
>>
>> Read LEN bytes at OFFSET into a buffer at MYADDR. */
>> - virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
>> + virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
>
> Please update the doc of the method to replace "from the inferior
> process" to from "process with pid PID".
>
> Simon
Simon Marchi <simark@simark.ca> writes:
> On 11/28/22 06:50, Luis Machado via Gdb-patches wrote:
>> Hi,
>>
>> On 11/26/22 02:04, Thiago Jung Bauermann wrote:
>>> This patch doesn't change gdbserver behaviour, but after later changes are
>>> made it avoids a null pointer dereference when HWCAP needs to be obtained
>>> for a specific process while current_thread is nullptr.
>>>
>>> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
>>> parameter seems more correct than setting current_thread in one particular
>>> code path.
>>
>> I'm wondering if passing the pid is needed at all in gdbserver for the purposes of
>> fetching the hwcap entries from auxv.
>>
>> While current thread can be nullptr, I suppose current_process will never be nullptr if
>> we have a valid inferior. And the auxv entries are per-process rather than per-thread.
>>
>> There is a bit of a corner case when we have extended-remote without a live process and
>> we need to fetch the hwcap bits to determine a particular feature to report via
>> qSupported. But I suppose this is not what this change is trying to address, is it?
>>
>> If not, it may be the case that we don't need these changes, and we can just use
>> current_process.
>
> Even if we could just use current_process, this patch is going in the
> general direction I like, which is to reduce referring to the global
> state everywhere, try to fetch it only at the entry points and then pass
> the necessary context down by parameters.
Thanks. As I looked into how to fix the problem I ran into, I thought
this was a nice cleanup that would solve my problem and couldn't resist
doing it. :-)
Simon Marchi <simark@simark.ca> writes:
> On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
>> This patch doesn't change gdbserver behaviour, but after later changes are
>> made it avoids a null pointer dereference when HWCAP needs to be obtained
>> for a specific process while current_thread is nullptr.
>>
>> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
>> parameter seems more correct than setting current_thread in one particular
>> code path.
>>
>> Changes are propagated to allow passing the new parameter through the call
>> chain.
>
> Thanks, this is ok, with the following nits fixed:
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>
> ... unless Luis is strongly against it.
Thanks!
v3 will have the nits fixed and also change uses of “pid_of (thread)” to
“thread->id.pid ()”.
>> ---
>> gdbserver/linux-aarch64-low.cc | 7 ++++---
>> gdbserver/linux-arm-low.cc | 2 +-
>> gdbserver/linux-low.cc | 18 +++++++++---------
>> gdbserver/linux-low.h | 9 ++++-----
>> gdbserver/linux-ppc-low.cc | 6 +++---
>> gdbserver/linux-s390-low.cc | 2 +-
>> gdbserver/netbsd-low.cc | 4 +---
>> gdbserver/netbsd-low.h | 2 +-
>> gdbserver/server.cc | 2 +-
>> gdbserver/target.cc | 4 ++--
>> gdbserver/target.h | 2 +-
>> 11 files changed, 28 insertions(+), 30 deletions(-)
>>
>> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
>> index db5086962612..a6ed68f93029 100644
>> --- a/gdbserver/linux-aarch64-low.cc
>> +++ b/gdbserver/linux-aarch64-low.cc
>> @@ -823,12 +823,13 @@ aarch64_target::low_arch_setup ()
>> if (is_elf64)
>> {
>> struct aarch64_features features;
>> + int pid = pid_of (current_thread);
>>
>> features.vq = aarch64_sve_get_vq (tid);
>> /* A-profile PAC is 64-bit only. */
>> - features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA;
>> + features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
>
> Avoid adding the outer parenthesis.
Ah, that's a remnant from an older version of the code where this line
was longer and I added the parentheses to fix indentation. Fixed for v3.
>> diff --git a/gdbserver/target.h b/gdbserver/target.h
>> index 18ab969dda70..fe7a80b645bc 100644
>> --- a/gdbserver/target.h
>> +++ b/gdbserver/target.h
>> @@ -175,7 +175,7 @@ class process_stratum_target
>> /* Read auxiliary vector data from the inferior process.
>>
>> Read LEN bytes at OFFSET into a buffer at MYADDR. */
>> - virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
>> + virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
>
> Please update the doc of the method to replace "from the inferior
> process" to from "process with pid PID".
Indeed, thanks for spotting it. Fixed for v3.
@@ -823,12 +823,13 @@ aarch64_target::low_arch_setup ()
if (is_elf64)
{
struct aarch64_features features;
+ int pid = pid_of (current_thread);
features.vq = aarch64_sve_get_vq (tid);
/* A-profile PAC is 64-bit only. */
- features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA;
+ features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
/* A-profile MTE is 64-bit only. */
- features.mte = linux_get_hwcap2 (8) & HWCAP2_MTE;
+ features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE;
features.tls = true;
current_process ()->tdesc = aarch64_linux_read_description (features);
@@ -3299,7 +3300,7 @@ aarch64_target::supports_memory_tagging ()
#endif
}
- return (linux_get_hwcap2 (8) & HWCAP2_MTE) != 0;
+ return (linux_get_hwcap2 (pid_of (current_thread), 8) & HWCAP2_MTE) != 0;
}
bool
@@ -958,7 +958,7 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self)
static const struct target_desc *
arm_read_description (void)
{
- unsigned long arm_hwcap = linux_get_hwcap (4);
+ unsigned long arm_hwcap = linux_get_hwcap (pid_of (current_thread), 4);
if (arm_hwcap & HWCAP_IWMMXT)
return arm_linux_read_description (ARM_FP_TYPE_IWMMXT);
@@ -5480,12 +5480,11 @@ linux_process_target::supports_read_auxv ()
to debugger memory starting at MYADDR. */
int
-linux_process_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr,
- unsigned int len)
+linux_process_target::read_auxv (int pid, CORE_ADDR offset,
+ unsigned char *myaddr, unsigned int len)
{
char filename[PATH_MAX];
int fd, n;
- int pid = lwpid_of (current_thread);
xsnprintf (filename, sizeof filename, "/proc/%d/auxv", pid);
@@ -6979,14 +6978,15 @@ linux_get_pc_64bit (struct regcache *regcache)
/* See linux-low.h. */
int
-linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)
+linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp)
{
gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
int offset = 0;
gdb_assert (wordsize == 4 || wordsize == 8);
- while (the_target->read_auxv (offset, data, 2 * wordsize) == 2 * wordsize)
+ while (the_target->read_auxv (pid, offset, data, 2 * wordsize)
+ == 2 * wordsize)
{
if (wordsize == 4)
{
@@ -7016,20 +7016,20 @@ linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)
/* See linux-low.h. */
CORE_ADDR
-linux_get_hwcap (int wordsize)
+linux_get_hwcap (int pid, int wordsize)
{
CORE_ADDR hwcap = 0;
- linux_get_auxv (wordsize, AT_HWCAP, &hwcap);
+ linux_get_auxv (pid, wordsize, AT_HWCAP, &hwcap);
return hwcap;
}
/* See linux-low.h. */
CORE_ADDR
-linux_get_hwcap2 (int wordsize)
+linux_get_hwcap2 (int pid, int wordsize)
{
CORE_ADDR hwcap2 = 0;
- linux_get_auxv (wordsize, AT_HWCAP2, &hwcap2);
+ linux_get_auxv (pid, wordsize, AT_HWCAP2, &hwcap2);
return hwcap2;
}
@@ -178,7 +178,7 @@ class linux_process_target : public process_stratum_target
bool supports_read_auxv () override;
- int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
+ int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
unsigned int len) override;
int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
@@ -946,17 +946,16 @@ extern int have_ptrace_getregset;
*VALP and return 1. If not found or if there is an error, return
0. */
-int linux_get_auxv (int wordsize, CORE_ADDR match,
- CORE_ADDR *valp);
+int linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp);
/* Fetch the AT_HWCAP entry from the auxv vector, where entries are length
WORDSIZE. If no entry was found, return zero. */
-CORE_ADDR linux_get_hwcap (int wordsize);
+CORE_ADDR linux_get_hwcap (int pid, int wordsize);
/* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length
WORDSIZE. If no entry was found, return zero. */
-CORE_ADDR linux_get_hwcap2 (int wordsize);
+CORE_ADDR linux_get_hwcap2 (int pid, int wordsize);
#endif /* GDBSERVER_LINUX_LOW_H */
@@ -894,8 +894,8 @@ ppc_target::low_arch_setup ()
/* The value of current_process ()->tdesc needs to be set for this
call. */
- ppc_hwcap = linux_get_hwcap (features.wordsize);
- ppc_hwcap2 = linux_get_hwcap2 (features.wordsize);
+ ppc_hwcap = linux_get_hwcap (pid_of (current_thread), features.wordsize);
+ ppc_hwcap2 = linux_get_hwcap2 (pid_of (current_thread), features.wordsize);
features.isa205 = ppc_linux_has_isa205 (ppc_hwcap);
@@ -1097,7 +1097,7 @@ is_elfv2_inferior (void)
const struct target_desc *tdesc = current_process ()->tdesc;
int wordsize = register_size (tdesc, 0);
- if (!linux_get_auxv (wordsize, AT_PHDR, &phdr))
+ if (!linux_get_auxv (pid_of (current_thread), wordsize, AT_PHDR, &phdr))
return def_res;
/* Assume ELF header is at the beginning of the page where program headers
@@ -592,7 +592,7 @@ s390_target::low_arch_setup ()
/* Determine word size and HWCAP. */
int pid = pid_of (current_thread);
int wordsize = s390_get_wordsize (pid);
- unsigned long hwcap = linux_get_hwcap (wordsize);
+ unsigned long hwcap = linux_get_hwcap (pid, wordsize);
/* Check whether the kernel supports extra register sets. */
int have_regset_last_break
@@ -581,11 +581,9 @@ netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
to debugger memory starting at MYADDR. */
int
-netbsd_process_target::read_auxv (CORE_ADDR offset,
+netbsd_process_target::read_auxv (int pid, CORE_ADDR offset,
unsigned char *myaddr, unsigned int len)
{
- pid_t pid = pid_of (current_thread);
-
return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
}
@@ -77,7 +77,7 @@ class netbsd_process_target : public process_stratum_target
bool supports_read_auxv () override;
- int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
+ int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
unsigned int len) override;
bool supports_hardware_single_step () override;
@@ -1443,7 +1443,7 @@ handle_qxfer_auxv (const char *annex,
if (annex[0] != '\0' || current_thread == NULL)
return -1;
- return the_target->read_auxv (offset, readbuf, len);
+ return the_target->read_auxv (pid_of (current_thread), offset, readbuf, len);
}
/* Handle qXfer:exec-file:read. */
@@ -344,8 +344,8 @@ process_stratum_target::supports_read_auxv ()
}
int
-process_stratum_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr,
- unsigned int len)
+process_stratum_target::read_auxv (int pid, CORE_ADDR offset,
+ unsigned char *myaddr, unsigned int len)
{
gdb_assert_not_reached ("target op read_auxv not supported");
}
@@ -175,7 +175,7 @@ class process_stratum_target
/* Read auxiliary vector data from the inferior process.
Read LEN bytes at OFFSET into a buffer at MYADDR. */
- virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
+ virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr,
unsigned int len);
/* Returns true if GDB Z breakpoint type TYPE is supported, false