[PATCH]: 73fefbb25 arm_adi_v5: fix use of uninitialized values

[gerrit]

This is an automated email from Gerrit.

"Antonio Borneo <borneo.anto...@gmail.com>" just uploaded a new patch set to 
Gerrit, which you can find at https://review.openocd.org/c/openocd/+/6793

-- gerrit

commit 73fefbb2544f8be4ad1087d8c5b89e6d22bebdd4
Author: Antonio Borneo <borneo.anto...@gmail.com>
Date:   Fri Dec 24 11:52:25 2021 +0100

    arm_adi_v5: fix use of uninitialized values
    
    Commit 020e46d1868a ("arm_adi_v5: fix access to 64-bit MEM-AP")
    queues a read of register MEM_AP_REG_CFG but uses the register's
    value "before" is it read in the following dap_run().
    
    The register MEM_AP_REG_BASE64 is conditionally read based on the
    value of MEM_AP_REG_CFG. It can always be read, also in case of 32
    bit AP.
    Reorganize the code to guarantee that MEM_AP_REG_CFG's value is
    used only when not in a pending read.
    
    Error detected with valgrind after code instrumentation.
    
    Change-Id: I64dd22d0f65dac0ed63a903df157fcad7970d6d4
    Fixes: 020e46d1868a ("arm_adi_v5: fix access to 64-bit MEM-AP")
    Signed-off-by: Antonio Borneo <borneo.anto...@gmail.com>

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index fff97a7c2..64ec782df 100644
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -963,11 +963,6 @@ int dap_get_debugbase(struct adiv5_ap *ap,
        int retval;
        uint32_t baseptr_upper, baseptr_lower;
 
-       if (ap->cfg_reg == MEM_AP_REG_CFG_INVALID) {
-               retval = dap_queue_ap_read(ap, MEM_AP_REG_CFG, &ap->cfg_reg);
-               if (retval != ERROR_OK)
-                       return retval;
-       }
        retval = dap_queue_ap_read(ap, MEM_AP_REG_BASE, &baseptr_lower);
        if (retval != ERROR_OK)
                return retval;
@@ -980,6 +975,11 @@ int dap_get_debugbase(struct adiv5_ap *ap,
                if (retval != ERROR_OK)
                        return retval;
        }
+       if (ap->cfg_reg == MEM_AP_REG_CFG_INVALID) {
+               retval = dap_queue_ap_read(ap, MEM_AP_REG_CFG, &ap->cfg_reg);
+               if (retval != ERROR_OK)
+                       return retval;
+       }
 
        retval = dap_run(dap);
        if (retval != ERROR_OK)
@@ -1782,14 +1782,14 @@ COMMAND_HANDLER(dap_baseaddr_command)
        ap = dap_ap(dap, apsel);
        retval = dap_queue_ap_read(ap, MEM_AP_REG_BASE, &baseaddr_lower);
 
-       if (retval == ERROR_OK && ap->cfg_reg == MEM_AP_REG_CFG_INVALID)
-               retval = dap_queue_ap_read(ap, MEM_AP_REG_CFG, &ap->cfg_reg);
-
        if (retval == ERROR_OK && (ap->cfg_reg == MEM_AP_REG_CFG_INVALID || 
is_64bit_ap(ap))) {
                /* MEM_AP_REG_BASE64 is defined as 'RES0'; can be read and then 
ignored on 32 bits AP */
                retval = dap_queue_ap_read(ap, MEM_AP_REG_BASE64, 
&baseaddr_upper);
        }
 
+       if (retval == ERROR_OK && ap->cfg_reg == MEM_AP_REG_CFG_INVALID)
+               retval = dap_queue_ap_read(ap, MEM_AP_REG_CFG, &ap->cfg_reg);
+
        if (retval == ERROR_OK)
                retval = dap_run(dap);
        if (retval != ERROR_OK)

--