Bug 94981 - Wrong casts on Power machines dealing with fctiwuz instruction
Summary: Wrong casts on Power machines dealing with fctiwuz instruction
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 9.3.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-07 10:17 UTC by Tony Reix
Modified: 2020-05-11 09:14 UTC (History)
1 user (show)

See Also:
Host:
Target: powerpc
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Reix 2020-05-07 10:17:49 UTC
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
Comment 1 Andreas Schwab 2020-05-07 10:41:27 UTC
Converting a negative float to an unsigned integer is undefined.
Comment 2 Tony Reix 2020-05-07 11:40:42 UTC
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 !
  --------------|-----------|-----------|
Comment 3 Tony Reix 2020-05-07 12:05:11 UTC
(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.
Comment 4 Jakub Jelinek 2020-05-07 12:14:47 UTC
(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.
Comment 5 Tony Reix 2020-05-07 12:39:03 UTC
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
Comment 6 Andrew Pinski 2020-05-07 13:07:09 UTC
Report the bug to boost then.  BOOST is broken and should not depend on undefined behavior for out of range conversions.
Comment 7 Tony Reix 2020-05-07 15:46:02 UTC
(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
Comment 8 Tony Reix 2020-05-11 08:52:36 UTC
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.
Comment 9 Andrew Pinski 2020-05-11 09:14:56 UTC
I should note aarch64 and arm both use unsigned char as the default too.