[3/7] Arm: Add read_description read funcs and use in GDB
Commit Message
Switch the Arm target to get target descriptions via arm_read_description
and aarch32_read_description, in the same style as other feature targets.
Add an enum to specify the different types - this will also be of use to
gdbserver in a later patch.
Call arm_create_mprofile_target_description directly as these will only be
called the once, therefore they do not need caching.
Under the hood return the same existing pre-feature target descriptions.
Note: This commit will break the AArch64 gdbserver build.
2019-07-05 Alan Hayward <alan.hayward@arm.com>
* Makefile.in: Add new files.
* aarch32-tdep.c: New file.
* aarch32-tdep.h: New file.
* aarch64-linux-nat.c (aarch64_linux_nat_target::read_description):
Call aarch32_read_description.
* arch/aarch32.c: New file.
* arch/aarch32.h: New file.
* arch/arm.c (arm_create_target_description)
(arm_create_mprofile_target_description): New function.
* arch/arm.h (arm_fp_type, arm_m_profile_type): New enum.
(arm_create_target_description)
(arm_create_mprofile_target_description): New declaration.
* arm-fbsd-tdep.c (arm_fbsd_read_description_auxv): Call
read_description functions.
* arm-linux-nat.c (arm_linux_nat_target::read_description):
Likewise.
* arm-linux-tdep.c (arm_linux_core_read_description): Likewise.
* arm-tdep.c (tdesc_arm_list): New variable.
(arm_register_g_packet_guesses): Call create description functions.
(arm_read_description): New function.
* arm-tdep.h (arm_read_description): Add declaration.
* configure.tgt: Add new files.
---
gdb/Makefile.in | 5 ++++
gdb/aarch32-tdep.c | 33 +++++++++++++++++++++++++
gdb/aarch32-tdep.h | 25 +++++++++++++++++++
gdb/aarch64-linux-nat.c | 6 ++---
gdb/arch/aarch32.c | 29 ++++++++++++++++++++++
gdb/arch/aarch32.h | 27 ++++++++++++++++++++
gdb/arch/arm.c | 55 +++++++++++++++++++++++++++++++++++++++++
gdb/arch/arm.h | 27 ++++++++++++++++++++
gdb/arm-fbsd-tdep.c | 11 +++++----
gdb/arm-linux-nat.c | 11 +++++----
gdb/arm-linux-tdep.c | 11 +++++----
gdb/arm-tdep.c | 34 ++++++++++++++++++++++---
gdb/arm-tdep.h | 8 ++----
gdb/configure.tgt | 8 +++---
14 files changed, 259 insertions(+), 31 deletions(-)
create mode 100644 gdb/aarch32-tdep.c
create mode 100644 gdb/aarch32-tdep.h
create mode 100644 gdb/arch/aarch32.c
create mode 100644 gdb/arch/aarch32.h
Comments
On 2019-07-05 5:45 a.m., Alan Hayward wrote:
> Switch the Arm target to get target descriptions via arm_read_description
> and aarch32_read_description, in the same style as other feature targets.
> Add an enum to specify the different types - this will also be of use to
> gdbserver in a later patch.
>
> Call arm_create_mprofile_target_description directly as these will only be
> called the once, therefore they do not need caching.
I was going to point out "called the once", because it sounds strange to me,
but there is another instance of it in function arm_register_g_packet_guesses,
so maybe it's ok?
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 7308ea5767..9352dd92ff 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -665,7 +665,9 @@ ALL_64_TARGET_OBS = \
>
> # All other target-dependent objects files (used with --enable-targets=all).
> ALL_TARGET_OBS = \
> + aarch32-tdep.o \
> arc-tdep.o \
> + arch/aarch32.o \
> arch/arm.o \
> arch/arm-get-next-pcs.o \
> arch/arm-linux.o \
> @@ -1184,6 +1186,7 @@ SFILES = \
> # right, it is probably easiest just to list .h files here directly.
>
> HFILES_NO_SRCDIR = \
> + aarch32-tdep.h \
> aarch64-ravenscar-thread.h \
> aarch64-tdep.h \
> ada-lang.h \
> @@ -1431,6 +1434,7 @@ HFILES_NO_SRCDIR = \
> xtensa-tdep.h \
> arch/aarch64.h \
> arch/aarch64-insn.h \
> + arch/aarch32.h \
This line should go above the aarch64 ones.
> diff --git a/gdb/aarch32-tdep.c b/gdb/aarch32-tdep.c
> new file mode 100644
> index 0000000000..d9355d0665
> --- /dev/null
> +++ b/gdb/aarch32-tdep.c
> @@ -0,0 +1,33 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "common/common-defs.h"
> +#include "common/common-regcache.h"
> +#include "arch/aarch32.h"
> +
> +struct target_desc *tdesc_aarch32;
static
> +
> +/* See linux-aarch32-tdep.h. */
/* See aarch32-tdep.h. */
> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
> new file mode 100644
> index 0000000000..f3cb8c7855
> --- /dev/null
> +++ b/gdb/arch/aarch32.c
> @@ -0,0 +1,29 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "common/common-defs.h"
> +#include "aarch32.h"
> +
> +extern struct target_desc *tdesc_arm_with_neon;
> +
> +/* See aarch32.h. */
> +
> +target_desc *
> +aarch32_create_target_description ()
> +{
> + return tdesc_arm_with_neon;
> +}
Can you briefly explain the difference (from the point of view of GDB) between the
ARM and AArch32 architecture? My current understanding is that AArch32 is Arm(v7?)
emulation on AArch64, so they are very close. But there might be some subtle
differences, requiring GDB to consider them as different architectures.
Edit: I now went through the patch and saw the configure.in change. My guess would
now be that it allows to include in an AArch64 build just what's actually supported
by the AArch32 mode, which is arm-with-neon, without having to pull the entire Arm
support. Does that sound right?
> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
> index 93738f0a0f..37806753cb 100644
> --- a/gdb/arch/arm.c
> +++ b/gdb/arch/arm.c
> @@ -21,6 +21,15 @@
> #include "common/common-regcache.h"
> #include "arm.h"
>
> +extern struct target_desc *tdesc_arm_with_vfpv2;
> +extern struct target_desc *tdesc_arm_with_vfpv3;
> +extern struct target_desc *tdesc_arm_with_iwmmxt;
> +#ifndef GDBSERVER
> +extern struct target_desc *tdesc_arm_with_m;
> +extern struct target_desc *tdesc_arm_with_m_vfp_d16;
> +extern struct target_desc *tdesc_arm_with_m_fpa_layout;
> +#endif
> +
> /* See arm.h. */
>
> int
> @@ -372,3 +381,49 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
>
> return res & 0xffffffff;
> }
> +
> +/* See arch/arm.h. */
> +
> +target_desc *
> +arm_create_target_description (arm_fp_type fp_type)
> +{
> + switch (fp_type)
> + {
> + case ARM_FP_TYPE_NONE:
> + return nullptr;
> +
> + case ARM_FP_TYPE_VFPV2:
> + return tdesc_arm_with_vfpv2;
> +
> + case ARM_FP_TYPE_VFPV3:
> + return tdesc_arm_with_vfpv3;
> +
> + case ARM_FP_TYPE_IWMMXT:
> + return tdesc_arm_with_iwmmxt;
> +
> + default:
> + error (_("Invalid Arm FP type: %d"), fp_type);
> + }
> +}
> +
> +/* See arch/arm.h. */
> +
> +target_desc *
> +arm_create_mprofile_target_description (arm_m_profile_type m_type)
> +{
> + switch (m_type)
> + {
> +#ifndef GDBSERVER
> + case ARM_M_TYPE_M_PROFILE:
> + return tdesc_arm_with_m;
> +
> + case ARM_M_TYPE_VFP_D16:
> + return tdesc_arm_with_m_fpa_layout;
> +
> + case ARM_M_TYPE_WITH_FPA:
> + return tdesc_arm_with_m_vfp_d16;
> +#endif
> + default:
> + error (_("Invalid Arm M type: %d"), m_type);
> + }
> +}
If it doesn't make sense to have this function shared with GDBserver (given
that GDBserver doesn't run on Cortex-Ms), it should probably go in gdb/arm-tdep.c.
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 8e3607cdea..f3f6458a27 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -240,6 +240,9 @@ static const char **valid_disassembly_styles;
> /* Disassembly style to use. Default to "std" register names. */
> static const char *disassembly_style;
>
> +/* All possible arm target descriptors. */
> +struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID];
static
> +
> /* This is used to keep the bfd arch_info in sync with the disassembly
> style. */
> static void set_disassembly_style_sfunc (const char *, int,
> @@ -8763,7 +8766,6 @@ arm_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
> return default_register_reggroup_p (gdbarch, regnum, group);
> }
>
> -
> /* For backward-compatibility we allow two 'g' packet lengths with
> the remote protocol depending on whether FPA registers are
> supplied. M-profile targets do not have FPA registers, but some
> @@ -8777,21 +8779,29 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
> {
> if (gdbarch_tdep (gdbarch)->is_m)
> {
> + const target_desc *tdesc;
> +
> + /* This function is only called the once, therefore it's safe to call the
> + tdesc creation function directly. */
The other instance of "called the once".
Would it be "unsafe" to call tdesc creation functions multiple times in general? I
thought it was just a question of efficiency/caching. If so, I'd say "therefore
it's not worth caching the descriptions".
It might be true that they are called only once today (I guess because the only way to
debug them in practice is through some server that only support debugging one at the
time), but it could change eventually. For example, with multi-target, you could
connect to two of them. Since it's not really difficult, I'd use the same caching pattern
as for the other ones.
Thanks,
Simon
> On 10 Jul 2019, at 04:45, Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-07-05 5:45 a.m., Alan Hayward wrote:
>> Switch the Arm target to get target descriptions via arm_read_description
>> and aarch32_read_description, in the same style as other feature targets.
>> Add an enum to specify the different types - this will also be of use to
>> gdbserver in a later patch.
>>
>> Call arm_create_mprofile_target_description directly as these will only be
>> called the once, therefore they do not need caching.
>
> I was going to point out "called the once", because it sounds strange to me,
> but there is another instance of it in function arm_register_g_packet_guesses,
> so maybe it's ok?
Should probably read “called the once for each target description type”. But,
removed due to comments below.
>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 7308ea5767..9352dd92ff 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -665,7 +665,9 @@ ALL_64_TARGET_OBS = \
>>
>> # All other target-dependent objects files (used with --enable-targets=all).
>> ALL_TARGET_OBS = \
>> + aarch32-tdep.o \
>> arc-tdep.o \
>> + arch/aarch32.o \
>> arch/arm.o \
>> arch/arm-get-next-pcs.o \
>> arch/arm-linux.o \
>> @@ -1184,6 +1186,7 @@ SFILES = \
>> # right, it is probably easiest just to list .h files here directly.
>>
>> HFILES_NO_SRCDIR = \
>> + aarch32-tdep.h \
>> aarch64-ravenscar-thread.h \
>> aarch64-tdep.h \
>> ada-lang.h \
>> @@ -1431,6 +1434,7 @@ HFILES_NO_SRCDIR = \
>> xtensa-tdep.h \
>> arch/aarch64.h \
>> arch/aarch64-insn.h \
>> + arch/aarch32.h \
>
> This line should go above the aarch64 ones.
Done.
>
>> diff --git a/gdb/aarch32-tdep.c b/gdb/aarch32-tdep.c
>> new file mode 100644
>> index 0000000000..d9355d0665
>> --- /dev/null
>> +++ b/gdb/aarch32-tdep.c
>> @@ -0,0 +1,33 @@
>> +/* Copyright (C) 2019 Free Software Foundation, Inc.
>> +
>> + This file is part of GDB.
>> +
>> + This program is free software; you can redistribute it and/or modify
>> + it under the terms of the GNU General Public License as published by
>> + the Free Software Foundation; either version 3 of the License, or
>> + (at your option) any later version.
>> +
>> + This program is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + GNU General Public License for more details.
>> +
>> + You should have received a copy of the GNU General Public License
>> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
>> +
>> +#include "common/common-defs.h"
>> +#include "common/common-regcache.h"
>> +#include "arch/aarch32.h"
>> +
>> +struct target_desc *tdesc_aarch32;
>
> static
Done.
>
>> +
>> +/* See linux-aarch32-tdep.h. */
>
> /* See aarch32-tdep.h. */
Gah! Done.
>
>> diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
>> new file mode 100644
>> index 0000000000..f3cb8c7855
>> --- /dev/null
>> +++ b/gdb/arch/aarch32.c
>> @@ -0,0 +1,29 @@
>> +/* Copyright (C) 2019 Free Software Foundation, Inc.
>> +
>> + This file is part of GDB.
>> +
>> + This program is free software; you can redistribute it and/or modify
>> + it under the terms of the GNU General Public License as published by
>> + the Free Software Foundation; either version 3 of the License, or
>> + (at your option) any later version.
>> +
>> + This program is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + GNU General Public License for more details.
>> +
>> + You should have received a copy of the GNU General Public License
>> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
>> +
>> +#include "common/common-defs.h"
>> +#include "aarch32.h"
>> +
>> +extern struct target_desc *tdesc_arm_with_neon;
>> +
>> +/* See aarch32.h. */
>> +
>> +target_desc *
>> +aarch32_create_target_description ()
>> +{
>> + return tdesc_arm_with_neon;
>> +}
>
> Can you briefly explain the difference (from the point of view of GDB) between the
> ARM and AArch32 architecture? My current understanding is that AArch32 is Arm(v7?)
> emulation on AArch64, so they are very close. But there might be some subtle
> differences, requiring GDB to consider them as different architectures.
>
AArch32 is the 32bit mode in Arm v8. Compatible with Arm v7, but new features continue
to go into it via v8-M and v8-R. Emulation is probably not quite the correct word, as
it’s all down in hardware.
> Edit: I now went through the patch and saw the configure.in change. My guess would
> now be that it allows to include in an AArch64 build just what's actually supported
> by the AArch32 mode, which is arm-with-neon, without having to pull the entire Arm
> support. Does that sound right?
Yes. Yao split the AArch32 parts out from Arm in order to support multi-arch debugging
on AArch64. I’m not sure exactly what works on it.
>
>> diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
>> index 93738f0a0f..37806753cb 100644
>> --- a/gdb/arch/arm.c
>> +++ b/gdb/arch/arm.c
>> @@ -21,6 +21,15 @@
>> #include "common/common-regcache.h"
>> #include "arm.h"
>>
>> +extern struct target_desc *tdesc_arm_with_vfpv2;
>> +extern struct target_desc *tdesc_arm_with_vfpv3;
>> +extern struct target_desc *tdesc_arm_with_iwmmxt;
>> +#ifndef GDBSERVER
>> +extern struct target_desc *tdesc_arm_with_m;
>> +extern struct target_desc *tdesc_arm_with_m_vfp_d16;
>> +extern struct target_desc *tdesc_arm_with_m_fpa_layout;
>> +#endif
>> +
>> /* See arm.h. */
>>
>> int
>> @@ -372,3 +381,49 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
>>
>> return res & 0xffffffff;
>> }
>> +
>> +/* See arch/arm.h. */
>> +
>> +target_desc *
>> +arm_create_target_description (arm_fp_type fp_type)
>> +{
>> + switch (fp_type)
>> + {
>> + case ARM_FP_TYPE_NONE:
>> + return nullptr;
>> +
>> + case ARM_FP_TYPE_VFPV2:
>> + return tdesc_arm_with_vfpv2;
>> +
>> + case ARM_FP_TYPE_VFPV3:
>> + return tdesc_arm_with_vfpv3;
>> +
>> + case ARM_FP_TYPE_IWMMXT:
>> + return tdesc_arm_with_iwmmxt;
>> +
>> + default:
>> + error (_("Invalid Arm FP type: %d"), fp_type);
>> + }
>> +}
>> +
>> +/* See arch/arm.h. */
>> +
>> +target_desc *
>> +arm_create_mprofile_target_description (arm_m_profile_type m_type)
>> +{
>> + switch (m_type)
>> + {
>> +#ifndef GDBSERVER
>> + case ARM_M_TYPE_M_PROFILE:
>> + return tdesc_arm_with_m;
>> +
>> + case ARM_M_TYPE_VFP_D16:
>> + return tdesc_arm_with_m_fpa_layout;
>> +
>> + case ARM_M_TYPE_WITH_FPA:
>> + return tdesc_arm_with_m_vfp_d16;
>> +#endif
>> + default:
>> + error (_("Invalid Arm M type: %d"), m_type);
>> + }
>> +}
>
> If it doesn't make sense to have this function shared with GDBserver (given
> that GDBserver doesn't run on Cortex-Ms), it should probably go in gdb/arm-tdep.c.
Right. I wasn’t thinking of arch as “architecture code shared with gdbserver”,
but it makes sense.
The GDBSERVER defines do vanish later in the series.
I think I preferred having the two functions together in arch, but I’ve moved it.
>
>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> index 8e3607cdea..f3f6458a27 100644
>> --- a/gdb/arm-tdep.c
>> +++ b/gdb/arm-tdep.c
>> @@ -240,6 +240,9 @@ static const char **valid_disassembly_styles;
>> /* Disassembly style to use. Default to "std" register names. */
>> static const char *disassembly_style;
>>
>> +/* All possible arm target descriptors. */
>> +struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID];
>
> static
Done.
>
>> +
>> /* This is used to keep the bfd arch_info in sync with the disassembly
>> style. */
>> static void set_disassembly_style_sfunc (const char *, int,
>> @@ -8763,7 +8766,6 @@ arm_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>> return default_register_reggroup_p (gdbarch, regnum, group);
>> }
>>
>> -
>> /* For backward-compatibility we allow two 'g' packet lengths with
>> the remote protocol depending on whether FPA registers are
>> supplied. M-profile targets do not have FPA registers, but some
>> @@ -8777,21 +8779,29 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
>> {
>> if (gdbarch_tdep (gdbarch)->is_m)
>> {
>> + const target_desc *tdesc;
>> +
>> + /* This function is only called the once, therefore it's safe to call the
>> + tdesc creation function directly. */
>
> The other instance of "called the once".
>
> Would it be "unsafe" to call tdesc creation functions multiple times in general? I
> thought it was just a question of efficiency/caching. If so, I'd say "therefore
> it's not worth caching the descriptions”.
Yes, it’s just efficiency/caching.
>
> It might be true that they are called only once today (I guess because the only way to
> debug them in practice is through some server that only support debugging one at the
> time), but it could change eventually. For example, with multi-target, you could
> connect to two of them. Since it's not really difficult, I'd use the same caching pattern
> as for the other ones.
>
Agreed and done.
The above changes are going to require rebasing the other patches too. I’ll repost the
set as a V2.
Alan.
> On 10 Jul 2019, at 14:52, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>
>
>> On 10 Jul 2019, at 04:45, Simon Marchi <simark@simark.ca> wrote:
>>
>> On 2019-07-05 5:45 a.m., Alan Hayward wrote:
>>>
>>> +
>>> +/* See arch/arm.h. */
>>> +
>>> +target_desc *
>>> +arm_create_target_description (arm_fp_type fp_type)
>>> +{
>>> + switch (fp_type)
>>> + {
>>> + case ARM_FP_TYPE_NONE:
>>> + return nullptr;
>>> +
>>> + case ARM_FP_TYPE_VFPV2:
>>> + return tdesc_arm_with_vfpv2;
>>> +
>>> + case ARM_FP_TYPE_VFPV3:
>>> + return tdesc_arm_with_vfpv3;
>>> +
>>> + case ARM_FP_TYPE_IWMMXT:
>>> + return tdesc_arm_with_iwmmxt;
>>> +
>>> + default:
>>> + error (_("Invalid Arm FP type: %d"), fp_type);
>>> + }
>>> +}
>>> +
>>> +/* See arch/arm.h. */
>>> +
>>> +target_desc *
>>> +arm_create_mprofile_target_description (arm_m_profile_type m_type)
>>> +{
>>> + switch (m_type)
>>> + {
>>> +#ifndef GDBSERVER
>>> + case ARM_M_TYPE_M_PROFILE:
>>> + return tdesc_arm_with_m;
>>> +
>>> + case ARM_M_TYPE_VFP_D16:
>>> + return tdesc_arm_with_m_fpa_layout;
>>> +
>>> + case ARM_M_TYPE_WITH_FPA:
>>> + return tdesc_arm_with_m_vfp_d16;
>>> +#endif
>>> + default:
>>> + error (_("Invalid Arm M type: %d"), m_type);
>>> + }
>>> +}
>>
>> If it doesn't make sense to have this function shared with GDBserver (given
>> that GDBserver doesn't run on Cortex-Ms), it should probably go in gdb/arm-tdep.c.
>
> Right. I wasn’t thinking of arch as “architecture code shared with gdbserver”,
> but it makes sense.
>
> The GDBSERVER defines do vanish later in the series.
>
> I think I preferred having the two functions together in arch, but I’ve moved it.
>
To this, I’ll add that moving the mprofile function into arm-tdep.c means that
in a later patch, both arm-tdep.c and arch/arm.c have to
#include "features/arm/arm-vfpv2.c”
This is because ARM_M_TYPE_VFP_D16 uses the vfpv2 functions.
It works, but means that the same static function is being included twice.
Alan.
On 2019-07-10 10:26 a.m., Alan Hayward wrote:
>>> If it doesn't make sense to have this function shared with GDBserver (given
>>> that GDBserver doesn't run on Cortex-Ms), it should probably go in gdb/arm-tdep.c.
>>
>> Right. I wasn’t thinking of arch as “architecture code shared with gdbserver”,
>> but it makes sense.
>>
>> The GDBSERVER defines do vanish later in the series.
>>
>> I think I preferred having the two functions together in arch, but I’ve moved it.
>>
>
> To this, I’ll add that moving the mprofile function into arm-tdep.c means that
> in a later patch, both arm-tdep.c and arch/arm.c have to
> #include "features/arm/arm-vfpv2.c”
>
> This is because ARM_M_TYPE_VFP_D16 uses the vfpv2 functions.
>
> It works, but means that the same static function is being included twice.
>
>
> Alan.
Ok, well I agree that the end result looks ok, it's not a big deal if there is a function in
arch/arm.h that only gdb uses. It's true that it also makes sense to keep these functions
together. I was just a bit put off by the "#ifndef GDBSERVER" initially.
Simon
On 2019-07-10 9:52 a.m., Alan Hayward wrote:
> Should probably read “called the once for each target description type”. But,
> removed due to comments below.
But shouldn't it be "called once" instead of "called the once", or it's really a valid
locution? I have never heard that, and Googling "called the once" finds nothing of
interest.
Simon
> On 10 Jul 2019, at 17:07, Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-07-10 9:52 a.m., Alan Hayward wrote:
>> Should probably read “called the once for each target description type”. But,
>> removed due to comments below.
>
> But shouldn't it be "called once" instead of "called the once", or it's really a valid
> locution? I have never heard that, and Googling "called the once" finds nothing of
> interest.
>
> Simon
Yes, you’re correct. I didn’t spot what you meant the first time.
Alan.
@@ -665,7 +665,9 @@ ALL_64_TARGET_OBS = \
# All other target-dependent objects files (used with --enable-targets=all).
ALL_TARGET_OBS = \
+ aarch32-tdep.o \
arc-tdep.o \
+ arch/aarch32.o \
arch/arm.o \
arch/arm-get-next-pcs.o \
arch/arm-linux.o \
@@ -1184,6 +1186,7 @@ SFILES = \
# right, it is probably easiest just to list .h files here directly.
HFILES_NO_SRCDIR = \
+ aarch32-tdep.h \
aarch64-ravenscar-thread.h \
aarch64-tdep.h \
ada-lang.h \
@@ -1431,6 +1434,7 @@ HFILES_NO_SRCDIR = \
xtensa-tdep.h \
arch/aarch64.h \
arch/aarch64-insn.h \
+ arch/aarch32.h \
arch/arm.h \
arch/i386.h \
arch/ppc-linux-common.h \
@@ -2133,6 +2137,7 @@ force_update:
MAKEOVERRIDES =
ALLDEPFILES = \
+ aarch32-tdep.c \
aarch64-fbsd-nat.c \
aarch64-fbsd-tdep.c \
aarch64-linux-nat.c \
new file mode 100644
@@ -0,0 +1,33 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "common/common-defs.h"
+#include "common/common-regcache.h"
+#include "arch/aarch32.h"
+
+struct target_desc *tdesc_aarch32;
+
+/* See linux-aarch32-tdep.h. */
+
+const target_desc *
+aarch32_read_description ()
+{
+ if (tdesc_aarch32 == nullptr)
+ tdesc_aarch32 = aarch32_create_target_description ();
+
+ return tdesc_aarch32;
+}
new file mode 100644
@@ -0,0 +1,25 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef AARCH32_TDEP_H
+#define AARCH32_TDEP_H
+
+/* Get the AArch32 target description. */
+
+const target_desc *aarch32_read_description ();
+
+#endif /* aarch32-tdep.h. */
@@ -27,9 +27,11 @@
#include "target-descriptions.h"
#include "auxv.h"
#include "gdbcmd.h"
+#include "arch/arm.h"
#include "aarch64-tdep.h"
#include "aarch64-linux-tdep.h"
#include "aarch32-linux-nat.h"
+#include "aarch32-tdep.h"
#include "arch/arm.h"
#include "nat/aarch64-linux.h"
#include "nat/aarch64-linux-hw-point.h"
@@ -631,8 +633,6 @@ aarch64_linux_nat_target::post_attach (int pid)
linux_nat_target::post_attach (pid);
}
-extern struct target_desc *tdesc_arm_with_neon;
-
/* Implement the "read_description" target_ops method. */
const struct target_desc *
@@ -649,7 +649,7 @@ aarch64_linux_nat_target::read_description ()
ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_VFP, &iovec);
if (ret == 0)
- return tdesc_arm_with_neon;
+ return aarch32_read_description ();
CORE_ADDR hwcap = linux_get_hwcap (this);
new file mode 100644
@@ -0,0 +1,29 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "common/common-defs.h"
+#include "aarch32.h"
+
+extern struct target_desc *tdesc_arm_with_neon;
+
+/* See aarch32.h. */
+
+target_desc *
+aarch32_create_target_description ()
+{
+ return tdesc_arm_with_neon;
+}
new file mode 100644
@@ -0,0 +1,27 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef ARCH_AARCH32_H
+#define ARCH_AARCH32_H
+
+#include "common/tdesc.h"
+
+/* Create the AArch32 target description. */
+
+target_desc *aarch32_create_target_description ();
+
+#endif /* aarch32.h. */
@@ -21,6 +21,15 @@
#include "common/common-regcache.h"
#include "arm.h"
+extern struct target_desc *tdesc_arm_with_vfpv2;
+extern struct target_desc *tdesc_arm_with_vfpv3;
+extern struct target_desc *tdesc_arm_with_iwmmxt;
+#ifndef GDBSERVER
+extern struct target_desc *tdesc_arm_with_m;
+extern struct target_desc *tdesc_arm_with_m_vfp_d16;
+extern struct target_desc *tdesc_arm_with_m_fpa_layout;
+#endif
+
/* See arm.h. */
int
@@ -372,3 +381,49 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,
return res & 0xffffffff;
}
+
+/* See arch/arm.h. */
+
+target_desc *
+arm_create_target_description (arm_fp_type fp_type)
+{
+ switch (fp_type)
+ {
+ case ARM_FP_TYPE_NONE:
+ return nullptr;
+
+ case ARM_FP_TYPE_VFPV2:
+ return tdesc_arm_with_vfpv2;
+
+ case ARM_FP_TYPE_VFPV3:
+ return tdesc_arm_with_vfpv3;
+
+ case ARM_FP_TYPE_IWMMXT:
+ return tdesc_arm_with_iwmmxt;
+
+ default:
+ error (_("Invalid Arm FP type: %d"), fp_type);
+ }
+}
+
+/* See arch/arm.h. */
+
+target_desc *
+arm_create_mprofile_target_description (arm_m_profile_type m_type)
+{
+ switch (m_type)
+ {
+#ifndef GDBSERVER
+ case ARM_M_TYPE_M_PROFILE:
+ return tdesc_arm_with_m;
+
+ case ARM_M_TYPE_VFP_D16:
+ return tdesc_arm_with_m_fpa_layout;
+
+ case ARM_M_TYPE_WITH_FPA:
+ return tdesc_arm_with_m_vfp_d16;
+#endif
+ default:
+ error (_("Invalid Arm M type: %d"), m_type);
+ }
+}
@@ -19,6 +19,8 @@
#ifndef ARCH_ARM_H
#define ARCH_ARM_H
+#include "common/tdesc.h"
+
/* Register numbers of various important registers. */
enum gdb_regnum {
@@ -66,6 +68,23 @@ enum arm_breakpoint_kinds
ARM_BP_KIND_ARM = 4,
};
+/* Supported Arm FP hardware types. */
+enum arm_fp_type {
+ ARM_FP_TYPE_NONE = 0,
+ ARM_FP_TYPE_VFPV2,
+ ARM_FP_TYPE_VFPV3,
+ ARM_FP_TYPE_IWMMXT,
+ ARM_FP_TYPE_INVALID
+};
+
+/* Supported M-profile Arm types. */
+enum arm_m_profile_type {
+ ARM_M_TYPE_M_PROFILE,
+ ARM_M_TYPE_VFP_D16,
+ ARM_M_TYPE_WITH_FPA,
+ ARM_M_TYPE_INVALID
+};
+
/* Instruction condition field values. */
#define INST_EQ 0x0
#define INST_NE 0x1
@@ -165,4 +184,12 @@ unsigned long shifted_reg_val (struct regcache *regcache,
unsigned long pc_val,
unsigned long status_reg);
+/* Create an Arm target description with the given FP hardware type. */
+
+target_desc *arm_create_target_description (arm_fp_type fp_type);
+
+/* Create an Arm M-profile target description with the given hardware type. */
+
+target_desc *arm_create_mprofile_target_description (arm_m_profile_type m_type);
+
#endif /* ARCH_ARM_H */
@@ -20,6 +20,7 @@
#include "defs.h"
#include "elf/common.h"
+#include "aarch32-tdep.h"
#include "arm-tdep.h"
#include "arm-fbsd-tdep.h"
#include "auxv.h"
@@ -178,20 +179,20 @@ arm_fbsd_read_description_auxv (struct target_ops *target)
CORE_ADDR arm_hwcap = 0;
if (target_auxv_search (target, AT_FREEBSD_HWCAP, &arm_hwcap) != 1)
- return NULL;
+ return nullptr;
if (arm_hwcap & HWCAP_VFP)
{
if (arm_hwcap & HWCAP_NEON)
- return tdesc_arm_with_neon;
+ return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPD32))
== (HWCAP_VFPv3 | HWCAP_VFPD32))
- return tdesc_arm_with_vfpv3;
+ return arm_read_description (ARM_FP_TYPE_VFPV3);
else
- return tdesc_arm_with_vfpv2;
+ return arm_read_description (ARM_FP_TYPE_VFPV2);
}
- return NULL;
+ return nullptr;
}
/* Implement the "core_read_description" gdbarch method. */
@@ -27,6 +27,7 @@
#include "observable.h"
#include "gdbthread.h"
+#include "aarch32-tdep.h"
#include "arm-tdep.h"
#include "arm-linux-tdep.h"
#include "aarch32-linux-nat.h"
@@ -551,7 +552,7 @@ arm_linux_nat_target::read_description ()
}
if (arm_hwcap & HWCAP_IWMMXT)
- return tdesc_arm_with_iwmmxt;
+ return arm_read_description (ARM_FP_TYPE_IWMMXT);
if (arm_hwcap & HWCAP_VFP)
{
@@ -566,11 +567,11 @@ arm_linux_nat_target::read_description ()
/* NEON implies VFPv3-D32 or no-VFP unit. Say that we only support
Neon with VFPv3-D32. */
if (arm_hwcap & HWCAP_NEON)
- return tdesc_arm_with_neon;
+ return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
- return tdesc_arm_with_vfpv3;
- else
- return tdesc_arm_with_vfpv2;
+ return arm_read_description (ARM_FP_TYPE_VFPV3);
+
+ return arm_read_description (ARM_FP_TYPE_VFPV2);
}
return this->beneath ()->read_description ();
@@ -33,6 +33,7 @@
#include "auxv.h"
#include "xml-syscall.h"
+#include "aarch32-tdep.h"
#include "arch/arm.h"
#include "arch/arm-get-next-pcs.h"
#include "arch/arm-linux.h"
@@ -738,14 +739,14 @@ arm_linux_core_read_description (struct gdbarch *gdbarch,
/* NEON implies VFPv3-D32 or no-VFP unit. Say that we only support
Neon with VFPv3-D32. */
if (arm_hwcap & HWCAP_NEON)
- return tdesc_arm_with_neon;
+ return aarch32_read_description ();
else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
- return tdesc_arm_with_vfpv3;
- else
- return tdesc_arm_with_vfpv2;
+ return arm_read_description (ARM_FP_TYPE_VFPV3);
+
+ return arm_read_description (ARM_FP_TYPE_VFPV2);
}
- return NULL;
+ return nullptr;
}
@@ -240,6 +240,9 @@ static const char **valid_disassembly_styles;
/* Disassembly style to use. Default to "std" register names. */
static const char *disassembly_style;
+/* All possible arm target descriptors. */
+struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID];
+
/* This is used to keep the bfd arch_info in sync with the disassembly
style. */
static void set_disassembly_style_sfunc (const char *, int,
@@ -8763,7 +8766,6 @@ arm_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
return default_register_reggroup_p (gdbarch, regnum, group);
}
-
/* For backward-compatibility we allow two 'g' packet lengths with
the remote protocol depending on whether FPA registers are
supplied. M-profile targets do not have FPA registers, but some
@@ -8777,21 +8779,29 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
{
if (gdbarch_tdep (gdbarch)->is_m)
{
+ const target_desc *tdesc;
+
+ /* This function is only called the once, therefore it's safe to call the
+ tdesc creation function directly. */
+
/* If we know from the executable this is an M-profile target,
cater for remote targets whose register set layout is the
same as the FPA layout. */
+ tdesc = arm_create_mprofile_target_description (ARM_M_TYPE_WITH_FPA);
register_remote_g_packet_guess (gdbarch,
ARM_CORE_REGS_SIZE + ARM_FP_REGS_SIZE,
- tdesc_arm_with_m_fpa_layout);
+ tdesc);
/* The regular M-profile layout. */
+ tdesc = arm_create_mprofile_target_description (ARM_M_TYPE_M_PROFILE);
register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE,
- tdesc_arm_with_m);
+ tdesc);
/* M-profile plus M4F VFP. */
+ tdesc = arm_create_mprofile_target_description (ARM_M_TYPE_VFP_D16);
register_remote_g_packet_guess (gdbarch,
ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
- tdesc_arm_with_m_vfp_d16);
+ tdesc);
}
/* Otherwise we don't have a useful guess. */
@@ -13336,3 +13346,19 @@ arm_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
return ret;
}
+
+/* See arm-tdep.h. */
+
+const target_desc *
+arm_read_description (arm_fp_type fp_type)
+{
+ struct target_desc *tdesc = tdesc_arm_list[fp_type];
+
+ if (tdesc == nullptr)
+ {
+ tdesc = arm_create_target_description (fp_type);
+ tdesc_arm_list[fp_type] = tdesc;
+ }
+
+ return tdesc;
+}
@@ -281,11 +281,7 @@ extern void
void *cb_data,
const struct regcache *regcache);
-/* Target descriptions. */
-extern struct target_desc *tdesc_arm_with_m;
-extern struct target_desc *tdesc_arm_with_iwmmxt;
-extern struct target_desc *tdesc_arm_with_vfpv2;
-extern struct target_desc *tdesc_arm_with_vfpv3;
-extern struct target_desc *tdesc_arm_with_neon;
+/* Get the correct target description with given FP hardware type. */
+const target_desc *arm_read_description (arm_fp_type fp_type);
#endif /* arm-tdep.h */
@@ -48,8 +48,9 @@ amd64_tobjs="amd64-tdep.o arch/amd64.o ${x86_tobjs}"
case "${targ}" in
aarch64*-*-*)
- cpu_obs="aarch64-tdep.o arch/aarch64-insn.o arch/aarch64.o \
- ravenscar-thread.o aarch64-ravenscar-thread.o";;
+ cpu_obs="aarch32-tdep.o aarch64-tdep.o arch/aarch32.o \
+ arch/aarch64-insn.o arch/aarch64.o ravenscar-thread.o \
+ aarch64-ravenscar-thread.o";;
alpha*-*-*)
# Target: Alpha
@@ -62,7 +63,8 @@ arc*-*-*)
;;
arm*-*-*)
- cpu_obs="arch/arm.o arch/arm-get-next-pcs.o arm-tdep.o";;
+ cpu_obs="aarch32-tdep.o arch/aarch32.o arch/arm.o \
+ arch/arm-get-next-pcs.o arm-tdep.o";;
hppa*-*-*)
# Target: HP PA-RISC