[PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Tiezhu Yang]

Tiezhu Yang (2):
  gdb: testsuite: fix failed testcases in gdb.base/charset.exp
  gdb: testsuite: fix wrong comment in gdb.base/charset.c

 gdb/testsuite/gdb.base/charset.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.27.0

[PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Tiezhu Yang]

In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
char, this will change the value due to sign extension.

For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
EBCDIC-US.

Make the type of string[] as unsigned char to fix the following six failed
testcases:

  $ grep FAIL gdb/testsuite/gdb.sum
  FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
  FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
  FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
  FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
  FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
  FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 gdb/testsuite/gdb.base/charset.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/charset.c b/gdb/testsuite/gdb.base/charset.c
index 35ab9c25503..97ee2029908 100644
--- a/gdb/testsuite/gdb.base/charset.c
+++ b/gdb/testsuite/gdb.base/charset.c
@@ -44,10 +44,10 @@
 
 #define NUM_CHARS (71)
 
-char ascii_string[NUM_CHARS];
-char iso_8859_1_string[NUM_CHARS];
-char ebcdic_us_string[NUM_CHARS];
-char ibm1047_string[NUM_CHARS];
+unsigned char ascii_string[NUM_CHARS];
+unsigned char iso_8859_1_string[NUM_CHARS];
+unsigned char ebcdic_us_string[NUM_CHARS];
+unsigned char ibm1047_string[NUM_CHARS];
 
 #ifndef __cplusplus
 
@@ -86,7 +86,7 @@ long long_array[3];
    explicit casts or warnings.  */
 
 void
-init_string (char string[],
+init_string (unsigned char string[],
 	     unsigned char x,
 	     unsigned char alert,
 	     unsigned char backspace,
@@ -115,7 +115,7 @@ init_string (char string[],
 
 
 void
-fill_run (char string[], int start, int len, int first)
+fill_run (unsigned char string[], int start, int len, int first)
 {
   int i;
 
-- 
2.27.0

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Simon Marchi]

On 2021-12-24 23:03, Tiezhu Yang wrote:
> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
> char, this will change the value due to sign extension.
> 
> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
> EBCDIC-US.
> 
> Make the type of string[] as unsigned char to fix the following six failed
> testcases:
> 
>   $ grep FAIL gdb/testsuite/gdb.sum
>   FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>   FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>   FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>   FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>   FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>   FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047

Out of curiosity, on which configuration do you see these failures?
Asking because I don't see them on x86-64 Linux.

Is it related to the fact that some architectures have "char" signed by
default and others unsigned by default?

Simon

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Tiezhu Yang]

On 1/10/22 10:42, Simon Marchi wrote:
> 
> 
> On 2021-12-24 23:03, Tiezhu Yang wrote:
>> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
>> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
>> char, this will change the value due to sign extension.
>>
>> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
>> EBCDIC-US.
>>
>> Make the type of string[] as unsigned char to fix the following six failed
>> testcases:
>>
>>    $ grep FAIL gdb/testsuite/gdb.sum
>>    FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>    FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>    FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>    FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>    FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>    FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
> 
> Out of curiosity, on which configuration do you see these failures?

I test this on LoongArch which is a new RISC architecture.

> Asking because I don't see them on x86-64 Linux.
> 
> Is it related to the fact that some architectures have "char" signed by
> default and others unsigned by default?

Yes.

Each kind of machine has a default for what char should be. It is either 
like unsigned char by default or like signed char by default.

Ideally, a portable program should always use signed char or unsigned 
char when it depends on the signedness of an object.

https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html

Thanks,
Tiezhu

> 
> Simon

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Simon Marchi]

On 2022-01-09 22:23, Tiezhu Yang wrote:
> 
> 
> On 1/10/22 10:42, Simon Marchi wrote:
>>
>>
>> On 2021-12-24 23:03, Tiezhu Yang wrote:
>>> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
>>> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
>>> char, this will change the value due to sign extension.
>>>
>>> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
>>> EBCDIC-US.
>>>
>>> Make the type of string[] as unsigned char to fix the following six failed
>>> testcases:
>>>
>>>    $ grep FAIL gdb/testsuite/gdb.sum
>>>    FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>>    FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>>    FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>>    FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>>    FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>>    FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>
>> Out of curiosity, on which configuration do you see these failures?

Hmm, I now see these failures on an x86-64 Linux (Ubuntu 20.04) machine:

FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047

The first FAIL looks like:

444 print /d 'A' == ebcdic_us_string[7]^M
445 $63 = 0^M
446 (gdb) FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US

Could you give it a test on x86-64?

Simon

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Tiezhu Yang]

On 1/12/22 12:28, Simon Marchi wrote:
> 
> 
> On 2022-01-09 22:23, Tiezhu Yang wrote:
>>
>>
>> On 1/10/22 10:42, Simon Marchi wrote:
>>>
>>>
>>> On 2021-12-24 23:03, Tiezhu Yang wrote:
>>>> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
>>>> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
>>>> char, this will change the value due to sign extension.
>>>>
>>>> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
>>>> EBCDIC-US.
>>>>
>>>> Make the type of string[] as unsigned char to fix the following six failed
>>>> testcases:
>>>>
>>>>     $ grep FAIL gdb/testsuite/gdb.sum
>>>>     FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>>>     FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>>>     FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>>>     FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>>>     FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>>>     FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>>
>>> Out of curiosity, on which configuration do you see these failures?
> 
> Hmm, I now see these failures on an x86-64 Linux (Ubuntu 20.04) machine:
> 
> FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
> FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
> FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
> FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
> 
> The first FAIL looks like:
> 
> 444 print /d 'A' == ebcdic_us_string[7]^M
> 445 $63 = 0^M
> 446 (gdb) FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
> 
> Could you give it a test on x86-64?

On x86-64:

   (gdb) set target-charset EBCDIC-US
   (gdb) print 'A'
   $1 = -63 'A'

On LoongArch:

   (gdb) set target-charset EBCDIC-US
   (gdb) print 'A'
   $1 = 193 'A'

In gdb/testsuite/gdb.base/charset.c, the original
ebcdic_us_string[7] is 193,
(1) if string[] type is signed, it will change 193 to -63.
(2) if string[] type is unsigned, it will keep it as 193.

According to the official doc, 'A' is 193 in EBCDIC.
https://www.ibm.com/docs/en/zos/2.1.0?topic=sequences-ebcdic

So it seems that there exists some issues when
set target-charset EBCDIC-US
on x86-64?

Thanks,
Tiezhu

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Simon Marchi]

On 2022-01-12 06:31, Tiezhu Yang wrote:
> 
> 
> On 1/12/22 12:28, Simon Marchi wrote:
>>
>>
>> On 2022-01-09 22:23, Tiezhu Yang wrote:
>>>
>>>
>>> On 1/10/22 10:42, Simon Marchi wrote:
>>>>
>>>>
>>>> On 2021-12-24 23:03, Tiezhu Yang wrote:
>>>>> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
>>>>> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
>>>>> char, this will change the value due to sign extension.
>>>>>
>>>>> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
>>>>> EBCDIC-US.
>>>>>
>>>>> Make the type of string[] as unsigned char to fix the following six failed
>>>>> testcases:
>>>>>
>>>>>     $ grep FAIL gdb/testsuite/gdb.sum
>>>>>     FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>>>>     FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>>>>     FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>>>>     FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>>>>     FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>>>>     FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>>>
>>>> Out of curiosity, on which configuration do you see these failures?
>>
>> Hmm, I now see these failures on an x86-64 Linux (Ubuntu 20.04) machine:
>>
>> FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>> FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>> FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>> FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>
>> The first FAIL looks like:
>>
>> 444 print /d 'A' == ebcdic_us_string[7]^M
>> 445 $63 = 0^M
>> 446 (gdb) FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>
>> Could you give it a test on x86-64?
> 
> On x86-64:
> 
>   (gdb) set target-charset EBCDIC-US
>   (gdb) print 'A'
>   $1 = -63 'A'
> 
> On LoongArch:
> 
>   (gdb) set target-charset EBCDIC-US
>   (gdb) print 'A'
>   $1 = 193 'A'
> 
> In gdb/testsuite/gdb.base/charset.c, the original
> ebcdic_us_string[7] is 193,
> (1) if string[] type is signed, it will change 193 to -63.
> (2) if string[] type is unsigned, it will keep it as 193.
> 
> According to the official doc, 'A' is 193 in EBCDIC.
> https://www.ibm.com/docs/en/zos/2.1.0?topic=sequences-ebcdic
> 
> So it seems that there exists some issues when
> set target-charset EBCDIC-US
> on x86-64?

Ok, so 'A' yields a "char".  It's signed on x86-64 because "char" is
signed on x86-64.  So the expression `'A' == ebcdic_us_string[7]`
compares the signed char with bits 0xc1 with the unsigned char with bits
0xc1.

This little program shows 0 on x86-64:

~~~
#include <stdio.h>

int main()  {
    char c1 = -63;
    unsigned char c2 = 193;
    printf("%d\n", c1 == c2);
    return 0;
}
~~~

On AArch64, where char is unsigned by default, it shows 1.

I don't really see the problem with the original test code, to be
honest.  On an architecture where char is signed, then both 'A' and
ebcdic_us_string[7] will yield -63, which makes the equality true.  On
an architecture where char is unsigned, then both 'A' and
ebcdic_us_string[7] will yield 193, which also makes the equality true.

An interesting thing is that the test did pass on AArch64 (which has
char unsigned like LoongArch) before your patch.  Can you investigate
why?

Simon

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Tiezhu Yang]

On 1/13/22 01:14, Simon Marchi wrote:
> 
> 
> On 2022-01-12 06:31, Tiezhu Yang wrote:
>>
>>
>> On 1/12/22 12:28, Simon Marchi wrote:
>>>
>>>
>>> On 2022-01-09 22:23, Tiezhu Yang wrote:
>>>>
>>>>
>>>> On 1/10/22 10:42, Simon Marchi wrote:
>>>>>
>>>>>
>>>>> On 2021-12-24 23:03, Tiezhu Yang wrote:
>>>>>> In gdb/testsuite/gdb.base/charset.c, the last argument is greater than 127
>>>>>> when call fill_run() in EBCDIC-US and IBM1047, but the type of string[] is
>>>>>> char, this will change the value due to sign extension.
>>>>>>
>>>>>> For example, ebcdic_us_string[7] will be -63 instead of the original 193 in
>>>>>> EBCDIC-US.
>>>>>>
>>>>>> Make the type of string[] as unsigned char to fix the following six failed
>>>>>> testcases:
>>>>>>
>>>>>>      $ grep FAIL gdb/testsuite/gdb.sum
>>>>>>      FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>>>>>      FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>>>>>      FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>>>>>      FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>>>>>      FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>>>>>      FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>>>>
>>>>> Out of curiosity, on which configuration do you see these failures?
>>>
>>> Hmm, I now see these failures on an x86-64 Linux (Ubuntu 20.04) machine:
>>>
>>> FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>> FAIL: gdb.base/charset.exp: check value of parsed string literal in EBCDIC-US
>>> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in EBCDIC-US
>>> FAIL: gdb.base/charset.exp: check value of parsed character literal in IBM1047
>>> FAIL: gdb.base/charset.exp: check value of parsed string literal in IBM1047
>>> FAIL: gdb.base/charset.exp: check value of escape that doesn't exist in IBM1047
>>>
>>> The first FAIL looks like:
>>>
>>> 444 print /d 'A' == ebcdic_us_string[7]^M
>>> 445 $63 = 0^M
>>> 446 (gdb) FAIL: gdb.base/charset.exp: check value of parsed character literal in EBCDIC-US
>>>
>>> Could you give it a test on x86-64?
>>
>> On x86-64:
>>
>>    (gdb) set target-charset EBCDIC-US
>>    (gdb) print 'A'
>>    $1 = -63 'A'
>>
>> On LoongArch:
>>
>>    (gdb) set target-charset EBCDIC-US
>>    (gdb) print 'A'
>>    $1 = 193 'A'
>>
>> In gdb/testsuite/gdb.base/charset.c, the original
>> ebcdic_us_string[7] is 193,
>> (1) if string[] type is signed, it will change 193 to -63.
>> (2) if string[] type is unsigned, it will keep it as 193.
>>
>> According to the official doc, 'A' is 193 in EBCDIC.
>> https://www.ibm.com/docs/en/zos/2.1.0?topic=sequences-ebcdic
>>
>> So it seems that there exists some issues when
>> set target-charset EBCDIC-US
>> on x86-64?
> 
> Ok, so 'A' yields a "char".  It's signed on x86-64 because "char" is
> signed on x86-64.  So the expression `'A' == ebcdic_us_string[7]`
> compares the signed char with bits 0xc1 with the unsigned char with bits
> 0xc1.
> 
> This little program shows 0 on x86-64:
> 
> ~~~
> #include <stdio.h>
> 
> int main()  {
>      char c1 = -63;
>      unsigned char c2 = 193;
>      printf("%d\n", c1 == c2);
>      return 0;
> }
> ~~~
> 
> On AArch64, where char is unsigned by default, it shows 1.
> 
> I don't really see the problem with the original test code, to be
> honest.  On an architecture where char is signed, then both 'A' and
> ebcdic_us_string[7] will yield -63, which makes the equality true.  On
> an architecture where char is unsigned, then both 'A' and
> ebcdic_us_string[7] will yield 193, which also makes the equality true.

