[Bug c/94981] New: Wrong casts on Power machines dealing with fctiwuz instruction

[Bug c/94981] New: Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

            Bug ID: 94981
           Summary: Wrong casts on Power machines dealing with fctiwuz
                    instruction
           Product: gcc
           Version: 9.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tony.reix at atos dot net
  Target Milestone: ---

There are several issues dealing with GCC 8.30 and 9.3.0 (and also 10), for C
and C++, on AIX and Fedora/PPC64LE .

This appears when casting a float/double/long-double to an unsigned type, with
different cases.
This is related to the assembly instruction fctiwuz .

Here are 3 programs showing all the cases, with a reference to Fedora/x86_64.

Also, it looks like there are missing tests in GCC.



# cat double.c

#include <stdio.h>

int main()
{
       double d = -2;
       char c = (char)d;
       signed char sc = (signed char)d;

       printf("%d\n", (int)c);
       printf("%d\n", (int)sc);

       return 0;
}


# cat vector.cc

#include <vector>
#include <iostream>

double const dbl[4] = { 10, -1, -2, 4 };

int main()
{
       double d = -2;
       char c = (char)d;

       std::cout << (int)c << std::endl;

       std::vector<char> v(&dbl[0], &dbl[4]);

       for (auto it = begin (v); it != end (v); ++it) {
               std::cout << (int)*it << std::endl;
       }

       return 0;
}


Bug is same if "char" is replaced by "unsigned short" or "unsigned int".
So, the issue seems to appear on:
 - AIX when converting a "double", a "float", or a long double"  toward:
          char, unsigned char, unsigned short, unsigned int, unsigned long
   in 32-bit and 64-bit.
 - Linux/Power when converting a "double" or a "float" toward an "unsigned"
type.




On AIX and Linux/Power.

double -2  -->  char
float  -2  -->  char

                                   C                          |                
 C++
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
                | Default   | -fsigned-char | -funsigned-char | Default   |
-fsigned-char | -funsigned-char |
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  Fedora/x86_64 | signed                                      | signed         
                            |
  9.3.1 20200408|    -2     |      -2       |      254        |    -2     |    
 -2       |       254       |
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  Fedora/Power  | signed, but: neg zeroed                     | signed, but:
neg zeroed                     |
  9.3.1 20200408|     0     |      -2       |        0        |     0     |    
 -2       |         0       | BUG
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  AIX           |                                             |                
                            |
  6.3.0         | unsigned                                    | unsigned       
                            |
                |   254     |      -2       |      254        |   254     |    
 -2       |       254       |
  8.4.0 & 9.3.0 | signed, but: neg zeroed                     | signed, but:
neg zeroed                     |
                |     0     |      -2       |        0        |     0     |    
 -2       |         0       | BUG !!
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|


long double -2  -->  char
                                   C                          |                
 C++
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
                | Default   | -fsigned-char | -funsigned-char | Default   |
-fsigned-char | -funsigned-char |
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  Fedora/x86_64 | signed                                      | signed         
                            |
  9.3.1 20200408|    -2     |      -2       |      254        |    -2     |    
 -2       |       254       |
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  Fedora/Power  | unsigned                                    | unsigned       
                            |
  9.3.1 20200408|   254     |      -2       |      254        |   254     |    
 -2       |       254       | BUG ??
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|
  AIX           |                                             |                
                            |
  6.3.0         | unsigned                                    | unsigned       
                            |
                |   254     |      -2       |      254        |   254     |    
 -2       |       254       |
  8.4.0 & 9.3.0 | signed, but: neg zeroed                     | signed, but:
neg zeroed                     |
                |     0     |      -2       |        0        |     0     |    
 -2       |         0       | BUG !
 
--------------|-----------|---------------|-----------------|-----------|---------------|-----------------|


# cat cast.c

#include <stdio.h>

