* [PATCH] linux-user: Add guest memory layout to exception dump
@ 2022-09-27 9:59 Helge Deller
2022-09-27 11:13 ` Philippe Mathieu-Daudé via
0 siblings, 1 reply; 3+ messages in thread
From: Helge Deller @ 2022-09-27 9:59 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
When the emulation stops with a hard exception it's very useful for
debugging purposes to dump the current guest memory layout (for an
example see /proc/self/maps) beside the CPU registers.
The open_self_maps() function provides such a memory dump, but since
it's located in the syscall.c file, various changes (add #includes, make
this function externally visible, ...) are needed to be able to call it
from the existing EXCP_DUMP() macro.
This patch takes another approach by un-macronizing EXCP_DUMP() and turn
it into a function located in syscall.c.
Beside a reduced code footprint, this approach allows to add the memory
dump and simplify the code to print to console and log file.
Signed-off-by: Helge Deller <deller@gmx.de>
diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
index 36ff5b14f2..0b26b56915 100644
--- a/linux-user/cpu_loop-common.h
+++ b/linux-user/cpu_loop-common.h
@@ -23,18 +23,7 @@
#include "exec/log.h"
#include "special-errno.h"
-#define EXCP_DUMP(env, fmt, ...) \
-do { \
- CPUState *cs = env_cpu(env); \
- fprintf(stderr, fmt , ## __VA_ARGS__); \
- fprintf(stderr, "Failing executable: %s\n", exec_path); \
- cpu_dump_state(cs, stderr, 0); \
- if (qemu_log_separate()) { \
- qemu_log(fmt, ## __VA_ARGS__); \
- qemu_log("Failing executable: %s\n", exec_path); \
- log_cpu_state(cs, 0); \
- } \
-} while (0)
+void EXCP_DUMP(CPUArchState *env, const char *fmt, int code);
void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs);
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d17f5d1c66..00861e9351 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -158,6 +158,7 @@
#include "qapi/error.h"
#include "fd-trans.h"
#include "tcg/tcg.h"
+#include "cpu_loop-common.h"
#ifndef CLONE_IO
#define CLONE_IO 0x80000000 /* Clone io context */
@@ -8177,6 +8178,33 @@ static int is_proc_myself(const char *filename, const char *entry)
return 0;
}
+static void excp_dump(FILE *logfile, CPUArchState *env,
+ const char *fmt, int code)
+{
+ if (logfile) {
+ CPUState *cs = env_cpu(env);
+
+ fprintf(logfile, fmt, code);
+ fprintf(logfile, "Failing executable: %s\n", exec_path);
+ cpu_dump_state(cs, logfile, 0);
+ open_self_maps(env, fileno(logfile));
+ }
+}
+
+void EXCP_DUMP(CPUArchState *env, const char *fmt, int code)
+{
+ /* dump to console */
+ excp_dump(stderr, env, fmt, code);
+
+ /* dump to log file */
+ if (qemu_log_separate()) {
+ FILE *logfile = qemu_log_trylock();
+
+ excp_dump(logfile, env, fmt, code);
+ qemu_log_unlock(logfile);
+ }
+}
+
#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
static int is_proc(const char *filename, const char *entry)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] linux-user: Add guest memory layout to exception dump
2022-09-27 9:59 [PATCH] linux-user: Add guest memory layout to exception dump Helge Deller
@ 2022-09-27 11:13 ` Philippe Mathieu-Daudé via
2022-09-27 11:44 ` Helge Deller
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-09-27 11:13 UTC (permalink / raw)
To: Helge Deller; +Cc: Laurent Vivier, qemu-devel@nongnu.org Developers
Hi Helge,
On Tue, Sep 27, 2022 at 12:09 PM Helge Deller <deller@gmx.de> wrote:
>
> When the emulation stops with a hard exception it's very useful for
> debugging purposes to dump the current guest memory layout (for an
> example see /proc/self/maps) beside the CPU registers.
>
> The open_self_maps() function provides such a memory dump, but since
> it's located in the syscall.c file, various changes (add #includes, make
> this function externally visible, ...) are needed to be able to call it
> from the existing EXCP_DUMP() macro.
>
> This patch takes another approach by un-macronizing EXCP_DUMP() and turn
> it into a function located in syscall.c.
> Beside a reduced code footprint, this approach allows to add the memory
> dump and simplify the code to print to console and log file.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
> index 36ff5b14f2..0b26b56915 100644
> --- a/linux-user/cpu_loop-common.h
> +++ b/linux-user/cpu_loop-common.h
> @@ -23,18 +23,7 @@
> #include "exec/log.h"
> #include "special-errno.h"
>
> -#define EXCP_DUMP(env, fmt, ...) \
> -do { \
> - CPUState *cs = env_cpu(env); \
> - fprintf(stderr, fmt , ## __VA_ARGS__); \
> - fprintf(stderr, "Failing executable: %s\n", exec_path); \
> - cpu_dump_state(cs, stderr, 0); \
> - if (qemu_log_separate()) { \
> - qemu_log(fmt, ## __VA_ARGS__); \
> - qemu_log("Failing executable: %s\n", exec_path); \
> - log_cpu_state(cs, 0); \
> - } \
> -} while (0)
> +void EXCP_DUMP(CPUArchState *env, const char *fmt, int code);
s/EXCP_DUMP/target_cpu_dump_exception/
Worth split as a preliminary patch updating all targets.
> void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs);
> #endif
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d17f5d1c66..00861e9351 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -158,6 +158,7 @@
> #include "qapi/error.h"
> #include "fd-trans.h"
> #include "tcg/tcg.h"
> +#include "cpu_loop-common.h"
>
> #ifndef CLONE_IO
> #define CLONE_IO 0x80000000 /* Clone io context */
> @@ -8177,6 +8178,33 @@ static int is_proc_myself(const char *filename, const char *entry)
> return 0;
> }
>
> +static void excp_dump(FILE *logfile, CPUArchState *env,
> + const char *fmt, int code)
s/excp_dump/dump_excp_file/
> +{
> + if (logfile) {
Rather:
assert(logfile);
(programming error).
> + CPUState *cs = env_cpu(env);
> +
> + fprintf(logfile, fmt, code);
> + fprintf(logfile, "Failing executable: %s\n", exec_path);
> + cpu_dump_state(cs, logfile, 0);
> + open_self_maps(env, fileno(logfile));
> + }
> +}
> +
> +void EXCP_DUMP(CPUArchState *env, const char *fmt, int code)
s/EXCP_DUMP/target_cpu_dump_exception/
> +{
> + /* dump to console */
> + excp_dump(stderr, env, fmt, code);
> +
> + /* dump to log file */
> + if (qemu_log_separate()) {
> + FILE *logfile = qemu_log_trylock();
> +
> + excp_dump(logfile, env, fmt, code);
> + qemu_log_unlock(logfile);
> + }
> +}
Nitpicking a bit, otherwise LGTM.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] linux-user: Add guest memory layout to exception dump
2022-09-27 11:13 ` Philippe Mathieu-Daudé via
@ 2022-09-27 11:44 ` Helge Deller
0 siblings, 0 replies; 3+ messages in thread
From: Helge Deller @ 2022-09-27 11:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Laurent Vivier, qemu-devel@nongnu.org Developers
Hi Philippe,
On 9/27/22 13:13, Philippe Mathieu-Daudé wrote:
> On Tue, Sep 27, 2022 at 12:09 PM Helge Deller <deller@gmx.de> wrote:
>>
>> When the emulation stops with a hard exception it's very useful for
>> debugging purposes to dump the current guest memory layout (for an
>> example see /proc/self/maps) beside the CPU registers.
>>
>> The open_self_maps() function provides such a memory dump, but since
>> it's located in the syscall.c file, various changes (add #includes, make
>> this function externally visible, ...) are needed to be able to call it
>> from the existing EXCP_DUMP() macro.
>>
>> This patch takes another approach by un-macronizing EXCP_DUMP() and turn
>> it into a function located in syscall.c.
>> Beside a reduced code footprint, this approach allows to add the memory
>> dump and simplify the code to print to console and log file.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
>> index 36ff5b14f2..0b26b56915 100644
>> --- a/linux-user/cpu_loop-common.h
>> +++ b/linux-user/cpu_loop-common.h
>> @@ -23,18 +23,7 @@
>> #include "exec/log.h"
>> #include "special-errno.h"
>>
>> -#define EXCP_DUMP(env, fmt, ...) \
>> -do { \
>> - CPUState *cs = env_cpu(env); \
>> - fprintf(stderr, fmt , ## __VA_ARGS__); \
>> - fprintf(stderr, "Failing executable: %s\n", exec_path); \
>> - cpu_dump_state(cs, stderr, 0); \
>> - if (qemu_log_separate()) { \
>> - qemu_log(fmt, ## __VA_ARGS__); \
>> - qemu_log("Failing executable: %s\n", exec_path); \
>> - log_cpu_state(cs, 0); \
>> - } \
>> -} while (0)
>> +void EXCP_DUMP(CPUArchState *env, const char *fmt, int code);
>
> s/EXCP_DUMP/target_cpu_dump_exception/
> Worth split as a preliminary patch updating all targets.
Sure.
The idea was to get feedback first before touching that many files.
If people think this approach is right, I'll send a v2 with the targets
updated accordingly.
Maybe better: s/EXCP_DUMP/target_exception_dump/
>> void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs);
>> #endif
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index d17f5d1c66..00861e9351 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -158,6 +158,7 @@
>> #include "qapi/error.h"
>> #include "fd-trans.h"
>> #include "tcg/tcg.h"
>> +#include "cpu_loop-common.h"
>>
>> #ifndef CLONE_IO
>> #define CLONE_IO 0x80000000 /* Clone io context */
>> @@ -8177,6 +8178,33 @@ static int is_proc_myself(const char *filename, const char *entry)
>> return 0;
>> }
>>
>> +static void excp_dump(FILE *logfile, CPUArchState *env,
>> + const char *fmt, int code)
>
> s/excp_dump/dump_excp_file/
Ok.
Maybe: s/excp_dump/excp_dump_file/ ?
>> +{
>> + if (logfile) {
>
> Rather:
>
> assert(logfile);
>
> (programming error).
I don't think this is a good idea here.
The logfile comes from qemu_log_trylock() which can return 0 and
this is ignored in many other places too.
The application will stop anyway, so aborting - just because we can't
write to the log file - doesn't seem appropriate for me.
>> + CPUState *cs = env_cpu(env);
>> +
>> + fprintf(logfile, fmt, code);
>> + fprintf(logfile, "Failing executable: %s\n", exec_path);
>> + cpu_dump_state(cs, logfile, 0);
>> + open_self_maps(env, fileno(logfile));
>> + }
>> +}
>> +
>> +void EXCP_DUMP(CPUArchState *env, const char *fmt, int code)
>
> s/EXCP_DUMP/target_cpu_dump_exception/
>
>> +{
>> + /* dump to console */
>> + excp_dump(stderr, env, fmt, code);
>> +
>> + /* dump to log file */
>> + if (qemu_log_separate()) {
>> + FILE *logfile = qemu_log_trylock();
>> +
>> + excp_dump(logfile, env, fmt, code);
>> + qemu_log_unlock(logfile);
>> + }
>> +}
>
> Nitpicking a bit, otherwise LGTM.
Thanks!
Helge
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-27 13:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-27 9:59 [PATCH] linux-user: Add guest memory layout to exception dump Helge Deller
2022-09-27 11:13 ` Philippe Mathieu-Daudé via
2022-09-27 11:44 ` Helge Deller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).