On Fri, Aug 15, 2025 at 11:31 PM Michael Forney <mfor...@mforney.org> wrote:
>
> Hi,
>
> I'm using OpenOCD with an ARM7TDMI core and noticed some issues
> when trying to use gdb to call various functions manually (e.g.
> with the gdb command `call foo()`).
>
> One issue is that gdb tries to change SP and LR, but those registers
> are banked and the ones that gdb tries to change are the the generic
> versions:
>
> /* These are only used for GDB target description, banked registers
> are accessed instead */
> [37] = { .name = "sp", .cookie = 13, .mode = ARM_MODE_ANY, .gdb_index
> = 13, },
> [38] = { .name = "lr", .cookie = 14, .mode = ARM_MODE_ANY, .gdb_index
> = 14, },
>
> When these registers are read, it reads r13 and r14 in the current
> processor mode. When they written, the value is saved in the struct
> reg and it is marked as dirty. However, in arm7_9_restore_context,
> it loops through the armv4_5_core_reg_map for each mode, but registers
> 37 and 38 don't appear anywhere in this map. So they are never
> actually updated.
>
> My first thought to fix this was to change arm_get_gdb_reg_list,
> which does map registers 0-16 to the current mode with arm_reg_current,
> but then replaces r13 and r14 with registers 37 and 38 shown above
> (due to their gdb_index). However, if I leave these with the mapped
> versions, it seems to break the XML target description.
>
> I did manage to find another approach to fix this by changing
> armv4_5_set_core_reg to resolve the register according to the
> processor mode before changing it:
>
> diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c
> index 597dc8990..74f2d6f0e 100644
> --- a/src/target/armv4_5.c
> +++ b/src/target/armv4_5.c
> @@ -618,6 +618,9 @@ static int armv4_5_set_core_reg(struct reg *reg, uint8_t
> *buf)
> return ERROR_TARGET_NOT_HALTED;
> }
>
> + if (reg->number < 16)
> + reg = arm_reg_current(armv4_5_target, reg->number);
> +
> /* Except for CPSR, the "reg" command exposes a writeback model
> * for the register cache.
> */
>
> This worked, but I think it might be better to map the register
> when the gdb register list is generated rather than redirecting it
> here.
>
> The other problem I encountered is that ARMv4/5 registers are marked
> as non-caller save:
>
> /* This really depends on the calling convention in use */
> reg_list[i].caller_save = false;
>
> This flag controls the save-restore attribute in the gdb target
> description, which is documented as follows:
>
> > save-restore
> >
> > Whether the register should be preserved across inferior function
> > calls; this must be either yes or no. The default is yes, which
> > is appropriate for most registers except for some system control
> > registers; this is not related to the target’s ABI.
>
> So I think the comment about calling convention is not correct.
> After gdb clobbers some registers for the function call, they are
> not restored. I looked at other targets and they all set caller_save
> = true for all registers (except RISCV vector registers).
>
> I think ARMv4/5 should have caller_save = true as well, which fixed
> the issue for me:
Hi,
I agree with your analysis. This looks like the right fix.
Here the caller-save attribute is related with GDB operations, not to
the ABI of the code under debug.
GDB needs to know which registers it has to save-restore across its operations.
> diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c
> index 597dc8990..74f2d6f0e 100644
> --- a/src/target/armv4_5.c
> +++ b/src/target/armv4_5.c
> @@ -707,7 +710,7 @@ struct reg_cache *arm_build_reg_cache(struct target
> *target, struct arm *arm)
> reg_list[i].exist = true;
>
> /* This really depends on the calling convention in use */
I think this comment should be reworked too
> - reg_list[i].caller_save = false;
> + reg_list[i].caller_save = true;
>
> /* Registers data type, as used by GDB target description */
> reg_list[i].reg_data_type = malloc(sizeof(struct
> reg_data_type));
>
> Do these changes seem like the correct way to solve these problems?
> If so, I can send a proper patch. If not, does anyone have any
> suggestions?
Please send a patch in gerrit.
Looking at GDB source files, just few registers are caller-save="no"
The caller-save argument is the 4th of tdesc_create_reg(). See
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/features/arm/arm-core.c
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/features/arm/arm-vfpv2.c
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/features/arm/arm-vfpv3.c
I think that also the second assignment of
reg_list[i].caller_save = false;
in the same function should be modified too as related to the floating
point registers.
Would you propose also a fix for the comment above `caller_save`
declaration in src/target/register.h ?
Thanks
Antonio
