Hi Philippe,
On 9/27/22 13:13, Philippe Mathieu-Daudé wrote:
On Tue, Sep 27, 2022 at 12:09 PM Helge Deller <del...@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 <del...@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
