From 740c3ec238adec146afb93e58fe7c17e1044f4ef Mon Sep 17 00:00:00 2001 From: Jean-Christian de Rivaz Date: Mon, 17 Dec 2018 16:07:29 +0100 Subject: [PATCH] target start_algorithm: Don't copy the IN mem_params fix uninitialised value. Fix the write only out params TODO on armv7m.c Fix conditional move depends on uninitialised value. It was detected while programming a LPC8Nxx with a FTDI adapter. valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes [...] ==8696== Conditional jump or move depends on uninitialised value(s) ==8696== at 0x16E4D3: buf_set_u32 (binarybuffer.h:52) ==8696== by 0x16E4D3: ftdi_swd_queue_cmd (ftdi.c:1206) ==8696== by 0x18D76D: swd_queue_ap_write (adi_v5_swd.c:271) ==8696== by 0x18E33B: dap_queue_ap_write (arm_adi_v5.h:382) ==8696== by 0x18E33B: mem_ap_write (arm_adi_v5.c:420) ==8696== by 0x197CD9: target_write_buffer_default (target.c:2176) ==8696== by 0x2464B3: armv7m_start_algorithm (armv7m.c:383) ==8696== by 0x246AEB: armv7m_run_algorithm (armv7m.c:330) ==8696== by 0x19D846: target_run_algorithm (target.c:814) ==8696== by 0x1DF3A6: lpc2000_iap_call.isra.3 (lpc2000.c:818) ==8696== by 0x1E0CF6: lpc2000_erase (lpc2000.c:992) ==8696== by 0x185BDF: flash_driver_erase (core.c:44) ==8696== by 0x18650D: flash_iterate_address_range_inner (core.c:541) ==8696== by 0x18650D: flash_iterate_address_range (core.c:567) ==8696== by 0x18732F: flash_erase_address_range (core.c:584) ==8696== by 0x18732F: flash_write_unlock (core.c:928) ==8696== Uninitialised value was created by a heap allocation ==8696== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==8696== by 0x220EF9: init_mem_param (algorithm.c:30) ==8696== by 0x1DF247: lpc2000_iap_call.isra.3 (lpc2000.c:777) ==8696== by 0x1E0CF6: lpc2000_erase (lpc2000.c:992) ==8696== by 0x185BDF: flash_driver_erase (core.c:44) ==8696== by 0x18650D: flash_iterate_address_range_inner (core.c:541) ==8696== by 0x18650D: flash_iterate_address_range (core.c:567) ==8696== by 0x18732F: flash_erase_address_range (core.c:584) ==8696== by 0x18732F: flash_write_unlock (core.c:928) ==8696== by 0x18ACDF: handle_flash_write_image_command (tcl.c:457) ==8696== by 0x1B7D99: run_command (command.c:623) ==8696== by 0x1B7D99: script_command_run (command.c:208) ==8696== by 0x1B7FD9: command_unknown (command.c:1033) ==8696== by 0x2E2D37: JimInvokeCommand (jim.c:10364) ==8696== by 0x2E3865: Jim_EvalObj (jim.c:10814) ==8696== ==8696== Conditional jump or move depends on uninitialised value(s) ==8696== at 0x16E506: buf_set_u32 (binarybuffer.h:52) ==8696== by 0x16E506: ftdi_swd_queue_cmd (ftdi.c:1207) ==8696== by 0x18D76D: swd_queue_ap_write (adi_v5_swd.c:271) ==8696== by 0x18E33B: dap_queue_ap_write (arm_adi_v5.h:382) ==8696== by 0x18E33B: mem_ap_write (arm_adi_v5.c:420) ==8696== by 0x197CD9: target_write_buffer_default (target.c:2176) ==8696== by 0x2464B3: armv7m_start_algorithm (armv7m.c:383) ==8696== by 0x246AEB: armv7m_run_algorithm (armv7m.c:330) ==8696== by 0x19D846: target_run_algorithm (target.c:814) ==8696== by 0x1DF3A6: lpc2000_iap_call.isra.3 (lpc2000.c:818) ==8696== by 0x1E0CF6: lpc2000_erase (lpc2000.c:992) ==8696== by 0x185BDF: flash_driver_erase (core.c:44) ==8696== by 0x18650D: flash_iterate_address_range_inner (core.c:541) ==8696== by 0x18650D: flash_iterate_address_range (core.c:567) ==8696== by 0x18732F: flash_erase_address_range (core.c:584) ==8696== by 0x18732F: flash_write_unlock (core.c:928) ==8696== Uninitialised value was created by a heap allocation ==8696== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==8696== by 0x220EF9: init_mem_param (algorithm.c:30) ==8696== by 0x1DF247: lpc2000_iap_call.isra.3 (lpc2000.c:777) ==8696== by 0x1E0CF6: lpc2000_erase (lpc2000.c:992) ==8696== by 0x185BDF: flash_driver_erase (core.c:44) ==8696== by 0x18650D: flash_iterate_address_range_inner (core.c:541) ==8696== by 0x18650D: flash_iterate_address_range (core.c:567) ==8696== by 0x18732F: flash_erase_address_range (core.c:584) ==8696== by 0x18732F: flash_write_unlock (core.c:928) ==8696== by 0x18ACDF: handle_flash_write_image_command (tcl.c:457) ==8696== by 0x1B7D99: run_command (command.c:623) ==8696== by 0x1B7D99: script_command_run (command.c:208) ==8696== by 0x1B7FD9: command_unknown (command.c:1033) ==8696== by 0x2E2D37: JimInvokeCommand (jim.c:10364) ==8696== by 0x2E3865: Jim_EvalObj (jim.c:10814) Change-Id: I50f9a8c4516b686cf62ac3c76f47c53465e949da Signed-off-by: Jean-Christian de Rivaz Reviewed-on: http://openocd.zylin.com/4811 Tested-by: jenkins Reviewed-by: Tomas Vanek --- src/target/armv4_5.c | 2 ++ src/target/armv7m.c | 3 ++- src/target/dsp563xx.c | 2 ++ src/target/mips32.c | 2 ++ src/target/stm8.c | 2 ++ 5 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c index 6c30acc466..30aeb43bb3 100644 --- a/src/target/armv4_5.c +++ b/src/target/armv4_5.c @@ -1355,6 +1355,8 @@ int armv4_5_run_algorithm_inner(struct target *target, cpsr = buf_get_u32(arm->cpsr->value, 0, 32); for (i = 0; i < num_mem_params; i++) { + if (mem_params[i].direction == PARAM_IN) + continue; retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value); if (retval != ERROR_OK) diff --git a/src/target/armv7m.c b/src/target/armv7m.c index ecca0e7524..ef00b94275 100644 --- a/src/target/armv7m.c +++ b/src/target/armv7m.c @@ -379,7 +379,8 @@ int armv7m_start_algorithm(struct target *target, } for (int i = 0; i < num_mem_params; i++) { - /* TODO: Write only out params */ + if (mem_params[i].direction == PARAM_IN) + continue; retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value); diff --git a/src/target/dsp563xx.c b/src/target/dsp563xx.c index e7306d2e4e..899161694c 100644 --- a/src/target/dsp563xx.c +++ b/src/target/dsp563xx.c @@ -1387,6 +1387,8 @@ static int dsp563xx_run_algorithm(struct target *target, } for (i = 0; i < num_mem_params; i++) { + if (mem_params[i].direction == PARAM_IN) + continue; retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value); if (retval != ERROR_OK) diff --git a/src/target/mips32.c b/src/target/mips32.c index abb4255961..9ac2507a34 100644 --- a/src/target/mips32.c +++ b/src/target/mips32.c @@ -461,6 +461,8 @@ int mips32_run_algorithm(struct target *target, int num_mem_params, } for (int i = 0; i < num_mem_params; i++) { + if (mem_params[i].direction == PARAM_IN) + continue; retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value); if (retval != ERROR_OK) diff --git a/src/target/stm8.c b/src/target/stm8.c index f5df2481a3..b62ff131de 100644 --- a/src/target/stm8.c +++ b/src/target/stm8.c @@ -1890,6 +1890,8 @@ static int stm8_run_algorithm(struct target *target, int num_mem_params, } for (int i = 0; i < num_mem_params; i++) { + if (mem_params[i].direction == PARAM_IN) + continue; retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value); if (retval != ERROR_OK) -- 2.30.2