From 0989cd4d5d69850df38122f9ba1e7d5b4009fb51 Mon Sep 17 00:00:00 2001 From: Andreas Fritiofson Date: Wed, 22 Aug 2012 19:42:02 +0200 Subject: [PATCH] arm7_9: Fix broken halfword/byte memory reads Always scan out all bits, but make sure only the allowed number of bytes end up in the caller-provided buffer. Discard the rest by adding another scan field when size < 4. Rewrite the endianness callback to avoid reading outside allocated memory. Make it directly usable as a callback without the need for a wrapper. Move the shared callback to a more suitable home in arm7_9_common. This fixes the regressions introduced in commits 991ed5a2b657e660f744eefddb084724e52938ea cb90d32e386a7489d31136997209c61e9559ff5e and c3074f377c1da33ca8ba8493826e1b52351eebc6 Change-Id: Ia8bde8c5a9844e89a1d6c0bc8534cd26f02f8d11 Signed-off-by: Andreas Fritiofson Reviewed-on: http://openocd.zylin.com/789 Tested-by: jenkins Reviewed-by: Spencer Oliver Reviewed-by: Freddie Chopin --- src/target/arm.h | 2 -- src/target/arm7_9_common.c | 40 +++++++++++++++++++++++++ src/target/arm7_9_common.h | 4 +++ src/target/arm7tdmi.c | 60 ++++++++++++-------------------------- src/target/arm9tdmi.c | 44 +++++++++++++--------------- 5 files changed, 84 insertions(+), 66 deletions(-) diff --git a/src/target/arm.h b/src/target/arm.h index 30e2c76eab..916b321eb9 100644 --- a/src/target/arm.h +++ b/src/target/arm.h @@ -237,8 +237,6 @@ int arm_blank_check_memory(struct target *target, void arm_set_cpsr(struct arm *arm, uint32_t cpsr); struct reg *arm_reg_current(struct arm *arm, unsigned regnum); -void arm_endianness(uint8_t *tmp, void *in, int size, int be, int flip); - extern struct reg arm_gdb_dummy_fp_reg; extern struct reg arm_gdb_dummy_fps_reg; diff --git a/src/target/arm7_9_common.c b/src/target/arm7_9_common.c index 3461da4c48..faeed0d24d 100644 --- a/src/target/arm7_9_common.c +++ b/src/target/arm7_9_common.c @@ -2659,6 +2659,46 @@ int arm7_9_check_reset(struct target *target) return ERROR_OK; } +int arm7_9_endianness_callback(jtag_callback_data_t pu8_in, + jtag_callback_data_t i_size, jtag_callback_data_t i_be, + jtag_callback_data_t i_flip) +{ + uint8_t *in = (uint8_t *)pu8_in; + int size = (int)i_size; + int be = (int)i_be; + int flip = (int)i_flip; + uint32_t readback; + + switch (size) { + case 4: + readback = le_to_h_u32(in); + if (flip) + readback = flip_u32(readback, 32); + if (be) + h_u32_to_be(in, readback); + else + h_u32_to_le(in, readback); + break; + case 2: + readback = le_to_h_u16(in); + if (flip) + readback = flip_u32(readback, 16); + if (be) + h_u16_to_be(in, readback & 0xffff); + else + h_u16_to_le(in, readback & 0xffff); + break; + case 1: + readback = *in; + if (flip) + readback = flip_u32(readback, 8); + *in = readback & 0xff; + break; + } + + return ERROR_OK; +} + COMMAND_HANDLER(handle_arm7_9_dbgrq_command) { struct target *target = get_current_target(CMD_CTX); diff --git a/src/target/arm7_9_common.h b/src/target/arm7_9_common.h index 0e706a75a2..5b79a0a5a4 100644 --- a/src/target/arm7_9_common.h +++ b/src/target/arm7_9_common.h @@ -172,4 +172,8 @@ int arm7_9_init_arch_info(struct target *target, struct arm7_9_common *arm7_9); int arm7_9_examine(struct target *target); int arm7_9_check_reset(struct target *target); +int arm7_9_endianness_callback(jtag_callback_data_t pu8_in, + jtag_callback_data_t i_size, jtag_callback_data_t i_be, + jtag_callback_data_t i_flip); + #endif /* ARM7_9_COMMON_H */ diff --git a/src/target/arm7tdmi.c b/src/target/arm7tdmi.c index 2721502f39..634aa2997f 100644 --- a/src/target/arm7tdmi.c +++ b/src/target/arm7tdmi.c @@ -168,40 +168,6 @@ static int arm7tdmi_clock_data_in(struct arm_jtag *jtag_info, uint32_t *in) return ERROR_OK; } -void arm_endianness(uint8_t *tmp, void *in, int size, int be, int flip) -{ - uint32_t readback = le_to_h_u32(tmp); - if (flip) - readback = flip_u32(readback, 32); - switch (size) { - case 4: - if (be) - h_u32_to_be(((uint8_t *)in), readback); - else - h_u32_to_le(((uint8_t *)in), readback); - break; - case 2: - if (be) - h_u16_to_be(((uint8_t *)in), readback & 0xffff); - else - h_u16_to_le(((uint8_t *)in), readback & 0xffff); - break; - case 1: - *((uint8_t *)in) = readback & 0xff; - break; - } -} - -static int arm7endianness(jtag_callback_data_t arg, - jtag_callback_data_t size, jtag_callback_data_t be, - jtag_callback_data_t captured) -{ - uint8_t *in = (uint8_t *)arg; - - arm_endianness((uint8_t *)captured, in, (int)size, (int)be, 1); - return ERROR_OK; -} - /* clock the target, and read the databus * the *in pointer points to a buffer where elements of 'size' bytes * are stored in big (be == 1) or little (be == 0) endianness @@ -210,7 +176,7 @@ static int arm7tdmi_clock_data_in_endianness(struct arm_jtag *jtag_info, void *in, int size, int be) { int retval = ERROR_OK; - struct scan_field fields[2]; + struct scan_field fields[3]; retval = arm_jtag_scann(jtag_info, 0x1, TAP_DRPAUSE); if (retval != ERROR_OK) @@ -223,17 +189,29 @@ static int arm7tdmi_clock_data_in_endianness(struct arm_jtag *jtag_info, fields[0].out_value = NULL; fields[0].in_value = NULL; - fields[1].num_bits = size * 8; - fields[1].out_value = NULL; - fields[1].in_value = in; + if (size == 4) { + fields[1].num_bits = 32; + fields[1].out_value = NULL; + fields[1].in_value = in; + } else { + /* Discard irrelevant bits of the scan, making sure we don't write more + * than size bytes to in */ + fields[1].num_bits = 32 - size * 8; + fields[1].out_value = NULL; + fields[1].in_value = NULL; - jtag_add_dr_scan(jtag_info->tap, 2, fields, TAP_DRPAUSE); + fields[2].num_bits = size * 8; + fields[2].out_value = NULL; + fields[2].in_value = in; + } + + jtag_add_dr_scan(jtag_info->tap, size == 4 ? 2 : 3, fields, TAP_DRPAUSE); - jtag_add_callback4(arm7endianness, + jtag_add_callback4(arm7_9_endianness_callback, (jtag_callback_data_t)in, (jtag_callback_data_t)size, (jtag_callback_data_t)be, - (jtag_callback_data_t)in); + (jtag_callback_data_t)1); jtag_add_runtest(0, TAP_DRPAUSE); diff --git a/src/target/arm9tdmi.c b/src/target/arm9tdmi.c index e8ad93289c..1cb9fbdb2b 100644 --- a/src/target/arm9tdmi.c +++ b/src/target/arm9tdmi.c @@ -244,16 +244,6 @@ int arm9tdmi_clock_data_in(struct arm_jtag *jtag_info, uint32_t *in) return ERROR_OK; } -static int arm9endianness(jtag_callback_data_t arg, - jtag_callback_data_t size, jtag_callback_data_t be, - jtag_callback_data_t captured) -{ - uint8_t *in = (uint8_t *)arg; - - arm_endianness((uint8_t *)captured, in, (int)size, (int)be, 0); - return ERROR_OK; -} - /* clock the target, and read the databus * the *in pointer points to a buffer where elements of 'size' bytes * are stored in big (be == 1) or little (be == 0) endianness @@ -262,7 +252,7 @@ int arm9tdmi_clock_data_in_endianness(struct arm_jtag *jtag_info, void *in, int size, int be) { int retval = ERROR_OK; - struct scan_field fields[3]; + struct scan_field fields[2]; retval = arm_jtag_scann(jtag_info, 0x1, TAP_DRPAUSE); if (retval != ERROR_OK) @@ -272,25 +262,33 @@ int arm9tdmi_clock_data_in_endianness(struct arm_jtag *jtag_info, if (retval != ERROR_OK) return retval; - fields[0].num_bits = size * 8; - fields[0].out_value = NULL; - fields[0].in_value = in; + if (size == 4) { + fields[0].num_bits = 32; + fields[0].out_value = NULL; + fields[0].in_value = in; - fields[1].num_bits = 3; - fields[1].out_value = NULL; - fields[1].in_value = NULL; + fields[1].num_bits = 3 + 32; + fields[1].out_value = NULL; + fields[1].in_value = NULL; + } else { + /* Discard irrelevant bits of the scan, making sure we don't write more + * than size bytes to in */ + fields[0].num_bits = size * 8; + fields[0].out_value = NULL; + fields[0].in_value = in; - fields[2].num_bits = 32; - fields[2].out_value = NULL; - fields[2].in_value = NULL; + fields[1].num_bits = 3 + 32 + 32 - size * 8; + fields[1].out_value = NULL; + fields[1].in_value = NULL; + } - jtag_add_dr_scan(jtag_info->tap, 3, fields, TAP_DRPAUSE); + jtag_add_dr_scan(jtag_info->tap, 2, fields, TAP_DRPAUSE); - jtag_add_callback4(arm9endianness, + jtag_add_callback4(arm7_9_endianness_callback, (jtag_callback_data_t)in, (jtag_callback_data_t)size, (jtag_callback_data_t)be, - (jtag_callback_data_t)in); + (jtag_callback_data_t)0); jtag_add_runtest(0, TAP_DRPAUSE); -- 2.30.2