cortex-a: fix single-step on infinite loop

On ARMv7a/r the single-step is implemented through a HW breakpoint
that hits instructions at any address except the address of the
current instruction.

The method above fails in case of an infinite loop coded by a
single instruction that jumps on itself; in such case, the same
instruction (at the same address) is executed over and over and
the breakpoint never hits. In current code this case is wrongly
considered as an error.

Reduce the timeout while waiting for the HW breakpoint being hit,
then halt.

The jump on itself would be executed several times before the
timeout and the halt, but this is not an issue. There are few
"pathological" instructions in ARMv7a/r that jumps on itself and
that can have side effects if executed more than once. They are
listed in the code. We do not consider these as real use cases
generated by a compiler.

Document the method in the code.

Report that the single-step function is not properly managing the
HW breakpoints if it exits on error. To be fixed in the future.

Change-Id: I9641a4a3e2f68b83897ccf3a12d3c34e98a7805c
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8871
Tested-by: jenkins

diff --git a/src/target/cortex_a.c b/src/target/cortex_a.c
index ee27e1b21722c3cae01d413e4eef0e72eacc2a71..46362017af745e23803ee49ea83bfb4a937104e1 100644 (file)
--- a/src/target/cortex_a.c
+++ b/src/target/cortex_a.c
@@ -1169,6 +1169,29 @@ static int cortex_a_set_dscr_bits(struct target *target,
        return retval;
 }
 
+/*
+ * Single-step on ARMv7a/r is implemented through a HW breakpoint that hits
+ * every instruction at any address except the address of the current
+ * instruction.
+ * Such HW breakpoint is never hit in case of a single instruction that jumps
+ * on itself (infinite loop), or a WFI or a WFE. In this case, halt the CPU
+ * after a timeout.
+ * The jump on itself would be executed several times before the timeout forces
+ * the halt, but this is not an issue. In ARMv7a/r there are few "pathological"
+ * instructions, listed below, that jumps on itself and that can have side
+ * effects if executed more than once; but they are not considered as real use
+ * cases generated by a compiler.
+ * Some example:
+ * - 'pop {pc}' or multi register 'pop' including PC, when the new PC value is
+ *   the same value of current PC. The single step will not stop at the first
+ *   'pop' and will continue taking values from the stack, modifying SP at each
+ *   iteration.
+ * - 'rfeda', 'rfedb', 'rfeia', 'rfeib', when the new PC value is the same
+ *   value of current PC. The register provided to the instruction (usually SP)
+ *   will be incremented or decremented at each iteration.
+ *
+ * TODO: fix exit in case of error, cleaning HW breakpoints.
+ */
 static int cortex_a_step(struct target *target, bool current, target_addr_t address,
        bool handle_breakpoints)
 {
@@ -1227,15 +1250,34 @@ static int cortex_a_step(struct target *target, bool current, target_addr_t addr
        if (retval != ERROR_OK)
                return retval;
 
-       int64_t then = timeval_ms();
+       // poll at least once before starting the timeout
+       retval = cortex_a_poll(target);
+       if (retval != ERROR_OK)
+               return retval;
+
+       int64_t then = timeval_ms() + 100;
        while (target->state != TARGET_HALTED) {
+               if (timeval_ms() > then)
+                       break;
+
                retval = cortex_a_poll(target);
                if (retval != ERROR_OK)
                        return retval;
-               if (target->state == TARGET_HALTED)
-                       break;
-               if (timeval_ms() > then + 1000) {
-                       LOG_ERROR("timeout waiting for target halt");
+       }
+
+       if (target->state != TARGET_HALTED) {
+               LOG_TARGET_DEBUG(target, "timeout waiting for target halt, try halt");
+
+               retval = cortex_a_halt(target);
+               if (retval != ERROR_OK)
+                       return retval;
+
+               retval = cortex_a_poll(target);
+               if (retval != ERROR_OK)
+                       return retval;
+
+               if (target->state != TARGET_HALTED) {
+                       LOG_TARGET_ERROR(target, "timeout waiting for target halt");
                        return ERROR_FAIL;
                }
        }
@@ -1255,9 +1297,6 @@ static int cortex_a_step(struct target *target, bool current, target_addr_t addr
        if (breakpoint)
                cortex_a_set_breakpoint(target, breakpoint, 0);
 
-       if (target->state != TARGET_HALTED)
-               LOG_DEBUG("target stepped");
-
        return ERROR_OK;
 }