mbox series

[v1,0/2] Fix the right shift of negative numbers

Message ID cover.1712086444.git.sergey_v_kaplun@mail.ru
Headers
Series Fix the right shift of negative numbers |

Message

Sergey Kaplun April 2, 2024, 8:17 p.m. UTC 
  The original issue is the incorrect truncated result of the right shift
for negative values:
```
(gdb) p /t -3
$1 = 11111111111111111111111111111101
(gdb) p /t -3 >> 1
$2 = 11111111111111111111111111111111
```
Where 11111111111111111111111111111110 is expected.

This is due to the use of `mpz_tdiv_q_2exp()` instead of
`mpz_fdiv_q_2exp()` for the right shift calculation.
According to the documentation of gmplib [1]:
```
For positive n both mpz_fdiv_q_2exp and mpz_tdiv_q_2exp are simple
bitwise right shifts.  For negative n, mpz_fdiv_q_2exp is effectively an
arithmetic right shift treating n as twos complement the same as the
bitwise logical functions do, whereas mpz_tdiv_q_2exp effectively treats
n as sign and magnitude.
```

The second patch fixes the behaviour and adds the testcase above to
gdb.base/bitshift.exp.

But when I tried to add the test, I've noticed that all test cases from
this file are skipped due to using `return` instead of `continue` for
the skip list of unhandled languages.  So, I've enabled this test back
in the first patch and fixed the behaviour for the negative shift of
negative values.

Unfortunately, some test cases for "opencl" still fail, so I've added it
to the skip list for this version of the patch since I'm not very
familiar with it (I need some guidance here):

| make check TESTS="gdb.base/bitshift.exp" | grep FAIL
| WARNING: Couldn't find the global config file.
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, promoted: print /x (signed char) 0x0f << 8
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, promoted: print (signed char) 0x0f << 8
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f << -1
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f >> -1
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print/x (signed char) 0x7f << 8
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print/x (signed char) 0x7f >> 8
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f << 32
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f >> 32
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f << 33
| FAIL: gdb.base/bitshift.exp: lang=opencl: 8-bit, invalid: print (signed char) 0x7f >> 33

For other tests, I see no regressions (some 3 tests are flaky on master):

| -# of expected passes		112908
| -# of unexpected failures	1791
| +# of expected passes		112011
| +# of unexpected failures	1788
|  # of expected failures		219
|  # of known failures		122
|  # of untested testcases		22

Within enabled back gdb.base/bitshift.exp (and disabled opencl):

| make check TESTS="gdb.base/bitshift.exp"
| ...
| === gdb Summary ===
|
| # of expected passes            921

Tested on Linux 6.1.57-gentoo-x86_64 x86_64.
Configure command flags:
| --enable-build-warnings --enable-ubsan --enable-unit-tests=yes

[1]: https://gmplib.org/gmp-man-6.2.1.pdf#Integer%20Division

Sergey Kaplun (2):
  gdb/testsuite: enable back gdb.base/bitshift.exp
  Fix the right shift of negative numbers

 gdb/gmp-utils.h                     |  4 ++--
 gdb/testsuite/gdb.base/bitshift.exp |  7 +++++--
 gdb/valarith.c                      | 11 ++++++++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

Comments

Sergey Kaplun April 23, 2024, 8:45 a.m. UTC | #1 
Hi, guys!
I see that my patch fixes the 31590 issue [1].

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=31590
Tom Tromey April 23, 2024, 2:46 p.m. UTC | #2 
>>>>> "Sergey" == Sergey Kaplun <sergey_v_kaplun@mail.ru> writes:

Sergey> Hi, guys!
Sergey> I see that my patch fixes the 31590 issue [1].

Sergey> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=31590

Hi.  Do you have a copyright assignment in place?
I think we'll probably need one for this series.

Tom
Sergey Kaplun April 25, 2024, 1:23 p.m. UTC | #3 
On 23.04.24, Tom Tromey wrote:
> >>>>> "Sergey" == Sergey Kaplun <sergey_v_kaplun@mail.ru> writes:
> 
> Sergey> Hi, guys!
> Sergey> I see that my patch fixes the 31590 issue [1].
> 
> Sergey> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=31590
> 
> Hi.  Do you have a copyright assignment in place?
> I think we'll probably need one for this series.
> 
> Tom

Hi, Tom.
No, I don't have a copyright assignment yet. What do I need to do?