target/aarch64: fix soft breakpoint when PE is in AArch32 state
authorTarek BOCHKATI <tarek.bouchkati@gmail.com>
Wed, 4 Dec 2019 14:09:51 +0000 (15:09 +0100)
committerAntonio Borneo <borneo.antonio@gmail.com>
Thu, 12 Mar 2020 09:46:43 +0000 (09:46 +0000)
Before this patch aarch64_set_breakpoint was using either A64, or A32
HLT opcode by relying on armv8_opcode helper.
This behaviors ignores the fact that in AArch32 state the core could
execute Thumb-2 instructions, and gdb could request to insert a soft
bkpt in a Thumb-2 code chunk.

In this change, we check the core_state and bkpt length to know the
correct opcode to use.

Note: based on https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html
      if bkpt length/kind == 3, we should replace a 32-bit Thumb-2 opcode,
      then we use twice the 16 bits Thumb-2 bkpt opcode and we fix-up the
      length to 4 bytes, in order to set correctly the bpkt.

Change-Id: I8f3551124412c61d155eae87761767e9937f917d
Signed-off-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com>
Reviewed-on: http://openocd.zylin.com/5355
Tested-by: jenkins
Reviewed-by: Muhammad Omair Javaid <omair.javaid@linaro.org>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
src/target/aarch64.c
src/target/armv8_opcodes.h

index 4d3d9819e856b6ed498c80c942f33f7617186048..3918b1575c844ddc5ba9f2ca0d329234eccf61bc 100644 (file)
@@ -1263,9 +1263,32 @@ static int aarch64_set_breakpoint(struct target *target,
                        brp_list[brp_i].value);
 
        } else if (breakpoint->type == BKPT_SOFT) {
+               uint32_t opcode;
                uint8_t code[4];
 
-               buf_set_u32(code, 0, 32, armv8_opcode(armv8, ARMV8_OPC_HLT));
+               if (armv8_dpm_get_core_state(&armv8->dpm) == ARM_STATE_AARCH64) {
+                       opcode = ARMV8_HLT(11);
+
+                       if (breakpoint->length != 4)
+                               LOG_ERROR("bug: breakpoint length should be 4 in AArch64 mode");
+               } else {
+                       /**
+                        * core_state is ARM_STATE_ARM
+                        * in that case the opcode depends on breakpoint length:
+                        *  - if length == 4 => A32 opcode
+                        *  - if length == 2 => T32 opcode
+                        *  - if length == 3 => T32 opcode (refer to gdb doc : ARM-Breakpoint-Kinds)
+                        *    in that case the length should be changed from 3 to 4 bytes
+                        **/
+                       opcode = (breakpoint->length == 4) ? ARMV8_HLT_A1(11) :
+                                       (uint32_t) (ARMV8_HLT_T1(11) | ARMV8_HLT_T1(11) << 16);
+
+                       if (breakpoint->length == 3)
+                               breakpoint->length = 4;
+               }
+
+               buf_set_u32(code, 0, 32, opcode);
+
                retval = target_read_memory(target,
                                breakpoint->address & 0xFFFFFFFFFFFFFFFE,
                                breakpoint->length, 1,
index 3fda29668a27dac00376d34d08198db0249f4dfa..217cc64c6ce21e1512dd401b60d3f27e50009a18 100644 (file)
 #define ARMV8_BKPT(Im) (0xD4200000 | ((Im & 0xffff) << 5))
 #define ARMV8_HLT(Im) (0x0D4400000 | ((Im & 0xffff) << 5))
 #define ARMV8_HLT_A1(Im) (0xE1000070 | ((Im & 0xFFF0) << 4) | (Im & 0xF))
+#define ARMV8_HLT_T1(Im) (0xba80 | (Im & 0x3f))
 
 #define ARMV8_MOVFSP_64(Rt) ((1 << 31) | 0x11000000 | (0x1f << 5) | (Rt))
 #define ARMV8_MOVTSP_64(Rt) ((1 << 31) | 0x11000000 | (Rt << 5) | (0x1F))