Re: Oh dear. I regret to inform you that commit de52b9607d2623f18b7a7dbee3e1123d8d63f5da might be unfortunate

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, June 29 2018, Joel Brobecker wrote:

>> I'm still seeing a breakage here on x86-64 Fedora GNU/Linux, when
>> building gdbserver:
>> 
>>   ../../../binutils-gdb/gdb/gdbserver/linux-x86-tdesc.c: In function ‘const target_desc* amd64_linux_read_description(uint64_t, bool)’:
>>   ../../../binutils-gdb/gdb/gdbserver/linux-x86-tdesc.c:121:67: error: too few arguments to function ‘target_desc* amd64_create_target_description(uint64_t, bool, bool, bool)’
>>          *tdesc = amd64_create_target_description (xcr0, is_x32, true);
>>                                                                      ^
>>   In file included from ../../../binutils-gdb/gdb/gdbserver/linux-x86-tdesc.c:26:0:
>>   ../../../binutils-gdb/gdb/gdbserver/../arch/amd64.h:21:14: note: declared here
>>    target_desc *amd64_create_target_description (uint64_t xcr0, bool is_x32,
>>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Does the patch below make sense?  It fixes the breakage for me.
>
> Thanks for looking into it and the patch!
>
> I think the parameter should be set to "true". Otherwise, it will
> not include the fs_base and gs_base register in the list of registers.
> Although the name of the source file says x86, the code itself is
> protected by...
>
>     #ifdef __x86_64__
>
> ... and is inside a function called amd64_linux_read_description.
> I also verified that this file gets compiled on amd64-linux platforms.
> See gdb/gdbserver/configure.srv:
>
>   x86_64-*-linux*)      srv_regobj="$srv_amd64_linux_regobj $srv_i386_linux_regobj"
>
> The last piece of confirmation is that setting the parameter to "true"
> provides the behavior before the parameter was added; and the reason
> for adding the parameter was to remove the {fs,gs}_base registers
> from the list for Windows only.
>
> Otherwise the rest (basically the ChangeLog) looks good.

Ah, thanks for the explanation.  I should have investigated more before
sending the patch; I was doing something else and just wanted to make
GDB compile again.  Glad I sent it here for your review!

> Thanks again!

No, I thank you!

I pushed the patch with s/false/true/, and your name in the ChangeLog.

>> diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
>> index 2fd2d90a49..743d465d81 100644
>> --- a/gdb/gdbserver/ChangeLog
>> +++ b/gdb/gdbserver/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2018-06-29  Sergio Durigan Junior  <sergiodj@redhat.com>
>> +
>> +	* linux-x86-tdesc.c (amd64_linux_read_description): Add missing
>> +	parameter in call to 'amd64_create_target_description'.
>> +
>>  2018-06-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
>>  
>>  	* x86-tdesc.h: Remove executable permission flag.
>> diff --git a/gdb/gdbserver/linux-x86-tdesc.c b/gdb/gdbserver/linux-x86-tdesc.c
>> index 358659b295..7a45953dc9 100644
>> --- a/gdb/gdbserver/linux-x86-tdesc.c
>> +++ b/gdb/gdbserver/linux-x86-tdesc.c
>> @@ -118,7 +118,7 @@ amd64_linux_read_description (uint64_t xcr0, bool is_x32)
>>  
>>    if (*tdesc == NULL)
>>      {
>> -      *tdesc = amd64_create_target_description (xcr0, is_x32, true);
>> +      *tdesc = amd64_create_target_description (xcr0, is_x32, true, false);
>>  
>>        init_target_desc (*tdesc, amd64_expedite_regs);
>>      }
>
> -- 
> Joel

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/