[PATCH]: 45c4f4ed50 rtos: Dynamically allocate memory for RTOS registers

[<ge...@op...>]

This is an automated email from Gerrit.

"Tomas Vanek <va...@fb...>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/9123

-- gerrit

commit 45c4f4ed503da0b567d98b7807d5d6489afcb867
Author: Tim Newsome <ti...@si...>
Date:   Tue Oct 5 10:03:53 2021 -0700

    rtos: Dynamically allocate memory for RTOS registers
    
    Imported from
    https://github.com/riscv-collab/riscv-openocd/pull/647
    
    FreeRTOS part of patch dropped as it depends on other
    riscv-collab specific changes.
    
    This makes things work on RISC-V cores with large vector registers
    (which can be up to kilobytes in size).
    
    Change-Id: Ie53cb43a88e2a475f695cd5c1e28605569926817
    Signed-off-by: Tim Newsome <ti...@si...>
    Signed-off-by: Tomas Vanek <va...@fb...>

diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c
index b2a45ade9f..e34d92d9d5 100644
--- a/src/rtos/hwthread.c
+++ b/src/rtos/hwthread.c
@@ -17,8 +17,8 @@
 static bool hwthread_detect_rtos(struct target *target);
 static int hwthread_create(struct target *target);
 static int hwthread_update_threads(struct rtos *rtos);
-static int hwthread_get_thread_reg(struct rtos *rtos, int64_t thread_id,
-		uint32_t reg_num, struct rtos_reg *rtos_reg);
+static int hwthread_get_thread_reg_value(struct rtos *rtos, int64_t thread_id,
+		uint32_t reg_num, uint32_t *size, uint8_t **value);
 static int hwthread_get_thread_reg_list(struct rtos *rtos, int64_t thread_id,
 		struct rtos_reg **reg_list, int *num_regs);
 static int hwthread_get_symbol_list_to_lookup(struct symbol_table_elem *symbol_list[]);
@@ -53,7 +53,7 @@ const struct rtos_type hwthread_rtos = {
 	.create = hwthread_create,
 	.update_threads = hwthread_update_threads,
 	.get_thread_reg_list = hwthread_get_thread_reg_list,
-	.get_thread_reg = hwthread_get_thread_reg,
+	.get_thread_reg_value = hwthread_get_thread_reg_value,
 	.get_symbol_list_to_lookup = hwthread_get_symbol_list_to_lookup,
 	.smp_init = hwthread_smp_init,
 	.set_reg = hwthread_set_reg,
@@ -291,8 +291,8 @@ static int hwthread_get_thread_reg_list(struct rtos *rtos, int64_t thread_id,
 	return ERROR_OK;
 }
 
-static int hwthread_get_thread_reg(struct rtos *rtos, int64_t thread_id,
-		uint32_t reg_num, struct rtos_reg *rtos_reg)
+static int hwthread_get_thread_reg_value(struct rtos *rtos, int64_t thread_id,
+		uint32_t reg_num, uint32_t *size, uint8_t **value)
 {
 	if (!rtos)
 		return ERROR_FAIL;
@@ -321,11 +321,14 @@ static int hwthread_get_thread_reg(struct rtos *rtos, int64_t thread_id,
 	if (reg->type->get(reg) != ERROR_OK)
 		return ERROR_FAIL;
 
-	rtos_reg->number = reg->number;
-	rtos_reg->size = reg->size;
-	unsigned int bytes = (reg->size + 7) / 8;
-	assert(bytes <= sizeof(rtos_reg->value));
-	memcpy(rtos_reg->value, reg->value, bytes);
+	*size = reg->size;
+	unsigned bytes = DIV_ROUND_UP(reg->size, 8);
+	*value = malloc(bytes);
+	if (!*value) {
+		LOG_ERROR("Failed to allocate memory for %d-bit register.", reg->size);
+		return ERROR_FAIL;
+	}
+	memcpy(*value, reg->value, bytes);
 
 	return ERROR_OK;
 }
diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 2ccccf1b0d..45366268de 100644
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -545,15 +545,33 @@ int rtos_get_gdb_reg(struct connection *connection, int reg_num)
 										target->rtos->current_thread);
 
 		int retval;
-		if (target->rtos->type->get_thread_reg) {
-			reg_list = calloc(1, sizeof(*reg_list));
-			num_regs = 1;
-			retval = target->rtos->type->get_thread_reg(target->rtos,
-					current_threadid, reg_num, &reg_list[0]);
+		if (target->rtos->type->get_thread_reg_value) {
+			uint32_t reg_size;
+			uint8_t *reg_value;
+			retval = target->rtos->type->get_thread_reg_value(target->rtos,
+					current_threadid, reg_num, &reg_size, &reg_value);
 			if (retval != ERROR_OK) {
 				LOG_ERROR("RTOS: failed to get register %d", reg_num);
 				return retval;
 			}
+
+			/* Create a reg_list with one register that can
+			 * accommodate the full size of the one we just got the
+			 * value for. To do that we allocate extra space off the
+			 * end of the struct, relying on the fact that
+			 * rtos_reg.value is the last element in the struct. */
+			reg_list = calloc(1, sizeof(*reg_list) + DIV_ROUND_UP(reg_size, 8));
+			if (!reg_list) {
+				free(reg_value);
+				LOG_ERROR("Failed to allocated reg_list for %d-byte register.",
+					  reg_size);
+				return ERROR_FAIL;
+			}
+			reg_list[0].number = reg_num;
+			reg_list[0].size = reg_size;
+			memcpy(&reg_list[0].value, reg_value, DIV_ROUND_UP(reg_size, 8));
+			free(reg_value);
+			num_regs = 1;
 		} else {
 			retval = target->rtos->type->get_thread_reg_list(target->rtos,
 					current_threadid,
diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index dbaa7e8ce8..3077acc9d4 100644
--- a/src/rtos/rtos.h
+++ b/src/rtos/rtos.h
@@ -53,6 +53,8 @@ struct rtos_reg {
 	uint32_t number;
 	uint32_t size;
 	uint8_t value[16];
+	/* WARNING: rtos_get_gdb_reg() relies on the fact that value is the last
+	 * element of this struct. Any new fields should be added *before* value. */
 };
 
 struct rtos_type {
@@ -64,8 +66,10 @@ struct rtos_type {
 	/** Return a list of general registers, with their values filled out. */
 	int (*get_thread_reg_list)(struct rtos *rtos, int64_t thread_id,
 			struct rtos_reg **reg_list, int *num_regs);
-	int (*get_thread_reg)(struct rtos *rtos, int64_t thread_id,
-			uint32_t reg_num, struct rtos_reg *reg);
+	/** Return the size and value of the specified reg_num. The value is
+	 * allocated by the callee and freed by the caller. */
+	int (*get_thread_reg_value)(struct rtos *rtos, threadid_t thread_id,
+			uint32_t reg_num, uint32_t *size, uint8_t **value);
 	int (*get_symbol_list_to_lookup)(struct symbol_table_elem *symbol_list[]);
 	int (*clean)(struct target *target);
 	char * (*ps_command)(struct target *target);

--