int main()
{
       float f = -2;
       double d = -2;
       long double ld = -2;
       char c;
       unsigned char uc;
       unsigned short us;
       unsigned int ui;
       unsigned long ul;

       printf("             %5s %5s %5s %5s %5s\n", "c", "uc", "us", "ui",
"ul");

       c = (char)f;
       uc = (unsigned char)f;
       us = (unsigned short)f;
       ui = (unsigned int)f;
       ul = (unsigned long)f;
       printf("float:       %5d %5d %5d %5d %5d\n", (int)c, (int)uc, (int)us,
(int)ui, (int)ul);

       c = (char)d;
       uc = (unsigned char)d;
       us = (unsigned short)d;
       ui = (unsigned int)d;
       ul = (unsigned long)d;
       printf("double:      %5d %5d %5d %5d %5d\n", (int)c, (int)uc, (int)us,
(int)ui, (int)ul);

       c = (char)ld;
       uc = (unsigned char)ld;
       us = (unsigned short)ld;
       ui = (unsigned int)ld;
       ul = (unsigned long)ld;
       printf("long double: %5d %5d %5d %5d %5d\n", (int)c, (int)uc, (int)us,
(int)ui, (int)ul);

       return 0;
}


Fedora/x86_64 : OK
                 c    uc    us    ui    ul
float:          -2   254 65534    -2    -2
double:         -2   254 65534    -2    -2
long double:    -2   254 65534    -2    -2

Fedora/Power : KO
                 c    uc    us    ui    ul
float:           0     0     0     0     0
double:          0     0     0     0     0
long double:   254   254 65534    -2     0

AIX GCC 6.3.0 : OK
                 c    uc    us    ui    ul
float:         254   254 65534    -2    -2
double:        254   254 65534    -2    -2
long double:   254   254 65534    -2    -2

AIX GCC 8.4.0 & 9.3.0 32bit or 64bit : KO
                 c    uc    us    ui    ul
float:           0     0     0     0     0
double:          0     0     0     0     0
long double:     0     0     0     0     0

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
Converting a negative float to an unsigned integer is undefined.

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #2 from Tony Reix <tony.reix at atos dot net> ---
Big table was too large.
Here is a shorter table.

double -2  -->  char
float  -2  -->  char

                     C      |     C++
  --------------|-----------|-----------|
                |  Default  |  Default  |
  --------------|-----------|-----------|
  Fedora/x86_64 |        signed         |
  9.3.1 20200408|    -2     |    -2     |
  --------------|-----------|-----------|
  Fedora/Power  | signed, but neg zeroed|
  9.3.1 20200408|     0     |     0     |   BUG
  --------------|-----------|-----------|
  AIX           |                       |
  6.3.0         |       unsigned        |
                |   254     |    254    |
  8.4.0 & 9.3.0 | signed, but neg zeroed| 
                |     0     |     0     |   BUG !!
  --------------|-----------|-----------|



