Context size is invalid (0x66f) #243
Comments
|
This is a problem we had already run into with Breakpad when dealing with new minidumps that use XSTATE data. See bug 1683232 on our tracker. The context size is variable depending on the combination of state that gets saved and so cannot be identified by its size. Additionally the context doesn't have the CPU flags immediately available and so the CPU type can't be directly extracted from there. I fixed it by patching breakpad this way: in a nutshell we take the CPU type from the system info stream (if present) and assume the context matches it. The only case when this can fail is if the system info stream is not present (which means the minidump would lack some pretty critical information) or if the CPU type of the threads' context is different from the CPU type of the minidump which would be odd... It shouldn't be hard to apply the same change here. |
|
One additional note: in my Breakpad patch I simply skipped over the XSTATE info because we didn't want to spend too much time on it. Here it's a different matter, we'd like to properly parse and include the XSTATE in the context. |
|
Is XSTATE is regular context + AVX? If yes, and we know it, then what else we want to know? |
... InitializeContext(0, CONTEXT_ALL | CONTEXT_XSTATE, 0, &len);returns exactly |
|
Some more scratches, at least we know something... DWORD64 FeatureMask;
DWORD FeatureLength;
ULONG Index;
PM128A Xmm;
PM128A Ymm;
CONTEXT* ctx;
BOOL r;
DWORD len = 0;
void* buf = 0;
r = InitializeContext(0, CONTEXT_ALL | CONTEXT_XSTATE, 0, &len);
printf("init1: %d %x\n", r, len);
buf = malloc(len);
memset(buf, 0, len);
printf("buf: %p\n", buf);
r = InitializeContext(buf, CONTEXT_ALL | CONTEXT_XSTATE, &ctx, &len);
printf("init2: %d %x\n", r, len);
r = SetXStateFeaturesMask(ctx, XSTATE_MASK_AVX);
printf("set: %d\n", r);
ctx->ContextFlags = CONTEXT_ALL | CONTEXT_XSTATE;
r = GetThreadContext(GetCurrentThread(), ctx);
printf("getthread: %d\n", r);
r = GetXStateFeaturesMask(ctx, &FeatureMask);
printf("getfeatures: %d (XSTATE_MASK_AVX = %x) %x\n", r, XSTATE_MASK_AVX, FeatureMask);
Xmm = (PM128A)LocateXStateFeature(ctx, XSTATE_LEGACY_SSE, &FeatureLength);
Ymm = (PM128A)LocateXStateFeature(ctx, XSTATE_AVX, NULL);
printf("FeatureLength = %x\n", FeatureLength);
printf("CONTEXT offset: %x\n", (size_t)ctx - (size_t)buf);
printf("Xmm offset = 0x%x\n", (size_t)Xmm - (size_t)buf);
printf("Ymm offset = 0x%x\n", (size_t)Ymm - (size_t)buf);
for (Index = 0; Index < FeatureLength / sizeof(Xmm[0]); Index += 1)
{
printf("Ymm%d: %I64d %I64d %I64d %I64d\n",
Index,
Xmm[Index].Low,
Xmm[Index].High,
Ymm[Index].Low,
Ymm[Index].High);
}
Output: raw memory: upd: |
|
So, the _CONTEXT starts with 0 offset |
|
@gabrielesvelto can I be of some more help? |
|
Thank you, that's more than enough info. We know already how the context is stored because we parse the XSAVE context information. We just need to pass the CPU type we've parsed from the system info stream to the context reader here and here and then use that to parse the context, instead of the context's size. |
|
Looking into this, sorry for the delay. Took me a while to get all the machinery in the right place. I'm a bit perplexed by the values that XSTATE is giving me for your minidump. Some debug logs I added to MinidumpContext::read: I don't see any way that the values in xstate can be combined to form the expected AMD64 size. Although it's been so long since I've thought about this stuff that maybe I just totally forget what these mean. In the comments I left I assert that all this XSTATE stuff is just "after the context" so I would have assumed that I could do some subtraction or something to get the right size. But the xstate leaves only ~800 bytes left while we expect ~1200 bytes for AMD64. Just blindly parsing from offset 0 produces this, which, idk seems vaguely reasonable? |
|
I'm assuming there's overlap between the "legacy region" and what we normally parse as context, but these diagrams seem pretty hard to reconcile with that theory, unless
|
|
Also it's just weird that the xstate header says that is has exactly the size of the buffer and everything starts at offset 0, but ostensibly the "hack fix" is to just assume the normal amd64 context starts at offset 0? |
|
(It's not impossible that our xsave parsing is totally messed up BUT the values it produces seem VERY plausible...) |
|
For comparison, the AMD64 context we use: /// An x86-64 (amd64) CPU context
///
/// This struct matches the definition of `CONTEXT` in WinNT.h for x86-64.
#[derive(Debug, SmartDefault, Clone, Pread, SizeWith)]
pub struct CONTEXT_AMD64 {
pub p1_home: u64,
pub p2_home: u64,
pub p3_home: u64,
pub p4_home: u64,
pub p5_home: u64,
pub p6_home: u64,
pub context_flags: u32,
pub mx_csr: u32,
pub cs: u16,
pub ds: u16,
pub es: u16,
pub fs: u16,
pub gs: u16,
pub ss: u16,
pub eflags: u32,
pub dr0: u64,
pub dr1: u64,
pub dr2: u64,
pub dr3: u64,
pub dr6: u64,
pub dr7: u64,
pub rax: u64,
pub rcx: u64,
pub rdx: u64,
pub rbx: u64,
pub rsp: u64,
pub rbp: u64,
pub rsi: u64,
pub rdi: u64,
pub r8: u64,
pub r9: u64,
pub r10: u64,
pub r11: u64,
pub r12: u64,
pub r13: u64,
pub r14: u64,
pub r15: u64,
pub rip: u64,
/// Floating point state
///
/// This is defined as a union in the C headers, but also
/// ` MAXIMUM_SUPPORTED_EXTENSION` is defined as 512 bytes.
///
/// Callers that want to access the underlying data can use [`Pread`] to read either
/// an [`XMM_SAVE_AREA32`] or [`SSE_REGISTERS`] struct from this raw data as appropriate.
#[default([0; 512])]
pub float_save: [u8; 512],
#[default([0; 26])]
pub vector_register: [u128; 26],
pub vector_control: u64,
pub debug_control: u64,
pub last_branch_to_rip: u64,
pub last_branch_from_rip: u64,
pub last_exception_to_rip: u64,
pub last_exception_from_rip: u64,
}In this struct, This still leaves ~160 unaccounted for bytes in the buffer size (which is the size of LEGACY_FLOATING_POINT? HMM???) |
|
Looking closer at our definition of XMM_SAVE_AREA_32: pub struct XMM_SAVE_AREA32 {
pub control_word: u16,
pub status_word: u16,
pub tag_word: u8,
pub reserved1: u8,
pub error_opcode: u16,
pub error_offset: u32,
pub error_selector: u16,
pub reserved2: u16,
pub data_offset: u32,
pub data_selector: u16,
pub reserved3: u16,
pub mx_csr: u32,
pub mx_csr_mask: u32,
pub float_registers: [u128; 8],
pub xmm_registers: [u128; 16],
pub reserved4: [u8; 96],
}That looks exactly like the Intel diagram. |
|
Ok so after mulling this over my guess is:
I've sent an email to the microsoft minidump tooling folks asking what's up with all of this. |
|
email from microsoft seems to largely confirm all of the above! |
|
I'm glad they confirmed that we can still rely on the old stuff because we've processed minidumps for a while "assuming" that it would be that way. We can delay parsing the extra state to a follow-up bug and just go for the straightforward fix here which IMHO captures 99% of the functionality we care about. |
|
Fixed by #289 |
Sup!
It looks like the
thread_context.data_sizeis wrong for some reason, thus it's impossible to decide the architecture and parse the context registers (( no context)note).Attaching the .dmp file: fuzzme1.windbg.zip
The code used:
Thank you!
The text was updated successfully, but these errors were encountered: