diff mbox series

regs: Allow user to opt in to backtrace

Message ID 20181015010241.1255-1-joel@jms.id.au
State Accepted
Headers show
Series regs: Allow user to opt in to backtrace | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied
snowpatch_ozlabs/build-multiarch fail Test build-multiarch on branch master

Commit Message

Joel Stanley Oct. 15, 2018, 1:02 a.m. UTC
The backtrace causes us to read memory which appears to have a high
likelihood of causing a checkstop due to an invalid address.

This adds a --backtrace flag to the regs command which enables the
backtrace. By default it is off to protect the user.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 src/main.c   |  2 +-
 src/optcmd.h | 20 ++++++++++++++++++++
 src/thread.c | 19 +++++++++++++++----
 3 files changed, 36 insertions(+), 5 deletions(-)

Comments

Alistair Popple Oct. 15, 2018, 5:08 a.m. UTC | #1
Thanks Joel, good idea. However I am going to split this in to two commits so I
can add a test for OPTCMD_DEFINE_CMD_ONLY_FLAGS along the way.

- Alistair

On Monday, 15 October 2018 11:32:41 AM AEDT Joel Stanley wrote:
> The backtrace causes us to read memory which appears to have a high
> likelihood of causing a checkstop due to an invalid address.
> 
> This adds a --backtrace flag to the regs command which enables the
> backtrace. By default it is off to protect the user.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  src/main.c   |  2 +-
>  src/optcmd.h | 20 ++++++++++++++++++++
>  src/thread.c | 19 +++++++++++++++----
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/src/main.c b/src/main.c
> index 8b7e2ddae6cc..4966d511c62e 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -135,7 +135,7 @@ static struct action actions[] = {
>  	{ "putmem",  "<address>", "Write to system memory" },
>  	{ "threadstatus", "", "Print the status of a thread" },
>  	{ "sreset",  "", "Reset" },
> -	{ "regs",  "", "State" },
> +	{ "regs",  "[--backtrace]", "State (optionally display backtrace)" },
>  };
>  
>  static void print_usage(char *pname)
> diff --git a/src/optcmd.h b/src/optcmd.h
> index e5f688e13ea6..7d1bf2d6613c 100644
> --- a/src/optcmd.h
> +++ b/src/optcmd.h
> @@ -179,6 +179,26 @@ struct optcmd_cmd {
>  		.cmdp = __##cmd_func_,			\
>  	}
>  
> +/*
> + * Defines a new command taking no arguments or flags.
> + *  @cmd_name - name of command used on the command line
> + *  @cmd_func - pointer to the function to call for this command
> + */
> +#define OPTCMD_DEFINE_CMD_ONLY_FLAGS(cmd_name_, cmd_func_, flag_, flags_) \
> +        int cmd_func_(struct flag_);                    \
> +    int __##cmd_func_(void *args[], void *flags[])            \
> +    {                                \
> +        struct flag_ flag;                    \
> +        CPPMAGIC_JOIN(, CPPMAGIC_MAP(OPTCMD_CAST_FLAGS, CPPMAGIC_EVAL flags_)) \
> +        return cmd_func_(flag);                    \
> +    }                                \
> +                                    \
> +    struct optcmd_cmd optcmd_##cmd_name_ = {            \
> +        .cmd = #cmd_name_,                    \
> +        .cmdp = __##cmd_func_,                    \
> +        .flags = { CPPMAGIC_MAP(OPTCMD_FLAG, CPPMAGIC_EVAL flags_), {NULL} }, \
> +    }
> +
>  optcmd_cmd_t *optcmd_parse(struct optcmd_cmd *cmd, const char *argv[], int argc,
>  			   void **args[], void **flags[]);
>  
> diff --git a/src/thread.c b/src/thread.c
> index d282307374c5..67b6c46443d7 100644
> --- a/src/thread.c
> +++ b/src/thread.c
> @@ -188,11 +188,13 @@ static int sreset_thread(struct pdbg_target *thread_target, uint32_t index, uint
>  static int state_thread(struct pdbg_target *thread_target, uint32_t index, uint64_t *unused, uint64_t *unused1)
>  {
>  	struct thread_regs regs;
> +	bool do_backtrace = (bool)unused;
>  
>  	if (ram_state_thread(thread_target, &regs))
>  		return 0;
>  
> -	dump_stack(&regs);
> +	if (do_backtrace)
> +		dump_stack(&regs);
>  
>  	return 1;
>  }
> @@ -227,14 +229,23 @@ static int thread_sreset(void)
>  }
>  OPTCMD_DEFINE_CMD(sreset, thread_sreset);
>  
> -static int thread_state(void)
> +
> +struct reg_flags {
> +	bool do_backtrace;
> +};
> +
> +#define REG_BACKTRACE_FLAG ("--backtrace", do_backtrace, parse_flag_noarg, false)
> +
> +static int thread_state(struct reg_flags flags)
>  {
>  	int err;
>  
> -	err = for_each_target("thread", state_thread, NULL, NULL);
> +	err = for_each_target("thread", state_thread,
> +			(uint64_t *)flags.do_backtrace, NULL);
>  
>  	for_each_target_release("thread");
>  
>  	return err;
>  }
> -OPTCMD_DEFINE_CMD(regs, thread_state);
> +OPTCMD_DEFINE_CMD_ONLY_FLAGS(regs, thread_state,
> +                  reg_flags, (REG_BACKTRACE_FLAG));
>
diff mbox series

