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)
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.
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
© 2016 - 2024 Red Hat, Inc.