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:

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 */
-               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?

Reply via email to