Yes, it is reasonable, thank you.

> 
> An interesting thing is that the test did pass on AArch64 (which has
> char unsigned like LoongArch) before your patch.  Can you investigate
> why?

   $ cat test_char.c
   #include <stdio.h>

   int main()
   {
           char c1 = 193;
	  unsigned char c2 = 193;

	  printf("%d\n", c1);
	  printf("%d\n", c1 == c2);

	  return 0;
   }

(1) On x86-64:

   $ gcc test_char.c -o test_char
   $ ./test_char
   -63
   0

   We can see that the default type of char is signed char
   on x86-64.

   When use gdb print command on x86-64, the default type
   of char is also signed char.

   (gdb) set target-charset EBCDIC-US
   (gdb) print 'A'
   $1 = -63 'A'
   (gdb) print /c 'A'
   $2 = -63 'A'
   (gdb) print /u 'A'
   $3 = 193
   (gdb) print /d 'A'
   $4 = -63
   (gdb) print /x 'A'
   $5 = 0xc1

(2) On LoongArch:

   $ gcc test_char.c -o test_char
   $ ./test_char
   -63
   0

   We can see that the default type of char is signed char
   on LoongArch, like x86-64.

   But when use gdb print command on LoongArch, the default type
   of char is unsigned char, this is wrong, I will look into it.

   (gdb) set target-charset EBCDIC-US
   (gdb) print 'A'
   $1 = 193 'A'
   (gdb) print /c 'A'
   $2 = 193 'A'
   (gdb) print /u 'A'
   $3 = 193
   (gdb) print /d 'A'
   $4 = -63
   (gdb) print /x 'A'
   $5 = 0xc1

   So at least for now, it is better to revert the following commit:

   gdb: testsuite: fix failed testcases in gdb.base/charset.exp
   https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ff656e2e1cb1

   If you are OK, I will send a patch to do it. I am sorry for that.

Thanks,
Tiezhu

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Simon Marchi]

>   $ cat test_char.c
>   #include <stdio.h>
> 
>   int main()
>   {
>           char c1 = 193;
>       unsigned char c2 = 193;
> 
>       printf("%d\n", c1);
>       printf("%d\n", c1 == c2);
> 
>       return 0;
>   }
> 
> (1) On x86-64:
> 
>   $ gcc test_char.c -o test_char
>   $ ./test_char
>   -63
>   0
> 
>   We can see that the default type of char is signed char
>   on x86-64.
> 
>   When use gdb print command on x86-64, the default type
>   of char is also signed char.
> 
>   (gdb) set target-charset EBCDIC-US
>   (gdb) print 'A'
>   $1 = -63 'A'
>   (gdb) print /c 'A'
>   $2 = -63 'A'
>   (gdb) print /u 'A'
>   $3 = 193
>   (gdb) print /d 'A'
>   $4 = -63
>   (gdb) print /x 'A'
>   $5 = 0xc1
> 
> (2) On LoongArch:
> 
>   $ gcc test_char.c -o test_char
>   $ ./test_char
>   -63
>   0
> 
>   We can see that the default type of char is signed char
>   on LoongArch, like x86-64.
> 
>   But when use gdb print command on LoongArch, the default type
>   of char is unsigned char, this is wrong, I will look into it.

Oh, so it's opposite of what we thought initially (we thought that
LoongArch had unsigned char by default)?

