Re: [PATCH][gdb/testsuite] Fix gdb.ada/float-bits.exp with -m32

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 5/3/22 11:54, Tom de Vries wrote:
> On 5/3/22 08:47, Luis Machado wrote:
>> On 5/2/22 08:10, Tom de Vries wrote:
>>> On 4/25/22 12:31, Luis Machado wrote:
>>>> On 4/14/22 14:14, Tom de Vries via Gdb-patches wrote:
>>>>> Hi,
>>>>>
>>>>> With test-case gdb.ada/float-bits.exp and native we get:
>>>>> ...
>>>>> (gdb) print 16llf#7FFFF7FF4054A56FA5B99019A5C8#^M
>>>>> $9 = 5.0e+25^M
>>>>> (gdb) PASS: gdb.ada/float-bits.exp: print 
>>>>> 16llf#7FFFF7FF4054A56FA5B99019A5C8#
>>>>> ...
>>>>> but with target board unix/-m32 we have instead:
>>>>> ...
>>>>> (gdb) print 16llf#7FFFF7FF4054A56FA5B99019A5C8#^M
>>>>> Cannot export value 2596145952482202326224873165792712 as 96-bits \
>>>>>    unsigned integer (must be between 0 and 
>>>>> 79228162514264337593543950335)^M
>>>>> (gdb) FAIL: gdb.ada/float-bits.exp: print 
>>>>> 16llf#7FFFF7FF4054A56FA5B99019A5C8#
>>>>> ...
>>>>>
>>>>> Fix this by testing whether 16llf is supported by doing ptype 
>>>>> long_long_float
>>>>> which gets us either:
>>>>> ...
>>>>> type = <16-byte float>^M
>>>>> ...
>>>>> or:
>>>>> ...
>>>>> type = <12-byte float>^M
>>>>> ...
>>>>>
>>>>> Tested on x86_64-linux with native and unix/-m32.
>>>>
>>>> Unfortunately not all targets support 128-bit long doubles. For arm 
>>>> and aarch64 the compiler won't generate a 128-bit float, but a 
>>>> 64-bit float, so the 16ll tests won't be meaningful.
>>>>
>>>
>>> Right, but I'd expect those tests are skipped because 16llf_supported 
>>> is 0 for 64-bit long double.
>>
>> They are skipped, but the testcase still assumes some 16llf tests can 
>> be executed:
>>
>> gdb_test "print val_long_double" " = 5.0e\\+25"
>>
>> gdb_test "print val_long_double" " = 5.0e\\+25" \
>>      "print val_long_double after assignment"
>>
> 
> Ah, I see, you mean the val_long_double tests that do not use 16llf.
> 
>> The above couple tests won't work correctly, as you're trying to force 
>> a 128-bit value into a 64-bit variable.
>>>
>>>> FAIL: gdb.ada/float-bits.exp: print val_long_double
>>>> FAIL: gdb.ada/float-bits.exp: print val_long_double after assignment
>>>>
>>>
>>> Can you show me the actual failure mode, that is, copy from gdb.log 
>>> instead of gdb.sum?  I'm surprised that these fail because AFAICT, 
>>> the used constant: 5.0e+25 is exactly representable in 64-bit ieee ( 
>>> I used https://babbage.cs.qc.cuny.edu/ieee-754.old/decimal.html to 
>>> check this ).
>>
>> Sure, here it is:
>>
>> print val_long_double
>> $9 = 5.0000000000000002e+25
>> (gdb) FAIL: gdb.ada/float-bits.exp: print val_long_double
>> print val_long_double
>> $10 = 5.0000000000000002e+25
>> (gdb) FAIL: gdb.ada/float-bits.exp: print val_long_double after 
>> assignment
>>
>>>
> 
> I see.  I can reproduce this using reproducer patch posted below, which 
> gives me:
> ...
> (gdb) print val_double_2^M
> $9 = 5.0000000000000002e+25^M
> (gdb) print /x val_double_2^M
> $10 = 0x4544adf4b7320335^M
> (gdb)
> ...
> 
> So, I realize now that my statement about it being exactly representable 
> was incorrect, it's represented as 2^85 * 1.2924697071141058.  I was 
> thrown off by the website presenting me a crisp '5.0000000000000000e+25' 
> as the value, but that's just one way to print it, and glibc evidently 
> uses a different rounding in printing, which is allowed AFAICT, as per 
> 'The low-order digit shall be rounded in an implementation-defined manner'.
> 
> So I suppose we could just accept
>    " = (5.0e\+25|5.000000000000000\de\+25)"
> for 64-bits long double.
> 
>>>> I wonder if it would be best to bail out as soon as we find out the 
>>>> target has no support for 128-bit floats. I can write a patch for that.
>>>>
>>>
>>> With a rewrite like this:
>>> ...
>>> -set 16llf_supported 0
>>> +set long_double_bytes 0
>>>   gdb_test_multiple "ptype long_long_float" "" {
>>> -    -re -wrap "<16-byte float>" {
>>> -       set 16llf_supported 1
>>> -       pass $gdb_test_name
>>> -    }
>>> -    -re -wrap "<\\d+-byte float>" {
>>> -       pass $gdb_test_name
>>> +    -re -wrap "<\(\\d+\)-byte float>" {
>>> +       set long_double_bytes $expect_out(1,string)
>>>       }
>>>   }
>>>
>>> +set 16llf_supported [expr $long_double_bytes >= 16]
>>> ...
>>> we can formulate the precondition for any test in terms number of 
>>> long double bytes.
>>
>> I've sent a patch to make this test a bit more generic: 
>> https://sourceware.org/pipermail/gdb-patches/2022-April/188440.html
>>
>> Did you see it?
>>
> 
> I did not, I've just come back from vacation and am still working 
> through my inbox (and you didn't post the patch in reply to this thread, 
> or cc-ed me, or updated this thread with a reference-to-post, any of 
> which would mean I would have some pointer in my inbox) .  I'll take a 
> look.
> 

No worries. I assumed so. It's just that I decided to go ahead and 
improve the test somewhat, and the improvement seemed beyond what this 
patch touched.