Patch

diff --git a/src/main.c b/src/main.c
index 8b7e2ddae6cc..4966d511c62e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -135,7 +135,7 @@  static struct action actions[] = {
 	{ "putmem",  "<address>", "Write to system memory" },
 	{ "threadstatus", "", "Print the status of a thread" },
 	{ "sreset",  "", "Reset" },
-	{ "regs",  "", "State" },
+	{ "regs",  "[--backtrace]", "State (optionally display backtrace)" },
 };
 
 static void print_usage(char *pname)
diff --git a/src/optcmd.h b/src/optcmd.h
index e5f688e13ea6..7d1bf2d6613c 100644
--- a/src/optcmd.h
+++ b/src/optcmd.h
@@ -179,6 +179,26 @@  struct optcmd_cmd {
 		.cmdp = __##cmd_func_,			\
 	}
 
+/*
+ * Defines a new command taking no arguments or flags.
+ *  @cmd_name - name of command used on the command line
+ *  @cmd_func - pointer to the function to call for this command
+ */
+#define OPTCMD_DEFINE_CMD_ONLY_FLAGS(cmd_name_, cmd_func_, flag_, flags_) \
+        int cmd_func_(struct flag_);                    \
+    int __##cmd_func_(void *args[], void *flags[])            \
+    {                                \
+        struct flag_ flag;                    \
+        CPPMAGIC_JOIN(, CPPMAGIC_MAP(OPTCMD_CAST_FLAGS, CPPMAGIC_EVAL flags_)) \
+        return cmd_func_(flag);                    \
+    }                                \
+                                    \
+    struct optcmd_cmd optcmd_##cmd_name_ = {            \
+        .cmd = #cmd_name_,                    \
+        .cmdp = __##cmd_func_,                    \
+        .flags = { CPPMAGIC_MAP(OPTCMD_FLAG, CPPMAGIC_EVAL flags_), {NULL} }, \
+    }
+
 optcmd_cmd_t *optcmd_parse(struct optcmd_cmd *cmd, const char *argv[], int argc,
 			   void **args[], void **flags[]);
 
diff --git a/src/thread.c b/src/thread.c
index d282307374c5..67b6c46443d7 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -188,11 +188,13 @@  static int sreset_thread(struct pdbg_target *thread_target, uint32_t index, uint
 static int state_thread(struct pdbg_target *thread_target, uint32_t index, uint64_t *unused, uint64_t *unused1)
 {
 	struct thread_regs regs;
+	bool do_backtrace = (bool)unused;
 
 	if (ram_state_thread(thread_target, &regs))
 		return 0;
 
-	dump_stack(&regs);
+	if (do_backtrace)
+		dump_stack(&regs);
 
 	return 1;
 }
@@ -227,14 +229,23 @@  static int thread_sreset(void)
 }
 OPTCMD_DEFINE_CMD(sreset, thread_sreset);
 
-static int thread_state(void)
+
+struct reg_flags {
+	bool do_backtrace;
+};
+
+#define REG_BACKTRACE_FLAG ("--backtrace", do_backtrace, parse_flag_noarg, false)
+
+static int thread_state(struct reg_flags flags)
 {
 	int err;
 
-	err = for_each_target("thread", state_thread, NULL, NULL);
+	err = for_each_target("thread", state_thread,
+			(uint64_t *)flags.do_backtrace, NULL);
 
 	for_each_target_release("thread");
 
 	return err;
 }
-OPTCMD_DEFINE_CMD(regs, thread_state);
+OPTCMD_DEFINE_CMD_ONLY_FLAGS(regs, thread_state,
+                  reg_flags, (REG_BACKTRACE_FLAG));