Menu
▾
▴
[OpenOCD-commit] Main OpenOCD repository branch, master, updated. v0.12.0-650-g53b94fad5
|
From: openocd-gerrit <ope...@us...> - 2024-07-13 22:23:47
|
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Main OpenOCD repository".
The branch, master has been updated
via 53b94fad58ab32b02531f13299968c41f49947fa (commit)
from c97a8ff10d250ad98597054322fd727dc292a332 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 53b94fad58ab32b02531f13299968c41f49947fa
Author: Jan Matyas <jan...@co...>
Date: Mon Jun 3 10:23:02 2024 +0200
binarybuffer: Fix str_to_buf() parsing function
The function str_to_buf() was too benevolent and did
not perform sufficient error checking on the input
string being parsed. Especially:
- Invalid numbers were silently ignored.
- Out-of-range numbers were silently truncated.
The following commands that use str_to_buf()
were affected:
- reg (when writing a register value)
- set_reg
- jtag drscan
This pull request fixes that by:
- Rewriting str_to_buf() to add the missing checks.
- Adding function command_parse_str_to_buf() which can
be used in command handlers. It parses the input
numbers and provides user-readable error messages
in case of parsing errors.
Examples:
jtag drscan 10 huh10
- Old behavior: The string "huh10" is silently
converted to 10 and the command is then executed.
No warning error or warning is shown to the user.
- New behavior: Error message is shown:
"'huh10' is not a valid number"
reg pc 0x123456789
Assuming the "pc" is 32 bits wide:
- Old behavior: The register value is silently
truncated to 0x23456789 and the command is performed.
- New behavior: Error message is shown to the user:
"Number 0x123456789 exceeds 32 bits"
Change-Id: I079e19cd153aec853a3c2eb66953024b8542d0f4
Signed-off-by: Jan Matyas <jan...@co...>
Reviewed-on: https://review.openocd.org/c/openocd/+/8315
Tested-by: jenkins
Reviewed-by: Marek Vrbka <mar...@co...>
Reviewed-by: Antonio Borneo <bor...@gm...>
diff --git a/src/helper/binarybuffer.c b/src/helper/binarybuffer.c
index 5f38b43ae..3e09143c6 100644
--- a/src/helper/binarybuffer.c
+++ b/src/helper/binarybuffer.c
@@ -102,7 +102,6 @@ bool buf_cmp_mask(const void *_buf1, const void *_buf2,
return buf_cmp_trailing(buf1[last], buf2[last], mask[last], trailing);
}
-
void *buf_set_ones(void *_buf, unsigned size)
{
uint8_t *buf = _buf;
@@ -206,36 +205,75 @@ char *buf_to_hex_str(const void *_buf, unsigned buf_len)
return str;
}
-/** identify radix, and skip radix-prefix (0, 0x or 0X) */
-static void str_radix_guess(const char **_str, unsigned *_str_len,
- unsigned *_radix)
+static bool str_has_hex_prefix(const char *s)
+{
+ /* Starts with "0x" or "0X" */
+ return (s[0] == '0') && (s[1] == 'x' || s[1] == 'X');
+}
+
+static bool str_has_octal_prefix(const char *s)
+{
+ /* - starts with '0',
+ * - has at least two characters, and
+ * - the second character is not 'x' or 'X' */
+ return (s[0] == '0') && (s[1] != '\0') && (s[1] != 'x') && (s[1] != 'X');
+}
+
+/**
+ * Try to identify the radix of the number by looking at its prefix.
+ * No further validation of the number is preformed.
+ */
+static unsigned int str_radix_guess(const char *str)
+{
+ assert(str);
+
+ if (str_has_hex_prefix(str))
+ return 16;
+
+ if (str_has_octal_prefix(str))
+ return 8;
+
+ /* Otherwise assume a decadic number. */
+ return 10;
+}
+
+/** Strip leading "0x" or "0X" from hex numbers or "0" from octal numbers. */
+static void str_strip_number_prefix_if_present(const char **_str, unsigned int radix)
{
- unsigned radix = *_radix;
- if (radix != 0)
- return;
+ assert(radix == 16 || radix == 10 || radix == 8);
+ assert(_str);
+
const char *str = *_str;
- unsigned str_len = *_str_len;
- if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X')) {
- radix = 16;
+ assert(str);
+
+ if (radix == 16 && str_has_hex_prefix(str))
str += 2;
- str_len -= 2;
- } else if ((str[0] == '0') && (str_len != 1)) {
- radix = 8;
+ else if (radix == 8 && str_has_octal_prefix(str))
str += 1;
- str_len -= 1;
- } else
- radix = 10;
+
+ /* No prefix to strip for radix == 10. */
+
*_str = str;
- *_str_len = str_len;
- *_radix = radix;
}
-int str_to_buf(const char *str, unsigned str_len,
- void *_buf, unsigned buf_len, unsigned radix)
+int str_to_buf(const char *str, void *_buf, unsigned int buf_len,
+ unsigned int radix, unsigned int *_detected_radix)
{
- str_radix_guess(&str, &str_len, &radix);
+ assert(radix == 0 || radix == 8 || radix == 10 || radix == 16);
+
+ if (radix == 0)
+ radix = str_radix_guess(str);
- float factor;
+ if (_detected_radix)
+ *_detected_radix = radix;
+
+ str_strip_number_prefix_if_present(&str, radix);
+
+ const size_t str_len = strlen(str);
+ if (str_len == 0)
+ return ERROR_INVALID_NUMBER;
+
+ float factor = 0.0;
if (radix == 16)
factor = 0.5; /* log(16) / log(256) = 0.5 */
else if (radix == 10)
@@ -243,41 +281,69 @@ int str_to_buf(const char *str, unsigned str_len,
else if (radix == 8)
factor = 0.375; /* log(8) / log(256) = 0.375 */
else
- return 0;
+ assert(false);
- /* copy to zero-terminated buffer */
- char *charbuf = strndup(str, str_len);
+ const unsigned int b256_len = ceil_f_to_u32(str_len * factor);
- /* number of digits in base-256 notation */
- unsigned b256_len = ceil_f_to_u32(str_len * factor);
+ /* Allocate a buffer for digits in base-256 notation */
uint8_t *b256_buf = calloc(b256_len, 1);
+ if (!b256_buf) {
+ LOG_ERROR("Unable to allocate memory");
+ return ERROR_FAIL;
+ }
- /* go through zero terminated buffer
- * input digits (ASCII) */
- unsigned i;
- for (i = 0; charbuf[i]; i++) {
- uint32_t tmp = charbuf[i];
- if ((tmp >= '0') && (tmp <= '9'))
+ /* Go through the zero-terminated buffer
+ * of input digits (ASCII) */
+ for (unsigned int i = 0; str[i]; i++) {
+ uint32_t tmp = str[i];
+ if ((tmp >= '0') && (tmp <= '9')) {
tmp = (tmp - '0');
- else if ((tmp >= 'a') && (tmp <= 'f'))
+ } else if ((tmp >= 'a') && (tmp <= 'f')) {
tmp = (tmp - 'a' + 10);
- else if ((tmp >= 'A') && (tmp <= 'F'))
+ } else if ((tmp >= 'A') && (tmp <= 'F')) {
tmp = (tmp - 'A' + 10);
- else
- continue; /* skip characters other than [0-9,a-f,A-F] */
+ } else {
+ /* Characters other than [0-9,a-f,A-F] are invalid */
+ free(b256_buf);
+ return ERROR_INVALID_NUMBER;
+ }
- if (tmp >= radix)
- continue; /* skip digits invalid for the current radix */
+ if (tmp >= radix) {
+ /* Encountered a digit that is invalid for the current radix */
+ free(b256_buf);
+ return ERROR_INVALID_NUMBER;
+ }
- /* base-256 digits */
- for (unsigned j = 0; j < b256_len; j++) {
+ /* Add the current digit (tmp) to the intermediate result
+ * in b256_buf (base-256 digits) */
+ for (unsigned int j = 0; j < b256_len; j++) {
tmp += (uint32_t)b256_buf[j] * radix;
- b256_buf[j] = (uint8_t)(tmp & 0xFF);
+ b256_buf[j] = (uint8_t)(tmp & 0xFFu);
tmp >>= 8;
}
+ /* The b256_t buffer is large enough to contain the whole result. */
+ assert(tmp == 0);
}
+ /* The result must not contain more bits than buf_len. */
+ /* Check the whole bytes: */
+ for (unsigned int j = DIV_ROUND_UP(buf_len, 8); j < b256_len; j++) {
+ if (b256_buf[j] != 0x0) {
+ free(b256_buf);
+ return ERROR_NUMBER_EXCEEDS_BUFFER;
+ }
+ }
+ /* Check the partial byte: */
+ if (buf_len % 8) {
+ const uint8_t mask = 0xFFu << (buf_len % 8);
+ if ((b256_buf[(buf_len / 8)] & mask) != 0x0) {
+ free(b256_buf);
+ return ERROR_NUMBER_EXCEEDS_BUFFER;
+ }
+ }
+
+ /* Copy the digits to the output buffer */
uint8_t *buf = _buf;
for (unsigned j = 0; j < DIV_ROUND_UP(buf_len, 8); j++) {
if (j < b256_len)
@@ -286,14 +352,8 @@ int str_to_buf(const char *str, unsigned str_len,
buf[j] = 0;
}
- /* mask out bits that don't belong to the buffer */
- if (buf_len % 8)
- buf[(buf_len / 8)] &= 0xff >> (8 - (buf_len % 8));
-
free(b256_buf);
- free(charbuf);
-
- return i;
+ return ERROR_OK;
}
void bit_copy_queue_init(struct bit_copy_queue *q)
diff --git a/src/helper/binarybuffer.h b/src/helper/binarybuffer.h
index 344629681..441374330 100644
--- a/src/helper/binarybuffer.h
+++ b/src/helper/binarybuffer.h
@@ -14,6 +14,9 @@
#include <helper/list.h>
#include <helper/types.h>
+#define ERROR_INVALID_NUMBER (-1700)
+#define ERROR_NUMBER_EXCEEDS_BUFFER (-1701)
+
/** @file
* Support functions to access arbitrary bits in a byte array
*/
@@ -189,8 +192,18 @@ void *buf_set_ones(void *buf, unsigned size);
void *buf_set_buf(const void *src, unsigned src_start,
void *dst, unsigned dst_start, unsigned len);
-int str_to_buf(const char *str, unsigned len,
- void *bin_buf, unsigned buf_size, unsigned radix);
+/**
+ * Parse an unsigned number (provided as a zero-terminated string)
+ * into a bit buffer whose size is buf_len bits.
+ * @param str Input number, zero-terminated string
+ * @param _buf Output buffer, allocated by the caller
+ * @param buf_len Output buffer size in bits
+ * @param radix Base of the input number - 16, 10, 8 or 0.
+ * 0 means auto-detect the radix.
+ */
+int str_to_buf(const char *str, void *_buf, unsigned int buf_len,
+ unsigned int radix, unsigned int *_detected_radix);
+
char *buf_to_hex_str(const void *buf, unsigned size);
/* read a uint32_t from a buffer in target memory endianness */
diff --git a/src/helper/command.c b/src/helper/command.c
index a775c730b..15a9b4a08 100644
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -1360,6 +1360,46 @@ int command_parse_bool_arg(const char *in, bool *out)
return ERROR_COMMAND_SYNTAX_ERROR;
}
+static const char *radix_to_str(unsigned int radix)
+{
+ switch (radix) {
+ case 16: return "hexadecimal";
+ case 10: return "decadic";
+ case 8: return "octal";
+ }
+ assert(false);
+ return "";
+}
+
+COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len,
+ unsigned int radix)
+{
+ assert(str);
+ assert(buf);
+
+ int ret = str_to_buf(str, buf, buf_len, radix, NULL);
+ if (ret == ERROR_OK)
+ return ret;
+
+ /* Provide a clear error message to the user */
+ if (ret == ERROR_INVALID_NUMBER) {
+ if (radix == 0) {
+ /* Any radix is accepted, so don't include it in the error message. */
+ command_print(CMD, "'%s' is not a valid number", str);
+ } else {
+ /* Specific radix is required - tell the user what it is. */
+ command_print(CMD, "'%s' is not a valid number (requiring %s number)",
+ str, radix_to_str(radix));
+ }
+ } else if (ret == ERROR_NUMBER_EXCEEDS_BUFFER) {
+ command_print(CMD, "Number %s exceeds %u bits", str, buf_len);
+ } else {
+ command_print(CMD, "Could not parse number '%s'", str);
+ }
+
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+}
+
COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label)
{
switch (CMD_ARGC) {
diff --git a/src/helper/command.h b/src/helper/command.h
index fc26dda81..7a044e619 100644
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -517,6 +517,17 @@ DECLARE_PARSE_WRAPPER(_target_addr, target_addr_t);
int command_parse_bool_arg(const char *in, bool *out);
COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label);
+/**
+ * Parse a number (base 10, base 16 or base 8) and store the result
+ * into a bit buffer.
+ *
+ * In case of parsing error, a user-readable error message is produced.
+ *
+ * If radix = 0 is given, the function guesses the radix by looking at the number prefix.
+ */
+COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len,
+ unsigned int radix);
+
/** parses an on/off command argument */
#define COMMAND_PARSE_ON_OFF(in, out) \
COMMAND_PARSE_BOOL(in, out, "on", "off")
diff --git a/src/jtag/tcl.c b/src/jtag/tcl.c
index 1a4c4b774..e53413427 100644
--- a/src/jtag/tcl.c
+++ b/src/jtag/tcl.c
@@ -87,8 +87,11 @@ static COMMAND_HELPER(handle_jtag_command_drscan_fields, struct scan_field *fiel
LOG_ERROR("Out of memory");
return ERROR_FAIL;
}
+
fields[field_count].out_value = t;
- str_to_buf(CMD_ARGV[i + 1], strlen(CMD_ARGV[i + 1]), t, bits, 0);
+ int ret = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[i + 1], t, bits, 0);
+ if (ret != ERROR_OK)
+ return ret;
fields[field_count].in_value = t;
field_count++;
}
diff --git a/src/target/target.c b/src/target/target.c
index 1f4817d5c..8ff665f47 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -3128,11 +3128,18 @@ COMMAND_HANDLER(handle_reg_command)
/* set register value */
if (CMD_ARGC == 2) {
uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8));
- if (!buf)
+ if (!buf) {
+ LOG_ERROR("Failed to allocate memory");
return ERROR_FAIL;
- str_to_buf(CMD_ARGV[1], strlen(CMD_ARGV[1]), buf, reg->size, 0);
+ }
+
+ int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[1], buf, reg->size, 0);
+ if (retval != ERROR_OK) {
+ free(buf);
+ return retval;
+ }
- int retval = reg->type->set(reg, buf);
+ retval = reg->type->set(reg, buf);
if (retval != ERROR_OK) {
LOG_ERROR("Could not write to register '%s'", reg->name);
} else {
@@ -4788,63 +4795,64 @@ static int target_jim_get_reg(Jim_Interp *interp, int argc,
return JIM_OK;
}
-static int target_jim_set_reg(Jim_Interp *interp, int argc,
- Jim_Obj * const *argv)
+COMMAND_HANDLER(handle_set_reg_command)
{
- if (argc != 2) {
- Jim_WrongNumArgs(interp, 1, argv, "dict");
- return JIM_ERR;
- }
+ if (CMD_ARGC != 1)
+ return ERROR_COMMAND_SYNTAX_ERROR;
int tmp;
#if JIM_VERSION >= 80
- Jim_Obj **dict = Jim_DictPairs(interp, argv[1], &tmp);
+ Jim_Obj **dict = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &tmp);
if (!dict)
- return JIM_ERR;
+ return ERROR_FAIL;
#else
Jim_Obj **dict;
- int ret = Jim_DictPairs(interp, argv[1], &dict, &tmp);
+ int ret = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &dict, &tmp);
if (ret != JIM_OK)
- return ret;
+ return ERROR_FAIL;
#endif
const unsigned int length = tmp;
- struct command_context *cmd_ctx = current_command_context(interp);
- assert(cmd_ctx);
- const struct target *target = get_current_target(cmd_ctx);
+
+ const struct target *target = get_current_target(CMD_CTX);
+ assert(target);
for (unsigned int i = 0; i < length; i += 2) {
const char *reg_name = Jim_String(dict[i]);
const char *reg_value = Jim_String(dict[i + 1]);
- struct reg *reg = register_get_by_name(target->reg_cache, reg_name,
- false);
+ struct reg *reg = register_get_by_name(target->reg_cache, reg_name, false);
if (!reg || !reg->exist) {
- Jim_SetResultFormatted(interp, "unknown register '%s'", reg_name);
- return JIM_ERR;
+ command_print(CMD, "unknown register '%s'", reg_name);
+ return ERROR_FAIL;
}
uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8));
-
if (!buf) {
LOG_ERROR("Failed to allocate memory");
- return JIM_ERR;
+ return ERROR_FAIL;
}
- str_to_buf(reg_value, strlen(reg_value), buf, reg->size, 0);
- int retval = reg->type->set(reg, buf);
+ int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf,
+ reg_value, buf, reg->size, 0);
+ if (retval != ERROR_OK) {
+ free(buf);
+ return retval;
+ }
+
+ retval = reg->type->set(reg, buf);
free(buf);
if (retval != ERROR_OK) {
- Jim_SetResultFormatted(interp, "failed to set '%s' to register '%s'",
+ command_print(CMD, "failed to set '%s' to register '%s'",
reg_value, reg_name);
- return JIM_ERR;
+ return retval;
}
}
- return JIM_OK;
+ return ERROR_OK;
}
/**
@@ -5584,7 +5592,7 @@ static const struct command_registration target_instance_command_handlers[] = {
{
.name = "set_reg",
.mode = COMMAND_EXEC,
- .jim_handler = target_jim_set_reg,
+ .handler = handle_set_reg_command,
.help = "Set target register values",
.usage = "dict",
},
@@ -6719,7 +6727,7 @@ static const struct command_registration target_exec_command_handlers[] = {
{
.name = "set_reg",
.mode = COMMAND_EXEC,
- .jim_handler = target_jim_set_reg,
+ .handler = handle_set_reg_command,
.help = "Set target register values",
.usage = "dict",
},
-----------------------------------------------------------------------
Summary of changes:
src/helper/binarybuffer.c | 158 ++++++++++++++++++++++++++++++++--------------
src/helper/binarybuffer.h | 17 ++++-
src/helper/command.c | 40 ++++++++++++
src/helper/command.h | 11 ++++
src/jtag/tcl.c | 5 +-
src/target/target.c | 66 ++++++++++---------
6 files changed, 216 insertions(+), 81 deletions(-)
hooks/post-receive
--
Main OpenOCD repository
|