long double -2  -->  char

                      C     |    C++    |
  --------------|-----------|-----------|
                |  Default  |  Default  |
  --------------|-----------|-----------|
  Fedora/x86_64 |        signed         |
  9.3.1 20200408|    -2     |    -2     |
  --------------|-----------|-----------|
  Fedora/Power  |       unsigned        |
  9.3.1 20200408|   254     |   254     | BUG ??
  --------------|-----------|-----------|
  AIX           |                       | 
  6.3.0         |       unsigned        |
                |   254     |   254     | 
  8.4.0 & 9.3.0 | signed, but neg zeroed|
                |     0     |     0     | BUG !
  --------------|-----------|-----------|

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #3 from Tony Reix <tony.reix at atos dot net> ---
(In reply to Andreas Schwab from comment #1)
> Converting a negative float to an unsigned integer is undefined.

Yes, I do see some comments about this in older GCC bugs.
Like: https://gcc.gnu.org/legacy-ml/gcc/2016-02/msg00373.html

Moreover, I see that this option could help: -fsanitize=float-cast-overflow .

However, I have 4 comments:
 - GCC on Fedora/x86_64 has made a choice (not forcing a 0 for negative values)
that seems coherent. Why not makes the same choice everywhere in order to
improve the portability?
 - GCC on Fedora/Power has made different choices for: (float, double, long
double) vs (long double) against: (char, unsigned char, unsigned short,
unsigned int). This looks incoherent. And the choice made for "long double" -->
"unsigned long" looks like a true bug.
 - Since that was working differently with GCC v6.3 on AIX vs GCC v8 & 9 on
AIX, that looks like a regression.
 - Defect https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84154 deals with the
same kind of issue, appearing in GCC 7, I think. And it was accepted and fixed.
However, it seems that only 1 patch out of 2 has been pushed.

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Tony Reix from comment #3)
> (In reply to Andreas Schwab from comment #1)
> > Converting a negative float to an unsigned integer is undefined.
> 
> Yes, I do see some comments about this in older GCC bugs.
> Like: https://gcc.gnu.org/legacy-ml/gcc/2016-02/msg00373.html
> 
> Moreover, I see that this option could help: -fsanitize=float-cast-overflow .
> 
> However, I have 4 comments:
>  - GCC on Fedora/x86_64 has made a choice (not forcing a 0 for negative
> values) that seems coherent. Why not makes the same choice everywhere in
> order to improve the portability?

No, that is not a choice, that is just one of the possible values you can get,
along with all other values the type can have, formatting your disc, crashing,
anything else.
The compiler optimizes on the assumption that UB doesn't happen, just don't
invoke it.

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #5 from Tony Reix <tony.reix at atos dot net> ---
I'm not invoking myself directly this conversion. It is made by C++.

The original issue appeared when running tests of Boost v1.73 math library,
dealing with include code of C++ , line 324 of file:
include/c++/bits/stl_algobase.h (GCC v8), with  _OI  being: char* .

Thus, this was deeply hidden in C++ internal code while doing some
initialization of a vector, with:
  boost::array<double, 4> const d3a = { { 10, -6, -4, 3 } };
  polynomial<T> const a(d3a.begin(), d3a.end());
where a is built wrong silently: (10, 0, 0, 3).

The Boost line generating the issue seems to be:
std::vector<T> m_data;

That seems to mean that anyone using standard C++ code , with his own kind of
array, could face this issue silently and break some data.

include/c++/bits/stl_algobase.h :

         std::__copy_move<false, false,
std::random_access_iterator_tag>::__copy_m<double const*, char*>
         (  __first=0x10002e80 <_GLOBAL__F__ZSt4bug3i+1168>,
            __last =0x10002ea0 <_GLOBAL__F__ZSt4bug3i+1200>,
           __result=0x20001ce8 "")

 +314    template<>
 +315      struct __copy_move<false, false, random_access_iterator_tag>
 +316      {
 +317        template<typename _II, typename _OI>
 +318          static _OI
 +319          __copy_m(_II __first, _II __last, _OI __result)
 +320          {
 +321            typedef typename iterator_traits<_II>::difference_type
_Distance;
 +322            for(_Distance __n = __last - __first; __n > 0; --__n)
 +323              {
 +324                *__result = *__first;
 +325                ++__first;
 +326                ++__result;
 +327              }
 +328            return __result;
 +329          }
 +330      };

generating the assembler code:

  0x10001978 <+56>:    lwz     r9,88(r31)
  0x1000197c <+60>:    lfd     f0,0(r9)
  0x10001980 <+64>:    fctiwuz f0,f0
  0x10001984 <+68>:    addi    r9,r31,40
  0x10001988 <+72>:    stfiwx  f0,0,r9

where fctiwuz replaces f0 by 0 when contents of f0 is negative.



Complete stack is below. Showing that Boost test code is testing with "char"
type.

Does it means that:
 - C++ should check that _OI cannot be "char*" because such conversion is
undefined ?
 - or that Boost should not use the char type in that code ?


Thread 2 hit Breakpoint 1, std::__copy_move<false, false,
std::random_access_iterator_tag>::__copy_m<double const*, char*> (
    __first=0x1003f8928 <_GLOBAL__F__ZSt4bug3i+136760>, __last=0x1003f8940
<_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042931 "")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_algobase.h:324
324                   *__result = *__first;
(gdb) where
#0  std::__copy_move<false, false,
std::random_access_iterator_tag>::__copy_m<double const*, char*>
(__first=0x1003f8928 <_GLOBAL__F__ZSt4bug3i+136760>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042931 "")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_algobase.h:324
#1  0x0fffffffffffd410 in ?? ()
#2  0x0000000100195338 in std::__copy_move_a2<false, double const*, char*>
(__first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042930
"\n")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_algobase.h:422
#3  0x0000000100195278 in std::copy<double const*, char*> (__first=0x1003f8920
<_GLOBAL__F__ZSt4bug3i+136752>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042930
"\n")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_algobase.h:455
#4  0x00000001001951a8 in std::__uninitialized_copy<true>::__uninit_copy<double
const*, char*> (__first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042930
"\n")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_uninitialized.h:101
#5  0x0000000100195114 in std::uninitialized_copy<double const*, char*>
(__first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042930
"\n")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_uninitialized.h:134
#6  0x000000010019506c in std::__uninitialized_copy_a<double const*, char*,
char> (__first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>,
    __last=0x1003f8940 <_GLOBAL__F__ZSt4bug3i+136784>, __result=0x110042930
"\n")
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_uninitialized.h:289
#7  0x0000000100194db4 in std::vector<char, std::allocator<char>
>::_M_range_initialize<double const*> (this=0xfffffffffffda08,
    __first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>, __last=0x1003f8940
<_GLOBAL__F__ZSt4bug3i+136784>)
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_vector.h:1474
#8  0x0000000100194c90 in std::vector<char, std::allocator<char>
>::_M_initialize_dispatch<double const*> (this=0xfffffffffffda08,
    __first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>, __last=0x1003f8940
<_GLOBAL__F__ZSt4bug3i+136784>)
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_vector.h:1442
#9  0x0000000100194a74 in std::vector<char, std::allocator<char>
>::vector<double const*, void> (this=0xfffffffffffda08,
    __first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>, __last=0x1003f8940
<_GLOBAL__F__ZSt4bug3i+136784>, __a=...)
    at
/opt/freeware/lib/gcc/powerpc-ibm-aix7.2.0.0/8/include/c++/bits/stl_vector.h:551
#10 0x0000000100194930 in
boost::math::tools::polynomial<char>::polynomial<double const*>
(this=0xfffffffffffda08,
    first=0x1003f8920 <_GLOBAL__F__ZSt4bug3i+136752>, last=0x1003f8940
<_GLOBAL__F__ZSt4bug3i+136784>) at ../boost/math/tools/polynomial.hpp:304
#11 0x00000001001d2f60 in test_multiplication<char>::test_method
(this=0xfffffffffffe3e0) at ../libs/math/test/test_polynomial.cpp:417
#12 0x00000001001d2a5c in test_multiplication_invoker::run<char> () at
../libs/math/test/test_polynomial.cpp:415
#13 0x00000001001d2640 in
boost::unit_test::ut_detail::test_case_template_invoker<test_multiplication_invoker,
char>::operator() (this=0x110045a90)
    at ../boost/test/tree/test_case_template.hpp:70
#14 0x00000001001d2540 in
boost::detail::function::void_function_obj_invoker0<boost::unit_test::ut_detail::test_case_template_invoker<test_multiplication_invoker,
char>, void>::invoke (function_obj_ptr=...) at
../boost/function/function_template.hpp:158

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Report the bug to boost then.  BOOST is broken and should not depend on
undefined behavior for out of range conversions.

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #7 from Tony Reix <tony.reix at atos dot net> ---
(In reply to Andreas Schwab from comment #1)
> Converting a negative float to an unsigned integer is undefined.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf says (page 43):
If the value of the integral part cannot be represented by the integer type,
the behavior is undefined.50)
50) The remaindering operation performed when a value of integer type is
converted to unsigned type need not be performed when a value of real floating
type is converted to unsigned type. Thus, the range of portable real floating
values is (−1, Utype_MAX+1).

Hummmmmmm

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[tony.reix at atos dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #8 from Tony Reix <tony.reix at atos dot net> ---
On fedora31/x86_64, when compiling with -funsigned-char, some parts of Boost
math do not compile. With some changes like: static_cast<T>(-5) , it is easy to
make these tests to work both with -fsigned-char or -funsigned-char.
However, there are several other places where Boost expects chars to be signed
and that would require to change them too. Moreover, I have no idea yet if such
a change does modify the semantic of this Boost math code, or not. Moreover,
Boost makes use of C++ include files which have a different behavior on AIX
when converting floating point numbers to unsigned ints, vs Fedora. Thus, I
should discuss with Boost community and check if Boost is designed to handle
unsigned chars in addition to signed char, but some cases were not correctly
handled, or if Boost requires signed chars.

[Bug c/94981] Wrong casts on Power machines dealing with fctiwuz instruction

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94981

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I should note aarch64 and arm both use unsigned char as the default too.