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 re-defining EXCP_DUMP() to call
target_exception_dump(), which is in syscall.c, consolidates the log
print functions and allows to add the call to dump the memory layout.
Beside a reduced code footprint, this approach keeps the changes across
the various callers minimal, and keeps EXCP_DUMP() highlighted as
important macro/function.
Signed-off-by: Helge Deller <deller@gmx.de>
---
v2:
Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
to excp_dump_file() and target_exception_dump(), and #define'ed
EXCP_DUMP() to target_exception_dump().
I intentionally did not replace all occurences of EXCP_DUMP() by
target_exception_dump() as I think it's unneccesary and not beneficial.
If this is really wished, I will send a v3.
diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
index 36ff5b14f2..e644d2ef90 100644
--- a/linux-user/cpu_loop-common.h
+++ b/linux-user/cpu_loop-common.h
@@ -23,18 +23,9 @@
#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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
+#define EXCP_DUMP(env, fmt, code) \
+ target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
@@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
return 0;
}
+static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
+{
+ /* dump to console */
+ excp_dump_file(stderr, env, fmt, code);
+
+ /* dump to log file */
+ if (qemu_log_separate()) {
+ FILE *logfile = qemu_log_trylock();
+
+ excp_dump_file(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)Le 27/09/2022 à 18:58, Helge Deller a écrit :
> 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 re-defining EXCP_DUMP() to call
> target_exception_dump(), which is in syscall.c, consolidates the log
> print functions and allows to add the call to dump the memory layout.
>
> Beside a reduced code footprint, this approach keeps the changes across
> the various callers minimal, and keeps EXCP_DUMP() highlighted as
> important macro/function.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> ---
>
> v2:
> Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
> to excp_dump_file() and target_exception_dump(), and #define'ed
> EXCP_DUMP() to target_exception_dump().
> I intentionally did not replace all occurences of EXCP_DUMP() by
> target_exception_dump() as I think it's unneccesary and not beneficial.
> If this is really wished, I will send a v3.
>
>
> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
> index 36ff5b14f2..e644d2ef90 100644
> --- a/linux-user/cpu_loop-common.h
> +++ b/linux-user/cpu_loop-common.h
> @@ -23,18 +23,9 @@
> #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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
> +#define EXCP_DUMP(env, fmt, code) \
> + target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
> @@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
> return 0;
> }
>
> +static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
> +{
> + /* dump to console */
> + excp_dump_file(stderr, env, fmt, code);
> +
> + /* dump to log file */
> + if (qemu_log_separate()) {
> + FILE *logfile = qemu_log_trylock();
> +
> + excp_dump_file(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)
>
Applied to my linux-user-for-7.2 branch.
Thanks,
Laurent
Le 21/10/2022 à 16:57, Laurent Vivier a écrit :
> Le 27/09/2022 à 18:58, Helge Deller a écrit :
>> 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 re-defining EXCP_DUMP() to call
>> target_exception_dump(), which is in syscall.c, consolidates the log
>> print functions and allows to add the call to dump the memory layout.
>>
>> Beside a reduced code footprint, this approach keeps the changes across
>> the various callers minimal, and keeps EXCP_DUMP() highlighted as
>> important macro/function.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> ---
>>
>> v2:
>> Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
>> to excp_dump_file() and target_exception_dump(), and #define'ed
>> EXCP_DUMP() to target_exception_dump().
>> I intentionally did not replace all occurences of EXCP_DUMP() by
>> target_exception_dump() as I think it's unneccesary and not beneficial.
>> If this is really wished, I will send a v3.
>>
>>
>> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
>> index 36ff5b14f2..e644d2ef90 100644
>> --- a/linux-user/cpu_loop-common.h
>> +++ b/linux-user/cpu_loop-common.h
>> @@ -23,18 +23,9 @@
>> #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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
>> +#define EXCP_DUMP(env, fmt, code) \
>> + target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
>> @@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
>> return 0;
>> }
>>
>> +static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
>> +{
>> + /* dump to console */
>> + excp_dump_file(stderr, env, fmt, code);
>> +
>> + /* dump to log file */
>> + if (qemu_log_separate()) {
>> + FILE *logfile = qemu_log_trylock();
>> +
>> + excp_dump_file(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)
>>
>
> Applied to my linux-user-for-7.2 branch.
This breaks build with:
.../linux-user/i386/cpu_loop.c: In function 'cpu_loop':
...linux-user/i386/cpu_loop.c:312:39: error: macro "EXCP_DUMP" passed 4 arguments, but takes just 3
312 | (long)pc, trapnr);
| ^
In file included from .../linux-user/i386/cpu_loop.c:24:
.../linux-user/cpu_loop-common.h:27: note: macro "EXCP_DUMP" defined here
27 | #define EXCP_DUMP(env, fmt, code) \
|
.../linux-user/i386/cpu_loop.c:311:13: error: 'EXCP_DUMP' undeclared (first use in this function)
311 | EXCP_DUMP(env, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
| ^~~~~~~~~
.../linux-user/i386/cpu_loop.c:311:13: note: each undeclared identifier is reported only once for
each function it appears in
.../linux-user/i386/cpu_loop.c:204:15: error: variable 'pc' set but not used
[-Werror=unused-but-set-variable]
204 | abi_ulong pc;
| ^~
cc1: all warnings being treated as errors
On 10/21/22 17:43, Laurent Vivier wrote:
> Le 21/10/2022 à 16:57, Laurent Vivier a écrit :
>> Le 27/09/2022 à 18:58, Helge Deller a écrit :
>>> 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 re-defining EXCP_DUMP() to call
>>> target_exception_dump(), which is in syscall.c, consolidates the log
>>> print functions and allows to add the call to dump the memory layout.
>>>
>>> Beside a reduced code footprint, this approach keeps the changes across
>>> the various callers minimal, and keeps EXCP_DUMP() highlighted as
>>> important macro/function.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>
>>> ---
>>>
>>> v2:
>>> Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
>>> to excp_dump_file() and target_exception_dump(), and #define'ed
>>> EXCP_DUMP() to target_exception_dump().
>>> I intentionally did not replace all occurences of EXCP_DUMP() by
>>> target_exception_dump() as I think it's unneccesary and not beneficial.
>>> If this is really wished, I will send a v3.
>>>
>>>
>>> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
>>> index 36ff5b14f2..e644d2ef90 100644
>>> --- a/linux-user/cpu_loop-common.h
>>> +++ b/linux-user/cpu_loop-common.h
>>> @@ -23,18 +23,9 @@
>>> #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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
>>> +#define EXCP_DUMP(env, fmt, code) \
>>> + target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
>>> @@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
>>> return 0;
>>> }
>>>
>>> +static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
>>> +{
>>> + /* dump to console */
>>> + excp_dump_file(stderr, env, fmt, code);
>>> +
>>> + /* dump to log file */
>>> + if (qemu_log_separate()) {
>>> + FILE *logfile = qemu_log_trylock();
>>> +
>>> + excp_dump_file(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)
>>>
>>
>> Applied to my linux-user-for-7.2 branch.
>
> This breaks build with:
>
> .../linux-user/i386/cpu_loop.c: In function 'cpu_loop':
> ...linux-user/i386/cpu_loop.c:312:39: error: macro "EXCP_DUMP" passed 4 arguments, but takes just 3
> 312 | (long)pc, trapnr);
> | ^
This is because of this line:
EXCP_DUMP(env, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
(long)pc, trapnr);
I wonder if it is ok to drop the pc value. It should be printed in the register dump
anyway.
If Ok, should I send a new v3 patch, or a patch in front of my v2 patch?
Helge
Le 21/10/2022 à 18:21, Helge Deller a écrit :
> On 10/21/22 17:43, Laurent Vivier wrote:
>> Le 21/10/2022 à 16:57, Laurent Vivier a écrit :
>>> Le 27/09/2022 à 18:58, Helge Deller a écrit :
>>>> 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 re-defining EXCP_DUMP() to call
>>>> target_exception_dump(), which is in syscall.c, consolidates the log
>>>> print functions and allows to add the call to dump the memory layout.
>>>>
>>>> Beside a reduced code footprint, this approach keeps the changes across
>>>> the various callers minimal, and keeps EXCP_DUMP() highlighted as
>>>> important macro/function.
>>>>
>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>>
>>>> ---
>>>>
>>>> v2:
>>>> Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
>>>> to excp_dump_file() and target_exception_dump(), and #define'ed
>>>> EXCP_DUMP() to target_exception_dump().
>>>> I intentionally did not replace all occurences of EXCP_DUMP() by
>>>> target_exception_dump() as I think it's unneccesary and not beneficial.
>>>> If this is really wished, I will send a v3.
>>>>
>>>>
>>>> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
>>>> index 36ff5b14f2..e644d2ef90 100644
>>>> --- a/linux-user/cpu_loop-common.h
>>>> +++ b/linux-user/cpu_loop-common.h
>>>> @@ -23,18 +23,9 @@
>>>> #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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
>>>> +#define EXCP_DUMP(env, fmt, code) \
>>>> + target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
>>>> @@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
>>>> return 0;
>>>> }
>>>>
>>>> +static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
>>>> +{
>>>> + /* dump to console */
>>>> + excp_dump_file(stderr, env, fmt, code);
>>>> +
>>>> + /* dump to log file */
>>>> + if (qemu_log_separate()) {
>>>> + FILE *logfile = qemu_log_trylock();
>>>> +
>>>> + excp_dump_file(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)
>>>>
>>>
>>> Applied to my linux-user-for-7.2 branch.
>>
>> This breaks build with:
>>
>> .../linux-user/i386/cpu_loop.c: In function 'cpu_loop':
>> ...linux-user/i386/cpu_loop.c:312:39: error: macro "EXCP_DUMP" passed 4 arguments, but takes just 3
>> 312 | (long)pc, trapnr);
>> | ^
>
> This is because of this line:
> EXCP_DUMP(env, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
> (long)pc, trapnr);
>
> I wonder if it is ok to drop the pc value. It should be printed in the register dump
> anyway.
> If Ok, should I send a new v3 patch, or a patch in front of my v2 patch?
I think you can drop the PC value. Send a v3 please.
Thanks,
Laurent
Le 27/09/2022 à 18:58, Helge Deller a écrit :
> 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 re-defining EXCP_DUMP() to call
> target_exception_dump(), which is in syscall.c, consolidates the log
> print functions and allows to add the call to dump the memory layout.
>
> Beside a reduced code footprint, this approach keeps the changes across
> the various callers minimal, and keeps EXCP_DUMP() highlighted as
> important macro/function.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> ---
>
> v2:
> Based on feedback by Philippe Mathieu-Daudé, renamed the two functions
> to excp_dump_file() and target_exception_dump(), and #define'ed
> EXCP_DUMP() to target_exception_dump().
> I intentionally did not replace all occurences of EXCP_DUMP() by
> target_exception_dump() as I think it's unneccesary and not beneficial.
> If this is really wished, I will send a v3.
>
>
> diff --git a/linux-user/cpu_loop-common.h b/linux-user/cpu_loop-common.h
> index 36ff5b14f2..e644d2ef90 100644
> --- a/linux-user/cpu_loop-common.h
> +++ b/linux-user/cpu_loop-common.h
> @@ -23,18 +23,9 @@
> #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 target_exception_dump(CPUArchState *env, const char *fmt, int code);
> +#define EXCP_DUMP(env, fmt, code) \
> + target_exception_dump(env, fmt, 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 2e954d8dbd..7d29c4c396 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 */
> @@ -8144,6 +8145,33 @@ static int is_proc_myself(const char *filename, const char *entry)
> return 0;
> }
>
> +static void excp_dump_file(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 target_exception_dump(CPUArchState *env, const char *fmt, int code)
> +{
> + /* dump to console */
> + excp_dump_file(stderr, env, fmt, code);
> +
> + /* dump to log file */
> + if (qemu_log_separate()) {
> + FILE *logfile = qemu_log_trylock();
> +
> + excp_dump_file(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)
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
© 2016 - 2024 Red Hat, Inc.