Re: [Qemu-devel] [PATCH v4 05/46] windbg: added helper features

[Ladi Prosek]

On Mon, Dec 11, 2017 at 2:21 PM, Mihail Abakumov
 wrote:
> Added some helper features for windbgstub.
>
> Signed-off-by: Mihail Abakumov 
> Signed-off-by: Pavel Dovgalyuk 
> Signed-off-by: Dmitriy Koltunov 
> ---
>  include/exec/windbgstub-utils.h |   31 +++
>  include/exec/windbgstub.h   |6 ++
>  2 files changed, 37 insertions(+)
>
> diff --git a/include/exec/windbgstub-utils.h b/include/exec/windbgstub-utils.h
> index 2390597f1f..4c747fa23e 100755
> --- a/include/exec/windbgstub-utils.h
> +++ b/include/exec/windbgstub-utils.h
> @@ -13,7 +13,38 @@
>  #define WINDBGSTUB_UTILS_H
>
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "log.h"
> +#include "cpu.h"
>  #include "exec/windbgstub.h"
>  #include "exec/windbgkd.h"
>
> +#define WINDBG_DEBUG(...) do { \
> +if (WINDBG_DEBUG_ON) { \
> +qemu_log(WINDBG ": " __VA_ARGS__); \
> +qemu_log("\n");\
> +}  \
> +} while (false)
> +
> +#define WINDBG_ERROR(...) error_report(WINDBG ": " __VA_ARGS__)
> +
> +#define FMT_ADDR "addr:0x" TARGET_FMT_lx
> +#define FMT_ERR  "Error:%d"
> +
> +#define UINT8_P(ptr) ((uint8_t *) (ptr))
> +#define PTR(var) UINT8_P()
> +
> +#define sizeof_field(type, field) sizeof(((type *) NULL)->field)
> +
> +#define READ_VMEM(cpu, addr, type) ({ \
> +type _t;  \
> +cpu_memory_rw_debug(cpu, addr, PTR(_t), sizeof(type), 0); \
> +_t;   \
> +})
> +
> +typedef struct SizedBuf {
> +uint8_t *data;
> +size_t size;
> +} SizedBuf;
> +

Ah, I thought you'd remove it completely. I was thinking about
something like this (patch on top of your series):

diff --git a/include/exec/windbgstub-utils.h
b/include/exec/windbgstub-utils.h
index d3e737e031..ece1c8ce60 100755
--- a/include/exec/windbgstub-utils.h
+++ b/include/exec/windbgstub-utils.h
@@ -50,11 +50,6 @@
 # define ldtul_p(p) ldl_p(p)
 #endif

-typedef struct SizedBuf {
-uint8_t *data;
-size_t size;
-} SizedBuf;
-
 typedef struct InitedAddr {
 target_ulong addr;
 bool is_init;
@@ -97,8 +92,8 @@ void kd_api_query_memory(CPUState *cpu, PacketData
*pd);
 void kd_api_get_context_ex(CPUState *cpu, PacketData *pd);
 void kd_api_set_context_ex(CPUState *cpu, PacketData *pd);

-SizedBuf kd_gen_exception_sc(CPUState *cpu);
-SizedBuf kd_gen_load_symbols_sc(CPUState *cpu);
+void kd_gen_exception_sc(CPUState *cpu, DBGKD_ANY_WAIT_STATE_CHANGE
*sc);
+void kd_gen_load_symbols_sc(CPUState *cpu,
DBGKD_ANY_WAIT_STATE_CHANGE *sc);

 bool windbg_on_load(void);

diff --git a/target/i386/windbgstub.c b/target/i386/windbgstub.c
index c38bfa7448..6c42b95fe9 100755
--- a/target/i386/windbgstub.c
+++ b/target/i386/windbgstub.c
@@ -1184,40 +1184,25 @@ static void kd_init_state_change(CPUState
*cpu,
 }
 }

-SizedBuf kd_gen_exception_sc(CPUState *cpu)
+void kd_gen_exception_sc(CPUState *cpu, DBGKD_ANY_WAIT_STATE_CHANGE
*sc)
 {
 CPUArchState *env = cpu->env_ptr;
-DBGKD_ANY_WAIT_STATE_CHANGE *sc;
 DBGKM_EXCEPTION_RECORD64 *exc;
-SizedBuf buf;

-buf.size = sizeof(DBGKD_ANY_WAIT_STATE_CHANGE) + sizeof(int);
-buf.data = g_malloc0(buf.size);
-sc = (DBGKD_ANY_WAIT_STATE_CHANGE *) buf.data;
 exc = >u.Exception.ExceptionRecord;
 kd_init_state_change(cpu, sc);

 stl_p(>NewState, DbgKdExceptionStateChange);
 stl_p(>ExceptionCode, 0x8003);
 sttul_p(>ExceptionAddress, env->eip);
-
-return buf;
 }

