[PATCH]: 54ea67bfc6 target/arm: use unmapped registers for REG_CLASS_ALL

[gerrit]

This is an automated email from Gerrit.

"Alexey Karyakin <aleksey.karya...@gmail.com>" just uploaded a new patch set to 
Gerrit, which you can find at https://review.openocd.org/c/openocd/+/8588

-- gerrit

commit 54ea67bfc6cde2444721d69aa0de608844fa0e81
Author: Alexey Karyakin <aleksey.karya...@gmail.com>
Date:   Sat Nov 16 19:06:07 2024 -0600

    target/arm: use unmapped registers for REG_CLASS_ALL
    
    The core register list in the target description for gdb is created
    using the list returned from arm_get_gdb_reg_list() with reg_class =
    REG_CLASS_ALL. If gdb attaches while the target is in a state when
    some registers are mapped (e.g. sp in the FIQ state), the core
    register list will be incorrect. The symptom is these gdb messages:
    
      warning: Architecture rejected target-supplied description
      Truncated register 16 in remote 'g' packet
    
    To fix this, use unmapped register list when arm_get_gdb_reg_list()
    is called with reg_class = REG_CLASS_ALL.
    
    Change-Id: I1c1059ae0434d54735db3d74651c48fec43e97ef
    Signed-off-by: Alexey Karyakin <aleksey.karya...@gmail.com>

diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c
index c1836bc7ae..2278b4cce1 100644
--- a/src/target/armv4_5.c
+++ b/src/target/armv4_5.c
@@ -1276,13 +1276,13 @@ int arm_get_gdb_reg_list(struct target *target,
        struct arm *arm = target_to_arm(target);
        unsigned int i;
 
-       if (!is_arm_mode(arm->core_mode)) {
-               LOG_ERROR("not a valid arm core mode - communication failure?");
-               return ERROR_FAIL;
-       }
-
        switch (reg_class) {
        case REG_CLASS_GENERAL:
+               if (!is_arm_mode(arm->core_mode)) {
+                       LOG_ERROR("not a valid arm core mode - communication 
failure?");
+                       return ERROR_FAIL;
+               }
+
                *reg_list_size = 26;
                *reg_list = malloc(sizeof(struct reg *) * (*reg_list_size));
 
@@ -1315,8 +1315,14 @@ int arm_get_gdb_reg_list(struct target *target,
 
                *reg_list = malloc(sizeof(struct reg *) * (*reg_list_size));
 
+               /* The existing pattern seems to be that REG_CLASS_ALL is only 
used with
+                * get_gdb_reg_list_noread() to retrieve register descriptions 
and the
+                * value is obtained later using the `get` function. Since the 
description
+                * list should be stable, no not perform register mapping 
(arm_reg_current())
+                * for REG_CLASS_ALL here.
+                */
                for (i = 0; i < 16; i++)
-                               (*reg_list)[i] = arm_reg_current(arm, i);
+                       (*reg_list)[i] = arm->core_cache->reg_list + i;
 
                for (i = 13; i < ARRAY_SIZE(arm_core_regs); i++) {
                                int reg_index = 
arm->core_cache->reg_list[i].number;

--