>   (gdb) set target-charset EBCDIC-US
>   (gdb) print 'A'
>   $1 = 193 'A'
>   (gdb) print /c 'A'
>   $2 = 193 'A'
>   (gdb) print /u 'A'
>   $3 = 193
>   (gdb) print /d 'A'
>   $4 = -63
>   (gdb) print /x 'A'
>   $5 = 0xc1
> 
>   So at least for now, it is better to revert the following commit:
> 
>   gdb: testsuite: fix failed testcases in gdb.base/charset.exp
>   https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ff656e2e1cb1
> 
>   If you are OK, I will send a patch to do it. I am sorry for that.

Yes, that is fine with me (it is pre-approved, you can push it right
away, just make sure to send a notification on the mailing list).

Thanks,

Simon

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Tiezhu Yang]

On 1/13/22 09:26, Simon Marchi wrote:
>>    $ cat test_char.c
>>    #include <stdio.h>
>>
>>    int main()
>>    {
>>            char c1 = 193;
>>        unsigned char c2 = 193;
>>
>>        printf("%d\n", c1);
>>        printf("%d\n", c1 == c2);
>>
>>        return 0;
>>    }
>>
>> (1) On x86-64:
>>
>>    $ gcc test_char.c -o test_char
>>    $ ./test_char
>>    -63
>>    0
>>
>>    We can see that the default type of char is signed char
>>    on x86-64.
>>
>>    When use gdb print command on x86-64, the default type
>>    of char is also signed char.
>>
>>    (gdb) set target-charset EBCDIC-US
>>    (gdb) print 'A'
>>    $1 = -63 'A'
>>    (gdb) print /c 'A'
>>    $2 = -63 'A'
>>    (gdb) print /u 'A'
>>    $3 = 193
>>    (gdb) print /d 'A'
>>    $4 = -63
>>    (gdb) print /x 'A'
>>    $5 = 0xc1
>>
>> (2) On LoongArch:
>>
>>    $ gcc test_char.c -o test_char
>>    $ ./test_char
>>    -63
>>    0
>>
>>    We can see that the default type of char is signed char
>>    on LoongArch, like x86-64.
>>
>>    But when use gdb print command on LoongArch, the default type
>>    of char is unsigned char, this is wrong, I will look into it.
> 
> Oh, so it's opposite of what we thought initially (we thought that
> LoongArch had unsigned char by default)?

Yes, thanks very much for your analysis.
I will be more careful in the future.

> 
>>    (gdb) set target-charset EBCDIC-US
>>    (gdb) print 'A'
>>    $1 = 193 'A'
>>    (gdb) print /c 'A'
>>    $2 = 193 'A'
>>    (gdb) print /u 'A'
>>    $3 = 193
>>    (gdb) print /d 'A'
>>    $4 = -63
>>    (gdb) print /x 'A'
>>    $5 = 0xc1
>>
>>    So at least for now, it is better to revert the following commit:
>>
>>    gdb: testsuite: fix failed testcases in gdb.base/charset.exp
>>    https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ff656e2e1cb1
>>
>>    If you are OK, I will send a patch to do it. I am sorry for that.
> 
> Yes, that is fine with me (it is pre-approved, you can push it right
> away, just make sure to send a notification on the mailing list).

Done. Thank you!

Thanks,
Tiezhu

Re: [PATCH 1/2] gdb: testsuite: fix failed testcases in gdb.base/charset.exp

[Simon Marchi]

>> Yes, that is fine with me (it is pre-approved, you can push it right
>> away, just make sure to send a notification on the mailing list).
> 
> Done. Thank you!

Thanks, and I look forward to see what's the root cause of the problem
on LoongArch!

Simon

[PATCH 2/2] gdb: testsuite: fix wrong comment in gdb.base/charset.c

[Tiezhu Yang]

In gdb/testsuite/gdb.base/charset.c, use "IBM1047" instead of "EBCDIC-US"
to fix the wrong comment.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 gdb/testsuite/gdb.base/charset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/charset.c b/gdb/testsuite/gdb.base/charset.c
index 97ee2029908..e1725f06fcb 100644
--- a/gdb/testsuite/gdb.base/charset.c
+++ b/gdb/testsuite/gdb.base/charset.c
@@ -183,7 +183,7 @@ int main ()
                47, 22, 12,
                37, 13, 5,
                11, 74, 17);
-  /* In EBCDIC, the upper-case letters are broken into three separate runs.  */
+  /* In IBM1047, the upper-case letters are broken into three separate runs.  */
   fill_run (ibm1047_string, 7, 9, 193);
   fill_run (ibm1047_string, 16, 9, 209);
   fill_run (ibm1047_string, 25, 8, 226);
