|
From: | Helge Deller |
Subject: | Re: [PATCH] linux-user: Add guest memory layout to exception dump |
Date: | Tue, 27 Sep 2022 13:44:47 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |