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

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

[Tom de Vries via Gdb-patches]

Hi,

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,
- Tom

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

---
 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)"

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

[Luis Machado via Gdb-patches]

On 7/27/22 11:34, Tom de Vries wrote:
> Hi,
> 
> 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,
> - Tom
> 
> [gdb/testsuite] Fix gdb.ada/literals.exp with aarch64
> 
> ---
>   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)"

LGTM

Thanks for investigating this.

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

[Joel Brobecker via Gdb-patches]

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.

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?

> ---
>  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)"

-- 
Joel

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

[Tom de Vries via Gdb-patches]

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)"
> 

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

[Joel Brobecker via Gdb-patches]

> > 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.

We're really outside of the Ada language. Would you mind checking
what the C language does? I think it would make sense to do the same,
here. E.g., on x86_64-linux, I get:

    (gdb) p 0xffffffffffffffff
    $1 = 18446744073709551615

What do you think, TomT?

> > >   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)"
> > 

-- 
Joel

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

[Tom de Vries via Gdb-patches]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On 7/28/22 15:10, Joel Brobecker wrote:
>>> 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.
> 
> We're really outside of the Ada language. Would you mind checking
> what the C language does? I think it would make sense to do the same,
> here. E.g., on x86_64-linux, I get:
> 
>      (gdb) p 0xffffffffffffffff
>      $1 = 18446744073709551615
> 

Ack, that's what the C language does.

This patch changes gdb to print 18446744073709551615 for ada as well.

So, pick your favorite patch ;)

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Fix-gdb.ada-literals.exp-with-aarch64.patch --]
[-- Type: text/x-patch, Size: 2886 bytes --]

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

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;
...

It's not clear why for ada we're using long long for the
fits-in-unsigned-long-long case.

Fix this by using unsigned long long for the fits-in-unsigned-long-long case,
meaning the new reference output is 18446744073709551615 instead of -1.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29416

---
 gdb/ada-lex.l                      | 4 ++--
 gdb/testsuite/gdb.ada/literals.exp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/ada-lex.l b/gdb/ada-lex.l
index 002eb811e41..ed88d502e9f 100644
--- a/gdb/ada-lex.l
+++ b/gdb/ada-lex.l
@@ -498,8 +498,8 @@ processInt (struct parser_state *par_state, const char *base0,
     yylval.typed_val.type = type_long_long (par_state);
   else if (fits_in_type (1, value, long_long_bits, false))
     {
-      /* Note: Interprets ULLONG_MAX as -1.  */
-      yylval.typed_val.type = type_long_long (par_state);
+      yylval.typed_val.type
+	= builtin_type (par_state->gdbarch ())->builtin_unsigned_long_long;
       /* See unsigned long case above.  */
       if (value & LONGEST_SIGN)
 	yylval.typed_val.val =
diff --git a/gdb/testsuite/gdb.ada/literals.exp b/gdb/testsuite/gdb.ada/literals.exp
index a6ac89b540f..6badc857292 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#" " = 18446744073709551615"

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

[Joel Brobecker via Gdb-patches]

Hi Tom,

> > We're really outside of the Ada language. Would you mind checking
> > what the C language does? I think it would make sense to do the same,
> > here. E.g., on x86_64-linux, I get:
> > 
> >      (gdb) p 0xffffffffffffffff
> >      $1 = 18446744073709551615
> 
> Ack, that's what the C language does.
> 
> This patch changes gdb to print 18446744073709551615 for ada as well.
> 
> So, pick your favorite patch ;)

:-) Thanks! Let us evaluate this patch within AdaCore, just to double-
check I haven't missed anything. We'll get back to you on it.

-- 
Joel

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

[Joel Brobecker via Gdb-patches]

Hi Tom,

On Thu, Jul 28, 2022 at 04:21:35PM +0200, Tom de Vries wrote:
> On 7/28/22 15:10, Joel Brobecker wrote:
> > > > 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.
> > 
> > We're really outside of the Ada language. Would you mind checking
> > what the C language does? I think it would make sense to do the same,
> > here. E.g., on x86_64-linux, I get:
> > 
> >      (gdb) p 0xffffffffffffffff
> >      $1 = 18446744073709551615
> > 
> 
> Ack, that's what the C language does.
> 
> This patch changes gdb to print 18446744073709551615 for ada as well.
> 
> So, pick your favorite patch ;)

TomT kindly tested your patch on a number of platforms for us,
and everything looked good on those. I propose we go with your
second patch which changes the behavior to match what we do
in C (I looked at the patch, and it looks good to me, so approved).

Thank you!

> 
> Thanks,
> - Tom

> [gdb/testsuite] Fix gdb.ada/literals.exp with aarch64
> 
> 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;
> ...
> 
> It's not clear why for ada we're using long long for the
> fits-in-unsigned-long-long case.
> 
> Fix this by using unsigned long long for the fits-in-unsigned-long-long case,
> meaning the new reference output is 18446744073709551615 instead of -1.
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29416
> 
> ---
>  gdb/ada-lex.l                      | 4 ++--
>  gdb/testsuite/gdb.ada/literals.exp | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/ada-lex.l b/gdb/ada-lex.l
> index 002eb811e41..ed88d502e9f 100644
> --- a/gdb/ada-lex.l
> +++ b/gdb/ada-lex.l
> @@ -498,8 +498,8 @@ processInt (struct parser_state *par_state, const char *base0,
>      yylval.typed_val.type = type_long_long (par_state);
>    else if (fits_in_type (1, value, long_long_bits, false))
>      {
> -      /* Note: Interprets ULLONG_MAX as -1.  */
> -      yylval.typed_val.type = type_long_long (par_state);
> +      yylval.typed_val.type
> +	= builtin_type (par_state->gdbarch ())->builtin_unsigned_long_long;
>        /* See unsigned long case above.  */
>        if (value & LONGEST_SIGN)
>  	yylval.typed_val.val =
> diff --git a/gdb/testsuite/gdb.ada/literals.exp b/gdb/testsuite/gdb.ada/literals.exp
> index a6ac89b540f..6badc857292 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#" " = 18446744073709551615"


-- 
Joel