Re: [PATCH v4 07/13] gdb: Update x86 Linux architectures to support XSAVE layouts.

[John Baldwin <jhb@FreeBSD.org>]

On 4/6/23 6:43 PM, Simon Marchi wrote:
>> -/* Get XSAVE extended state xcr0 from core dump.  */
>> +/* See i386-linux-tdep.h.  */
>>   
>>   uint64_t
>> -i386_linux_core_read_xcr0 (bfd *abfd)
>> +i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout *layout)
>>   {
>>     asection *xstate = bfd_get_section_by_name (abfd, ".reg-xstate");
>> -  uint64_t xcr0;
>> +  if (xstate == nullptr)
>> +    return X86_XSTATE_SSE_MASK;
>>   
>> -  if (xstate)
>> +  /* Check extended state size.  */
>> +  size_t size = bfd_section_size (xstate);
>> +  if (size < X86_XSTATE_AVX_SIZE)
>> +    return X86_XSTATE_SSE_MASK;
>> +
>> +  char contents[8];
>> +  if (! bfd_get_section_contents (abfd, xstate, contents,
>> +				  I386_LINUX_XSAVE_XCR0_OFFSET, 8))
>>       {
>> -      size_t size = bfd_section_size (xstate);
>> -
>> -      /* Check extended state size.  */
>> -      if (size < X86_XSTATE_AVX_SIZE)
>> -	xcr0 = X86_XSTATE_SSE_MASK;
>> -      else
>> -	{
>> -	  char contents[8];
>> -
>> -	  if (! bfd_get_section_contents (abfd, xstate, contents,
>> -					  I386_LINUX_XSAVE_XCR0_OFFSET,
>> -					  8))
>> -	    {
>> -	      warning (_("Couldn't read `xcr0' bytes from "
>> -			 "`.reg-xstate' section in core file."));
>> -	      return 0;
>> -	    }
>> -
>> -	  xcr0 = bfd_get_64 (abfd, contents);
>> -	}
>> +      warning (_("Couldn't read `xcr0' bytes from "
>> +		 "`.reg-xstate' section in core file."));
>> +      return 0;
>>       }
>> -  else
>> -    xcr0 = 0;
>> +
>> +  uint64_t xcr0 = bfd_get_64 (abfd, contents);
>> +
>> +  if (!i387_set_xsave_layout (xcr0, size, layout))
>> +    return 0;
> 
> Should these 3 "return 0" be "return X86_XSTATE_SSE_MASK"?

Yes, fixed.
   
>>     return xcr0;
>>   }
>>   
>> +/* Implement the core_xfer_x86_xsave_layout gdbarch method.  */
>> +
>> +LONGEST
>> +i386_linux_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
>> +				       gdb_byte *readbuf, ULONGEST offset,
>> +				       ULONGEST len)
> 
> I think the comment should be
> 
> /* See i386-linux-tdep.h.  */

Fixed.

-- 
John Baldwin