[PR,18702] Fix wrong output of x87 registers due to truncation to double on amd64

  When `info float` is used on an AMD64 system, GDB prints floating-point
values of x87 registers with raw contents like 0x361a867a8e0527397ce0 or
0xc4f988454a1ddd3cfdab wrongly. This happens due to truncation to double,
after which the former becomes 0.0, and the latter becomes negative infinity.
This is caused by failed detection of x86-64 host, which results in setting
gdb_host_{float,double,long_double}_format to zeros.
This commit fixes this misdetection.

gdb/ChangeLog:

	* configure.host: Fix detection of x86_64 host when setting floatformats

---
 gdb/configure.host |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Ruslan Kabatsayev <b7.10110111@gmail.com> writes:

Hi Ruslan,

> When `info float` is used on an AMD64 system, GDB prints floating-point
> values of x87 registers with raw contents like 0x361a867a8e0527397ce0 or
> 0xc4f988454a1ddd3cfdab wrongly. This happens due to truncation to double,
> after which the former becomes 0.0, and the latter becomes negative infinity.
> This is caused by failed detection of x86-64 host, which results in setting
> gdb_host_{float,double,long_double}_format to zeros.
> This commit fixes this misdetection.

I think your patch is correct, but I am not confident approving it
because I know few about floating point stuff.  Do you run GDB
regression tests?

>
> gdb/ChangeLog:
>
> 	* configure.host: Fix detection of x86_64 host when setting floatformats

This line is too long, the max is 74.  Sentence should be ended with ".".
  

Hi Yao,

Thanks for your reply.

On Wed, Dec 2, 2015 at 2:32 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
> I think your patch is correct, but I am not confident approving it
> because I know few about floating point stuff.  Do you run GDB
> regression tests?

I did a `make check` for unpatched gdb-7.10 and patched with my
changes. Here's the unpatched result:

# of expected passes            32377
# of unexpected failures        85
# of unexpected successes       2
# of expected failures          72
# of unknown successes          2
# of known failures             62
# of untested testcases         36
# of unsupported tests          201

And here's after my patch:

# of expected passes            32378
# of unexpected failures        85
# of unexpected successes       2
# of expected failures          71
# of unknown successes          2
# of known failures             62
# of untested testcases         36
# of unsupported tests          201

I.e. one "expected failure" less and one "expected pass" more. I'm not
sure how to interpret this result.

>> gdb/ChangeLog:
>>
>>       * configure.host: Fix detection of x86_64 host when setting floatformats
>
> This line is too long, the max is 74.  Sentence should be ended with ".".

Should I resend the whole original mail with this fixed?

Regards,
Ruslan
  

Ruslan Kabatsayev <b7.10110111@gmail.com> writes:

> I did a `make check` for unpatched gdb-7.10 and patched with my
> changes. Here's the unpatched result:

Thanks for running the test.  Could you test your patch against GDB git
mainline please?  The patch will be applied to mainline, so we need to
test it against mainline.

>
> # of expected passes            32377
> # of unexpected failures        85
> # of unexpected successes       2
> # of expected failures          72
> # of unknown successes          2
> # of known failures             62
> # of untested testcases         36
> # of unsupported tests          201
>
> And here's after my patch:
>
> # of expected passes            32378
> # of unexpected failures        85
> # of unexpected successes       2
> # of expected failures          71
> # of unknown successes          2
> # of known failures             62
> # of untested testcases         36
> # of unsupported tests          201
>
> I.e. one "expected failure" less and one "expected pass" more. I'm not
> sure how to interpret this result.
>

This may happen from time to time, especially in some gdb.threads tests.

>>> gdb/ChangeLog:
>>>
>>>       * configure.host: Fix detection of x86_64 host when setting
>>> floatformats
>>
>> This line is too long, the max is 74.  Sentence should be ended with ".".
>
> Should I resend the whole original mail with this fixed?

Please post the changelog entry only for the reference.

If there is no regression in the GDB mainline tests, and no objections
from other people in 3 days, your patch can go in.

Do you have write access to git repository and FSF copyright assignment?
If you don't have them, I can commit it for you as a tiny patch.
However, if you want to contribute more patches, you need an FSF
copyright assignment, and git write access, so that you can push by yourself.
  

On 11/20/2015 10:07 PM, Ruslan Kabatsayev wrote:
> When `info float` is used on an AMD64 system, GDB prints floating-point
> values of x87 registers with raw contents like 0x361a867a8e0527397ce0 or
> 0xc4f988454a1ddd3cfdab wrongly. This happens due to truncation to double,
> after which the former becomes 0.0, and the latter becomes negative infinity.
> This is caused by failed detection of x86-64 host, which results in setting
> gdb_host_{float,double,long_double}_format to zeros.
> This commit fixes this misdetection.

