[Bug sim/27483] New: aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[Bug sim/27483] New: aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[vapier at gentoo dot org]

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

            Bug ID: 27483
           Summary: aarch64: adds test failing when adding 0x80000000 &
                    0xffffffff
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: sim
          Assignee: nickc at redhat dot com
          Reporter: vapier at gentoo dot org
                CC: nickc at redhat dot com, vapier at gentoo dot org,
                    wilson at tuliptree dot org
  Target Milestone: ---

it seems like this test has failed since it was added ?  at least on my system,
checking out the original commit shows it failing.

$ ./aarch64/run --trace --trace-reg ./testsuite/adds.s.x 
...
insn:      pc = 400030 instr = 52b00000      
reg:      GR[ 0] changes from                2 to         80000000
insn:      pc = 400034 instr = 12800001      
insn:      pc = 400038 instr = 2b010002      
reg:      GR[ 2] changes from                1 to         7fffffff
reg:      CPSR stays at --C-
insn:      pc = 40003c instr = 54000b27 
reg:      NextPC changes from           400040 to           4001a0
...

where that code is:
  400030:   52b00000    mov w0, #0x80000000             // #-2147483648
  400034:   12800001    mov w1, #0xffffffff             // #-1
  400038:   2b010002    adds    w2, w0, w1
  40003c:   54000b27    b.vc    4001a0 <_start+0x1a0>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[wilson at gcc dot gnu.org]

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

Jim Wilson <wilson at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at gcc dot gnu.org

--- Comment #1 from Jim Wilson <wilson at gcc dot gnu.org> ---
It works for me.

Testing adds.s on machine aarch64.
Executing on host: /scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/gas/as-new
..\
/../../combined/sim/testsuite/aarch64/adds.s 
-I../../../combined/sim/testsuite\
/aarch64  -o adds.s.o    (timeout = 300)
spawn -ignore SIGHUP /scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/gas/as-new
\
../../../combined/sim/testsuite/aarch64/adds.s
-I../../../combined/sim/testsuit\
e/aarch64 -o adds.s.o^M
Executing on host: /scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/ld/ld-new
add\
s.s.o   -o adds.s.x    (timeout = 300)
spawn -ignore SIGHUP /scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/ld/ld-new
a\
dds.s.o -o adds.s.x^M
/scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/sim/testsuite/../aarch64/run   
\
 adds.s.x
spawn
/scratch/jimw/FOSS/COMBINED-TREE/X-aarch64-elf/sim/testsuite/../aarch64/r\
un adds.s.x^M
pass^M
PASS: aarch64 adds.s

Trying a trace, I see
insn:      pc = 400030 instr = 52b00000
reg:      GR[ 0] changes from                2 to         80000000
insn:      pc = 400034 instr = 12800001
insn:      pc = 400038 instr = 2b010002
reg:      GR[ 2] changes from                1 to         7fffffff
reg:      CPSR changes from --C- to --CV
insn:      pc = 40003c instr = 54000b27
insn:      pc = 400040 instr = 12800020

I've probably never tested it anywhere other than x86-64 host though.  Maybe
there is a subtle bug that only shows up on other hosts.  Or maybe it doesn't
work on a 32-bit host.

The flags should be set in set_flags_for_add32 where it does
  if (sresult != result)
    flags |= V;

I get this

(gdb) print/x sresult
$15 = 0xffffffff7fffffff
(gdb) print/x result
$16 = 0x7fffffff
(gdb) print sresult != result
$17 = 1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[schwab@linux-m68k.org]

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

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
Trying to detect overflow after the fact invokes undefined behaviour.  What do
you get with -fsanitize=undefined?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[wilson at gcc dot gnu.org]

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

--- Comment #3 from Jim Wilson <wilson at gcc dot gnu.org> ---
The code does a 32-bit add and a 64-bit add and then compares the results to
detect 32-bit overflow.  That seems like it should work.

-fsanitize=undefined reports nothing.

I should mention that I'm using an Ubuntu 16.04 x86_64 machine with a system
gcc of 5.4.0.  Maybe I should try a newer system.  I can try a build on Ubuntu
20.04.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[schwab@linux-m68k.org]

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

--- Comment #4 from Andreas Schwab <schwab@linux-m68k.org> ---
It does a _signed_ add, which is undefined on overflow.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[wilson at gcc dot gnu.org]

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

--- Comment #5 from Jim Wilson <wilson at gcc dot gnu.org> ---
I realize that the 32-bit signed add has undefined result in the presence of
overflow.  But no matter what result you get, it can't match the result you get
for the 64-bit signed add when there is an overflow.  So the code should work. 
Unless maybe compiler optimization is doing something surprising which defeats
our code.

After some experimentation, I discovered that the code works with gcc-9 but
fails with gcc-10.  Because gcc-10 is doing some unexpected optimization of the
expression.

I can fix it by changing the code to
  if (sresult != (int32_t)sresult)
    flags |= V;
which should give the same result as before.  it works for me with gcc-10 on
the sim testsuite, though I haven't tried wider testing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[schwab@linux-m68k.org]

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

--- Comment #6 from Andreas Schwab <schwab@linux-m68k.org> ---
The compiler can assume that overflow does not happen and infer that the
condition compares values that are always the same.  A cast does not invoke
undefined behavior, the result is merely implementation defined when the value
does not fit.  A truely portable check would compare against INT_MIN/INT_MAX,
and the 32-bit result would have to be computed with unsigned arithmethic.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[vapier at gentoo dot org]

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

Mike Frysinger <vapier at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Host|                            |x86_64-linux-gnu
             Target|                            |aarch64-elf
              Build|                            |x86_64-linux-gnu

--- Comment #7 from Mike Frysinger <vapier at gentoo dot org> ---
sorry, i should have included my build settings.  i'm on an x86-64 host.

gcc (Gentoo 10.2.0-r5 p6) 10.2.0
GNU C Library (Gentoo 2.33 p2) release release version 2.33.
GNU ld (Gentoo 2.35.1 p2) 2.35.1
CFLAGS='-g -O2'

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jim Wilson <wilson@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0592e80bcf1bbab2fbbb110ea395f9608e4f594c

commit 0592e80bcf1bbab2fbbb110ea395f9608e4f594c
Author: Jim Wilson <jimw@sifive.com>
Date:   Wed Apr 7 18:51:52 2021 -0700

    Aarch64 sim fix for gcc-10 miscompilation.

    This fixes a problem that occurs when compiled by gcc-10, as the code
    is relying on undefined overflow behavior.  This is fixed by replacing
    compares between 32-bit and 64-bit results with compares that just use
    the 64-bit results with a cast.

            PR sim/27483
            * simulator.c (set_flags_for_add32): Compare uresult against
            itself.  Compare sresult against itself.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug sim/27483] aarch64: adds test failing when adding 0x80000000 & 0xffffffff

[wilson at gcc dot gnu.org]

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

Jim Wilson <wilson at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
           Assignee|nickc at redhat dot com            |wilson at gcc dot gnu.org

--- Comment #9 from Jim Wilson <wilson at gcc dot gnu.org> ---
I tried fixing set_flags_for_add32 to look like set_flags_for_add64, and the
simulator ends up about 1.5% slower using dhrystone as the testcase.  This is
cleaner, but seems a bit too much of a slow down, and probably explains why the
function is written this way.  With the fix I checked in, the simulator is
about 0.7% slower which is within my measurement error so a more acceptable
slow down.

-- 
You are receiving this mail because:
You are on the CC list for the bug.