-- 
2.27.0

Re: [PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Joel Brobecker]

Hi Tiezhu,

On Sat, Dec 25, 2021 at 12:03:18PM +0800, Tiezhu Yang wrote:
> Tiezhu Yang (2):
>   gdb: testsuite: fix failed testcases in gdb.base/charset.exp
>   gdb: testsuite: fix wrong comment in gdb.base/charset.c

Thanks for those patches. They look good to me.

Do you have a copyright assignment on file for GDB? I searched
the FSF records, and couldn't find your name. If not, we can
take your changes under the "tiny patch" rule.

-- 
Joel

Re: [PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Tiezhu Yang]

On 12/29/21 12:25, Joel Brobecker wrote:
> Hi Tiezhu,
> 
> On Sat, Dec 25, 2021 at 12:03:18PM +0800, Tiezhu Yang wrote:
>> Tiezhu Yang (2):
>>    gdb: testsuite: fix failed testcases in gdb.base/charset.exp
>>    gdb: testsuite: fix wrong comment in gdb.base/charset.c
> 
> Thanks for those patches. They look good to me.
> 
> Do you have a copyright assignment on file for GDB? I searched
> the FSF records, and couldn't find your name. If not, we can
> take your changes under the "tiny patch" rule.
> 

Hi Joel,

Thank you very much for your reply.

I am working for Loongson Technology Corporation Limited.
Loongson has signed the copyright assignment.
Loongson GNU ANY is RT 1604586.

By the way, could you please have a look at the following patch series?

gdb: Add basic support for LoongArch
https://sourceware.org/pipermail/gdb-patches/2021-December/184476.html

Thanks,
Tiezhu

Re: [PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Joel Brobecker]

> > > Tiezhu Yang (2):
> > >    gdb: testsuite: fix failed testcases in gdb.base/charset.exp
> > >    gdb: testsuite: fix wrong comment in gdb.base/charset.c
> > 
> > Thanks for those patches. They look good to me.
> > 
> > Do you have a copyright assignment on file for GDB? I searched
> > the FSF records, and couldn't find your name. If not, we can
> > take your changes under the "tiny patch" rule.
>
> Thank you very much for your reply.
> 
> I am working for Loongson Technology Corporation Limited.
> Loongson has signed the copyright assignment.
> Loongson GNU ANY is RT 1604586.

Great. Do you have write access to the repository? It's looking
like you are contributing more to the project, so it would make
sense to me if you were allowed to push to the repository though
our "Write After Approval" procedure.

-- 
Joel

Re: [PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Tiezhu Yang]

On 1/8/22 16:29, Joel Brobecker wrote:
>>>> Tiezhu Yang (2):
>>>>     gdb: testsuite: fix failed testcases in gdb.base/charset.exp
>>>>     gdb: testsuite: fix wrong comment in gdb.base/charset.c
>>>
>>> Thanks for those patches. They look good to me.
>>>
>>> Do you have a copyright assignment on file for GDB? I searched
>>> the FSF records, and couldn't find your name. If not, we can
>>> take your changes under the "tiny patch" rule.
>>
>> Thank you very much for your reply.
>>
>> I am working for Loongson Technology Corporation Limited.
>> Loongson has signed the copyright assignment.
>> Loongson GNU ANY is RT 1604586.
> 
> Great. Do you have write access to the repository? It's looking
> like you are contributing more to the project, so it would make
> sense to me if you were allowed to push to the repository though
> our "Write After Approval" procedure.
> 

Hi Joel,

I do not have write access now, when I clone the read-write git,
it appears "Please make sure you have the correct access rights
and the repository exists."

My git config is:

   user.name=Tiezhu Yang
   user.email=yangtiezhu@loongson.cn

Could you please help me to enable write access?

Thanks,
Tiezhu

Re: [PATCH 0/2] Modify gdb.base/charset.c to fix some issues

[Joel Brobecker]

> I do not have write access now, when I clone the read-write git,
> it appears "Please make sure you have the correct access rights
> and the repository exists."
> 
> My git config is:
> 
>   user.name=Tiezhu Yang
>   user.email=yangtiezhu@loongson.cn
> 
> Could you please help me to enable write access?

No problem. We will take this off-list.

-- 
Joel