A test to make sure we don't regress this would be nice.
Maybe add something to gdb.arch/i386-float.exp ?

Thanks,
Pedro Alves
  

On 12/03/2015 02:27 PM, Yao Qi wrote:

> Thanks for running the test.  Could you test your patch against GDB git
> mainline please?  The patch will be applied to mainline, so we need to
> test it against mainline.

With current git master the result is the same: no noticeable changes between patched and unpatched version.

>> # of expected passes            32377
>> # of unexpected failures        85
>> # of unexpected successes       2
>> # of expected failures          72
>> # of unknown successes          2
>> # of known failures             62
>> # of untested testcases         36
>> # of unsupported tests          201
>>
>> And here's after my patch:
>>
>> # of expected passes            32378
>> # of unexpected failures        85
>> # of unexpected successes       2
>> # of expected failures          71
>> # of unknown successes          2
>> # of known failures             62
>> # of untested testcases         36
>> # of unsupported tests          201
>>
>> I.e. one "expected failure" less and one "expected pass" more. I'm not
>> sure how to interpret this result.
>>
> This may happen from time to time, especially in some gdb.threads tests.

For some reason such things happen with different individual tests almost on 
every `make check` here, be it patched or unpatched version.

>>>> gdb/ChangeLog:
>>>>
>>>>       * configure.host: Fix detection of x86_64 host when setting
>>>> floatformats
>>> This line is too long, the max is 74.  Sentence should be ended with ".".
>> Should I resend the whole original mail with this fixed?
> Please post the changelog entry only for the reference.

gdb/ChangeLog:

	* configure.host: Fix detection of x86_64 host when setting
	floatformats.

> If there is no regression in the GDB mainline tests, and no objections
> from other people in 3 days, your patch can go in.
>
> Do you have write access to git repository and FSF copyright assignment?
> If you don't have them, I can commit it for you as a tiny patch.
> However, if you want to contribute more patches, you need an FSF
> copyright assignment, and git write access, so that you can push by yourself.

I think I don't. How do I get them?
As for more patches, I'm going to make a regression test for this fix, as 
suggested by Pedro Alves in another email.
  

On Thu, Dec 3, 2015 at 3:28 PM, Pedro Alves <palves@redhat.com> wrote:
> A test to make sure we don't regress this would be nice.
> Maybe add something to gdb.arch/i386-float.exp ?

I think I'll do this. Should this go as a separate patch?

>
> Thanks,
> Pedro Alves
>
  

On 12/04/2015 03:06 PM, Ruslan Kabatsayev wrote:
> On Thu, Dec 3, 2015 at 3:28 PM, Pedro Alves <palves@redhat.com> wrote:
>> A test to make sure we don't regress this would be nice.
>> Maybe add something to gdb.arch/i386-float.exp ?
> 
> I think I'll do this. Should this go as a separate patch?

Ideally it'd go in the same patch.

Thanks,
Pedro Alves
  

On Fri, Dec 4, 2015 at 6:11 PM, Pedro Alves <palves@redhat.com> wrote:
> On 12/04/2015 03:06 PM, Ruslan Kabatsayev wrote:
>> On Thu, Dec 3, 2015 at 3:28 PM, Pedro Alves <palves@redhat.com> wrote:
>>> A test to make sure we don't regress this would be nice.
>>> Maybe add something to gdb.arch/i386-float.exp ?
>>
>> I think I'll do this. Should this go as a separate patch?
>
> Ideally it'd go in the same patch.
Oh, OK. What about email subject? Can I just send it as a follow-up to
this conversation, or does it have to be an independent email with
subject="[PATCH]..."? Or is there no difference?
(Sorry for silly questions, I'm just trying to fulfill all the
requirements at the first try :) )

>
> Thanks,
> Pedro Alves
>
  

On 12/04/2015 03:16 PM, Ruslan Kabatsayev wrote:
> On Fri, Dec 4, 2015 at 6:11 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 12/04/2015 03:06 PM, Ruslan Kabatsayev wrote:
>>> On Thu, Dec 3, 2015 at 3:28 PM, Pedro Alves <palves@redhat.com> wrote:
>>>> A test to make sure we don't regress this would be nice.
>>>> Maybe add something to gdb.arch/i386-float.exp ?
>>>
>>> I think I'll do this. Should this go as a separate patch?
>>
>> Ideally it'd go in the same patch.
> Oh, OK. What about email subject? Can I just send it as a follow-up to
> this conversation, or does it have to be an independent email with
> subject="[PATCH]..."? Or is there no difference?
> (Sorry for silly questions, I'm just trying to fulfill all the
> requirements at the first try :) )

As it's a single patch, reply is fine.  Repost as new thread
would be fine too.  Ideally tagged with "[PATCH v2]" in that case.
I suggest using "git send-email" to send patches.

Thanks,
Pedro Alves