Hi Helge,
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.
> 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.