-SizedBuf kd_gen_load_symbols_sc(CPUState *cpu)
+void kd_gen_load_symbols_sc(CPUState *cpu,
DBGKD_ANY_WAIT_STATE_CHANGE *sc)
 {
-DBGKD_ANY_WAIT_STATE_CHANGE *sc;
-SizedBuf buf;
-
-buf.size = sizeof(DBGKD_ANY_WAIT_STATE_CHANGE);
-buf.data = g_malloc0(buf.size);
-sc = (DBGKD_ANY_WAIT_STATE_CHANGE *) buf.data;
 kd_init_state_change(cpu, sc);

 stl_p(>NewState, DbgKdLoadSymbolsStateChange);
 stl_p(>u.LoadSymbols.PathNameLength, 0);
-
-return buf;
 }

 #endif
diff --git a/windbgstub.c b/windbgstub.c
index a458b962de..d8712af103 100755
--- a/windbgstub.c
+++ b/windbgstub.c
@@ -117,9 +117,15 @@ static void windbg_send_control_packet(uint16_t
type)

 static void windbg_bp_handler(CPUState *cpu)
 {
-SizedBuf buf = kd_gen_exception_sc(cpu);
-windbg_send_data_packet(buf.data, buf.size,
PACKET_TYPE_KD_STATE_CHANGE64);
-g_free(buf.data);
+struct {
+DBGKD_ANY_WAIT_STATE_CHANGE sc;
+int extra;
+} sc_with_extra;
+
+memset(_with_extra, 0, sizeof(sc_with_extra));
+kd_gen_exception_sc(cpu, _with_extra.sc);
+windbg_send_data_packet((uint8_t *)_with_extra,
sizeof(sc_with_extra),
+PACKET_TYPE_KD_STATE_CHANGE64);
 }

 static void windbg_vm_stop(void)
@@ -264,6 +270,8 @@ static void

[Qemu-devel] [PATCH v4 05/46] windbg: added helper features

[Mihail Abakumov]

Added some helper features for windbgstub.

Signed-off-by: Mihail Abakumov 
Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Dmitriy Koltunov 
---
 include/exec/windbgstub-utils.h |   31 +++
 include/exec/windbgstub.h   |6 ++
 2 files changed, 37 insertions(+)

diff --git a/include/exec/windbgstub-utils.h b/include/exec/windbgstub-utils.h
index 2390597f1f..4c747fa23e 100755
--- a/include/exec/windbgstub-utils.h
+++ b/include/exec/windbgstub-utils.h
@@ -13,7 +13,38 @@
 #define WINDBGSTUB_UTILS_H
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "log.h"
+#include "cpu.h"
 #include "exec/windbgstub.h"
 #include "exec/windbgkd.h"
 
+#define WINDBG_DEBUG(...) do { \
+if (WINDBG_DEBUG_ON) { \
+qemu_log(WINDBG ": " __VA_ARGS__); \
+qemu_log("\n");\
+}  \
+} while (false)
+
+#define WINDBG_ERROR(...) error_report(WINDBG ": " __VA_ARGS__)
+
+#define FMT_ADDR "addr:0x" TARGET_FMT_lx
+#define FMT_ERR  "Error:%d"
+
+#define UINT8_P(ptr) ((uint8_t *) (ptr))
+#define PTR(var) UINT8_P()
+
+#define sizeof_field(type, field) sizeof(((type *) NULL)->field)
+
+#define READ_VMEM(cpu, addr, type) ({ \
+type _t;  \
+cpu_memory_rw_debug(cpu, addr, PTR(_t), sizeof(type), 0); \
+_t;   \
+})
+
+typedef struct SizedBuf {
+uint8_t *data;
+size_t size;
+} SizedBuf;
+
 #endif
diff --git a/include/exec/windbgstub.h b/include/exec/windbgstub.h
index 1a6e1cc6e5..21bc552e58 100755
--- a/include/exec/windbgstub.h
+++ b/include/exec/windbgstub.h
@@ -12,6 +12,12 @@
 #ifndef WINDBGSTUB_H
 #define WINDBGSTUB_H
 
+#define WINDBG "windbg"
+
+#ifndef WINDBG_DEBUG_ON
+#define WINDBG_DEBUG_ON false
+#endif
+
 int windbg_server_start(const char *device);
 
 #endif