Re: [PATCH v2 3/3] elf2dmp: fill ContextBuffer with faulting CPU context

On 2026/08/05 18:05, Zhengrong Li wrote:
> When is_crash_occurred_cpu is set in QEMUCPUState, copy that CPU's
> context into WinDumpHeader64 ContextBuffer so WinDbg opens on the
> faulting CPU instead of CPU 0.
> 
> Also extract KiBugCheckData from the guest kernel to populate
> BugcheckCode and BugcheckParameters in the dump header.
> 
> Changes since v1:
>    - Add size-based field detection in init_states() for forward compatibility
>    - Use has_is_crash_occurred_cpu flag instead of version check
>    - Set header context before best-effort guest memory operations to avoid
>      losing faulting CPU context on partial dump failures
>    - Use LIVE_SYSTEM_DUMP when BugcheckCode is zero
>    - Use PRIx32 for BugcheckCode format specifier
>    - Use linux.alibaba.com identity
> 
> Signed-off-by: Zhengrong Li <[email protected]>
> ---
>   contrib/elf2dmp/kdbg.h     |  9 +++++++
>   contrib/elf2dmp/main.c     | 50 ++++++++++++++++++++++++++++++++++++--
>   contrib/elf2dmp/qemu_elf.c | 20 +++++++++------
>   contrib/elf2dmp/qemu_elf.h |  1 +
>   4 files changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/contrib/elf2dmp/kdbg.h b/contrib/elf2dmp/kdbg.h
> index 002e3d0cd5..15e4fb1140 100644
> --- a/contrib/elf2dmp/kdbg.h
> +++ b/contrib/elf2dmp/kdbg.h
> @@ -195,4 +195,13 @@ typedef struct KDDEBUGGER_DATA64 {
>       uint16_t OffsetPrcbContext;
>   } KDDEBUGGER_DATA64;
>   
> +typedef struct KIBUGCHECK_INFO {
> +    uint32_t BugcheckCode;
> +    uint32_t unused0;
> +    uint64_t BugcheckParameter1;
> +    uint64_t BugcheckParameter2;
> +    uint64_t BugcheckParameter3;
> +    uint64_t BugcheckParameter4;
> +} KIBUGCHECK_INFO;
> +
>   #endif /* KDBG_H */
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index a62abadcc0..59ffc0f2e8 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -338,11 +338,26 @@ static bool fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
>    * A dump may still contain valuable information even if it lacks contexts of
>    * some CPUs due to dump corruption or a failure before starting CPUs.
>    */
> -static void fill_context(KDDEBUGGER_DATA64 *kdbg,
> +static void fill_context(WinDumpHeader64 *hdr, KDDEBUGGER_DATA64 *kdbg,
>                            struct va_space *vs, QEMU_Elf *qe)
>   {
>       int i;
> +    int fault_cpu = -1;
>   
> +    /* First pass: identify faulting CPU and set header context early */
> +    for (i = 0; i < qe->state_nr; i++) {
> +        QEMUCPUState *s = qe->state[i];
> +        if (qe->has_is_crash_occurred_cpu && s->is_crash_occurred_cpu) {
> +            WinContext64 ctx;
> +            win_context_init_from_qemu_cpu_state(&ctx, s);
> +            memcpy(hdr->ContextBuffer, &ctx, sizeof(ctx));
> +            fault_cpu = i;

fault_cpu is assigned but never read. Consequently:

     error: variable ‘fault_cpu’ set but not used
            [-Werror=unused-but-set-variable]

make -j8 contrib/elf2dmp/elf2dmp fails.

> +            printf("Faulting CPU identified: #%d\n", i);
> +            break;
> +        }
> +    }
> +
> +    /* Second pass: fill context for all CPUs (best-effort) */
>       for (i = 0; i < qe->state_nr; i++) {
>           uint64_t Prcb;
>           uint64_t Context;
> @@ -512,6 +527,7 @@ int main(int argc, char *argv[])
>       uint64_t KdVersionBlock;
>       bool kernel_found = false;
>       OMFSignatureRSDS rsds;
> +    uint64_t KiBugCheckData;
>   
>       if (argc != 3) {
>           eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
> @@ -611,7 +627,37 @@ int main(int argc, char *argv[])
>           goto out_kdbg;
>       }
>   
> -    fill_context(kdbg, &vs, &qemu_elf);
> +    if (!SYM_RESOLVE(KernBase, &pdb, KiBugCheckData)) {
> +        eprintf("Failed to get KiBugCheckData.\n");
> +    } else {
> +        KIBUGCHECK_INFO data = { 0 };
> +        if (va_space_rw(&vs, KiBugCheckData, &data, sizeof(data), 0)) {
> +            header.BugcheckCode = data.BugcheckCode;
> +            header.BugcheckParameter1 = data.BugcheckParameter1;
> +            header.BugcheckParameter2 = data.BugcheckParameter2;
> +            header.BugcheckParameter3 = data.BugcheckParameter3;
> +            header.BugcheckParameter4 = data.BugcheckParameter4;
> +
> +            /*
> +             * If BugcheckCode wasn't saved, we consider guest OS as alive.
> +             */
> +            if (!header.BugcheckCode) {
> +                header.BugcheckCode = LIVE_SYSTEM_DUMP;
> +            }
> +
> +            printf("KiBugCheckData: 0x%016" PRIx64
> +                   ", BugcheckCode: 0x%08" PRIx32 ", Args:"
> +                   " 0x%016" PRIx64 " 0x%016" PRIx64
> +                   " 0x%016" PRIx64 " 0x%016" PRIx64 "\n",
> +                    KiBugCheckData, header.BugcheckCode,
> +                    data.BugcheckParameter1, data.BugcheckParameter2,
> +                    data.BugcheckParameter3, data.BugcheckParameter4);
> +        } else {
> +            eprintf("Failed to va_space_rw KiBugCheckData.\n");
> +        }
> +    }
> +
> +    fill_context(&header, kdbg, &vs, &qemu_elf);
>   
>       if (!write_dump(&ps, &header, argv[2])) {
>           eprintf("Failed to save dump\n");
> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> index c9bad6e82c..8eb2654356 100644
> --- a/contrib/elf2dmp/qemu_elf.c
> +++ b/contrib/elf2dmp/qemu_elf.c
> @@ -71,7 +71,8 @@ static bool init_states(QEMU_Elf *qe)
>           return false;
>       }
>   
> -    qe->has_kernel_gs_base = 1;
> +    qe->has_kernel_gs_base = 0;
> +    qe->has_is_crash_occurred_cpu = 0;
>       offset = phdr[0].p_offset;
>       states = g_ptr_array_new();
>   
> @@ -103,14 +104,19 @@ static bool init_states(QEMU_Elf *qe)
>               nhdr->n_descsz >= offsetof(QEMUCPUState, kernel_gs_base)) {
>               state_size = MIN(state->size, nhdr->n_descsz);
>   
> -            if (state_size < sizeof(*state)) {
> +            if (state_size >= offsetof(QEMUCPUState, kernel_gs_base) +
> +                              sizeof(state->kernel_gs_base)) {
> +                qe->has_kernel_gs_base = 1;
> +            }
> +
> +            if (state_size >= offsetof(QEMUCPUState, is_crash_occurred_cpu) +
> +                              sizeof(state->is_crash_occurred_cpu)) {
> +                qe->has_is_crash_occurred_cpu = 1;
> +            }
> +
> +            if (state_size != sizeof(*state)) {
>                   eprintf("CPU #%u: QEMU CPU state size %u doesn't match\n",
>                           states->len, state_size);
> -                /*
> -                 * We assume either every QEMU CPU state has KERNEL_GS_BASE or
> -                 * no one has.
> -                 */
> -                qe->has_kernel_gs_base = 0;
>               }

Field availability is ORed across all CPU notes and then applied to 
every note. With a 440-byte CPU 0 note followed by a 448-byte CPU 1 
note, CPU 1 sets has_is_crash_occurred_cpu, after which fill_context() 
reads byte 440 beyond CPU 0’s descriptor. That byte normally belongs to 
the next note header and is nonzero, so CPU 0 can be falsely selected. 
The same aggregation can make fix_dtb() read a missing kernel_gs_base. 
Please follow what's previously done for kernel_gs_base.

Regards,
Akihiko Odaki

>               g_ptr_array_add(states, state);
>           }
> diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
> index 3b28332e67..120de4a2f4 100644
> --- a/contrib/elf2dmp/qemu_elf.h
> +++ b/contrib/elf2dmp/qemu_elf.h
> @@ -42,6 +42,7 @@ typedef struct QEMU_Elf {
>       QEMUCPUState **state;
>       size_t state_nr;
>       int has_kernel_gs_base;
> +    int has_is_crash_occurred_cpu;
>   } QEMU_Elf;
>   
>   bool QEMU_Elf_init(QEMU_Elf *qe, const char *filename);