Re: [PATCH v6 00/15] Handle variable XSAVE layouts

[John Baldwin <jhb@FreeBSD.org>]

On 7/27/23 2:36 PM, Keith Seitz wrote:
> On 7/26/23 15:31, John Baldwin wrote:
>>
>> So I have a couple of things to try here.  First, a simple fix for the crash
>> is this change (relative to the branch), but it does mean the core dumps generated
>> by gcore for a remote target will not contain extended XSAVE state like AVX, etc.:
>>
>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
>> index c8b467a0416..a7d61f33a08 100644
>> --- a/gdb/i386-linux-tdep.c
>> +++ b/gdb/i386-linux-tdep.c
>> @@ -768,7 +768,7 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>>
>>      cb (".reg", 68, 68, &i386_gregset, NULL, cb_data);
>>
>> -  if (tdep->xcr0 & X86_XSTATE_AVX)
>> +  if (tdep->xsave_layout.sizeof_xsave != 0)
>>        cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
>>           tdep->xsave_layout.sizeof_xsave, &i386_linux_xstateregset,
>>           "XSAVE extended state", cb_data);
>>
>> Arguably that is more correct as it lines up with the way the rest of the x86 arches
>> in this series.
> 
> Yeah, that does fix a lot of problems. In the vein of "perfect is the enemy of good,"
> this LGTM.

Ok.

>> The other change I have locally is one to generate a "fallback" layout to use in
>> this case that a target (such as remote) doesn't provide a layout.
> 
>>
>> The fallback change:
> [snip]
> 
> This causes a new regression in gdb.base/solib-display.exp:
> 
> # make check RUNTESTFLAGS="--target_board native-extended-gdbserver.exp" \
>     TESTS=gdb.base/solib-display.exp
> Running /root/fsf/linux/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/solib-display.exp ...
> FAIL: gdb.base/solib-display.exp: NO: continue two
> FAIL: gdb.base/solib-display.exp: IN: continue two
> FAIL: gdb.base/solib-display.exp: SEP: continue two
> 
> 		=== gdb Summary ===
> 
> # of expected passes		32
> # of unexpected failures	3
> 
> The issue is the same with all three failing tests (-m32/-m64 don't matter).
> With this tweak, we now see:
> 
> warning: Unable to display "a_local": No symbol "a_local" in current context.
> warning: Unable to display "a_static": No symbol "a_static" in current context.
> 
> This disables the display of the two variables, and they are omitted from
> subsequent displays. [This happens on every x86_64 on which I've tested: Comet Lake,
> Raptor Lake, and Sapphire Rapids.]

Humm, ok.  I should be able to look at this locally in a x86-64 VM I have.
I'm inclined to probably leave this off the series though for now while I
figure out the right solution to gcore of a remote process.

-- 
John Baldwin