Re: [PATCH][gdb/testsuite] Fix gdb.ada/literals.exp with aarch64

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

On 7/27/22 19:06, Joel Brobecker wrote:
> Hi Tom,
> 
>> On aarch64-linux, I run into:
>> ...
>> (gdb) print 16#ffffffffffffffff#^M
>> $7 = 18446744073709551615^M
>> (gdb) FAIL: gdb.ada/literals.exp: print 16#ffffffffffffffff#
>> ...
>> while on x86_64-linux instead, I get:
>> ...
>> (gdb) print 16#ffffffffffffffff#^M
>> $7 = -1^M
>> (gdb) PASS: gdb.ada/literals.exp: print 16#ffffffffffffffff#
>> ...
>>
>> We can easily reproduce this on x86_64-linux using:
>> ...
>> $ gdb -q -batch -ex "set lang ada" -ex "set arch i386" \
>>    -ex "print 16#ffffffffffffffff#"
>> $1 = -1
>> $ gdb -q -batch -ex "set lang ada" -ex "set arch aarch64" \
>>    -ex "print 16#ffffffffffffffff#"
>> $1 = 18446744073709551615
>> ...
>>
>> With i386, we have:
>> ...
>> (gdb) p int_bits
>> $3 = 32
>> (gdb) p long_bits
>> $4 = 32
>> (gdb) p long_long_bits
>> $5 = 64
>> ...
>> and so in processInt we hit the fits-in-unsigned-long-long case where we use
>> as type long long:
>> ...
>>        /* Note: Interprets ULLONG_MAX as -1.  */
>>        yylval.typed_val.type = type_long_long (par_state);
>> ...
>>
>> With aarch64, we have instead:
>> ...
>> (gdb) p int_bits
>> $1 = 32
>> (gdb) p long_bits
>> $2 = 64
>> (gdb) p long_long_bits
>> $3 = 64
>> ...
>> and so in processInt we hit the fits-in-unsigned-long case where we use
>> as type unsigned long:
>> ...
>>        yylval.typed_val.type
>>          = builtin_type (par_state->gdbarch ())->builtin_unsigned_long;
>> ...
>>
>> Fix this by updating the test-case to accept 18446744073709551615 as well.
>>
>> Tested on x86_64-linux and aarch64-linux.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29416
>>
>> Any comments?
> 
> Thanks for the patch.
> 
> My question is whether it actually makes sense that -1 be
> a valid output for the print command above. I tried the addition
> of ...
> 
>      | /* Note: Interprets ULLONG_MAX as -1.  */
>      | yylval.typed_val.type = type_long_long (par_state);
> 
> ... to a patch of yours:
> 
>      | commit ac3afe36d73c84096685fece885d70b28bc9629f
>      | Author: Tom de Vries <tdevries@suse.de>
>      | Date:   Sat Jun 4 13:17:33 2022 +0200
>      | Subject: [gdb/ada] Fix literal truncation
>      |
>      | Make sure we error out on overflow instead of truncating in all cases.
>      |
>      | Tested on x86_64-linux, with a build with --enable-targets=all.
> 
> Do you remember why we do this? Intuitively, you'd expect that GDB
> would behave the same regardless of whether it selects type "long"
> or "long long" for its processing, as long as both types are 64-bit.
> 

I have no idea.  AFAICT it's been in gdb since the "Add base ada 
language files" commit from 2002 that we avoid "unsigned long long".

I've taken care to preserve the behaviour in the commit you refer to 
(and this patch), since I don't have the knowledge to decide that things 
should be different.

> With the above, we can take this patch as an intermediate remedy,
> but I think we might need to dig deeper into why we use a signed
> type in the case of long long but not long, particularly when both
> types are the same size.
> 
> WDYT?
> 

I'd be happy to write a patch to change the behaviour of gdb.  But 
somebody knowledgeable in Ada needs to specify what that needs to be.

Thanks,
- Tom

>> ---
>>   gdb/testsuite/gdb.ada/literals.exp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.ada/literals.exp b/gdb/testsuite/gdb.ada/literals.exp
>> index a6ac89b540f..744a6bc573c 100644
>> --- a/gdb/testsuite/gdb.ada/literals.exp
>> +++ b/gdb/testsuite/gdb.ada/literals.exp
>> @@ -36,4 +36,4 @@ gdb_test "print 16#f#e1" " = 240"
>>   gdb_test "print 16#1#e10" " = 1099511627776"
>>   
>>   gdb_test "print/x 16#7fffffffffffffff#" " = 0x7fffffffffffffff"
>> -gdb_test "print 16#ffffffffffffffff#" " = -1"
>> +gdb_test "print 16#ffffffffffffffff#" " = (-1|18446744073709551615)"
>