[gil] gcc fail with simple code

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[gil] gcc fail with simple code

Boost - Dev mailing list
Hi all,

we are, at the boost::gil corner, investigating a runtime failure when
compiling with gcc 5.4. When running in release configuration the following
code fails:

#include <iostream>
#include <boost/gil/gil_all.hpp>

using namespace boost::gil;
using namespace std;

void error_if(bool condition) {
    if (condition)
        throw std::exception();
}

bits32s c32s_min = channel_traits<bits32s>::min_value();
bits32s c32s_max = channel_traits<bits32s>::max_value();

// For channel values simply initialize the value directly
template <typename ChannelValue>
struct value_core {
    typedef ChannelValue channel_t;
    channel_t _min_v, _max_v;

    value_core() : _min_v(channel_traits<ChannelValue>::min_value()),
_max_v(channel_traits<ChannelValue>::max_value())
    {}
};

int main(int argc, char *argv[])
{
    value_core<bits32s> a;

    bits32s v_min, v_max;

    v_min = channel_convert<bits32s>(c32s_min);
    v_max = channel_convert<bits32s>(c32s_max);

    //std::cout << v_min << std::endl;
    // std::cout << this->_min_v << std::endl;
    //std::cout << v_max << std::endl;
    // std::cout << this->_max_v << std::endl;

    error_if(v_min != a._min_v || v_max != a._max_v);
}

The interesting thing is that everything works with the cout's enabled.

Has anyone ever dealt with a similar issue?

Thanks,
Christian

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
On 03/28/18 20:30, Christian Henning via Boost wrote:

> Hi all,
>
> we are, at the boost::gil corner, investigating a runtime failure when
> compiling with gcc 5.4. When running in release configuration the following
> code fails:
>
> #include <iostream>
> #include <boost/gil/gil_all.hpp>
>
> using namespace boost::gil;
> using namespace std;
>
> void error_if(bool condition) {
>      if (condition)
>          throw std::exception();
> }
>
> bits32s c32s_min = channel_traits<bits32s>::min_value();
> bits32s c32s_max = channel_traits<bits32s>::max_value();
>
> // For channel values simply initialize the value directly
> template <typename ChannelValue>
> struct value_core {
>      typedef ChannelValue channel_t;
>      channel_t _min_v, _max_v;
>
>      value_core() : _min_v(channel_traits<ChannelValue>::min_value()),
> _max_v(channel_traits<ChannelValue>::max_value())
>      {}
> };
>
> int main(int argc, char *argv[])
> {
>      value_core<bits32s> a;
>
>      bits32s v_min, v_max;
>
>      v_min = channel_convert<bits32s>(c32s_min);
>      v_max = channel_convert<bits32s>(c32s_max);
>
>      //std::cout << v_min << std::endl;
>      // std::cout << this->_min_v << std::endl;
>      //std::cout << v_max << std::endl;
>      // std::cout << this->_max_v << std::endl;
>
>      error_if(v_min != a._min_v || v_max != a._max_v);
> }
>
> The interesting thing is that everything works with the cout's enabled.
>
> Has anyone ever dealt with a similar issue?

Most likely, you have a signed integer overflow somewhere. The code
crashes on gcc 7.2 with -O3 but doesn't crash with -O0 or with -O3 -fwrapv.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
On 03/28/18 20:52, Andrey Semashev wrote:

> On 03/28/18 20:30, Christian Henning via Boost wrote:
>> Hi all,
>>
>> we are, at the boost::gil corner, investigating a runtime failure when
>> compiling with gcc 5.4. When running in release configuration the
>> following
>> code fails:
>>
>> #include <iostream>
>> #include <boost/gil/gil_all.hpp>
>>
>> using namespace boost::gil;
>> using namespace std;
>>
>> void error_if(bool condition) {
>>      if (condition)
>>          throw std::exception();
>> }
>>
>> bits32s c32s_min = channel_traits<bits32s>::min_value();
>> bits32s c32s_max = channel_traits<bits32s>::max_value();
>>
>> // For channel values simply initialize the value directly
>> template <typename ChannelValue>
>> struct value_core {
>>      typedef ChannelValue channel_t;
>>      channel_t _min_v, _max_v;
>>
>>      value_core() : _min_v(channel_traits<ChannelValue>::min_value()),
>> _max_v(channel_traits<ChannelValue>::max_value())
>>      {}
>> };
>>
>> int main(int argc, char *argv[])
>> {
>>      value_core<bits32s> a;
>>
>>      bits32s v_min, v_max;
>>
>>      v_min = channel_convert<bits32s>(c32s_min);
>>      v_max = channel_convert<bits32s>(c32s_max);
>>
>>      //std::cout << v_min << std::endl;
>>      // std::cout << this->_min_v << std::endl;
>>      //std::cout << v_max << std::endl;
>>      // std::cout << this->_max_v << std::endl;
>>
>>      error_if(v_min != a._min_v || v_max != a._max_v);
>> }
>>
>> The interesting thing is that everything works with the cout's enabled.
>>
>> Has anyone ever dealt with a similar issue?
>
> Most likely, you have a signed integer overflow somewhere. The code
> crashes on gcc 7.2 with -O3 but doesn't crash with -O0 or with -O3 -fwrapv.

With -ftrapv the code crashes at this point:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff7490f5d in __GI_abort () at abort.c:90
#2  0x00007ffff783b2d8 in __addvsi3 () from
/lib/x86_64-linux-gnu/libgcc_s.so.1
#3  0x00005555555548a9 in
boost::gil::detail::channel_convert_to_unsigned<int>::operator()
(this=<optimized out>, x=<optimized out>) at
./boost/gil/channel_algorithm.hpp:332
#4  boost::gil::channel_converter<int, int>::operator() (this=<optimized
out>, src=@0x555555755014: 2147483647) at
./boost/gil/channel_algorithm.hpp:368
#5  boost::gil::channel_convert<int, int> (src=@0x555555755014:
2147483647) at ./boost/gil/channel_algorithm.hpp:377
#6  main (argc=<optimized out>, argv=<optimized out>) at test_gil.cpp:32

channel_algorithm.hpp:332 is this line:

     type operator()(bits32s x) const { return
static_cast<bits32>(x+(1<<31)); }

This shift is UB because it overflows (until C++17, I think?). The
addition will also overflow unless x is 0.

I would suggest casting to unsigned first and then performing the math.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
Thank you Andrey!

With -ftrapv the code crashes at this point:

>
> #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1  0x00007ffff7490f5d in __GI_abort () at abort.c:90
> #2  0x00007ffff783b2d8 in __addvsi3 () from /lib/x86_64-linux-gnu/libgcc_s
> .so.1
> #3  0x00005555555548a9 in boost::gil::detail::channel_co
> nvert_to_unsigned<int>::operator() (this=<optimized out>, x=<optimized
> out>) at ./boost/gil/channel_algorithm.hpp:332
> #4  boost::gil::channel_converter<int, int>::operator() (this=<optimized
> out>, src=@0x555555755014: 2147483647) at ./boost/gil/channel_algorithm.
> hpp:368
> #5  boost::gil::channel_convert<int, int> (src=@0x555555755014: 2147483647)
> at ./boost/gil/channel_algorithm.hpp:377
> #6  main (argc=<optimized out>, argv=<optimized out>) at test_gil.cpp:32
>
> channel_algorithm.hpp:332 is this line:
>
>     type operator()(bits32s x) const { return
> static_cast<bits32>(x+(1<<31)); }
>
> This shift is UB because it overflows (until C++17, I think?). The
> addition will also overflow unless x is 0.
>
> I would suggest casting to unsigned first and then performing the math.



I assume this is what you meant:

template <> struct channel_convert_to_unsigned<bits32s> : public
std::unary_function<bits32s,bits32> {
    typedef bits32 type;
    type operator()(bits32s x) const
    {
        uint32_t a = static_cast<uint32_t>(x);
        a += (1 << 31);

        return static_cast<bits32>(a);
    }
};

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
On 28.03.2018 14:48, Christian Henning via Boost wrote:

> Thank you Andrey!
>
> With -ftrapv the code crashes at this point:
>> #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
>> #1  0x00007ffff7490f5d in __GI_abort () at abort.c:90
>> #2  0x00007ffff783b2d8 in __addvsi3 () from /lib/x86_64-linux-gnu/libgcc_s
>> .so.1
>> #3  0x00005555555548a9 in boost::gil::detail::channel_co
>> nvert_to_unsigned<int>::operator() (this=<optimized out>, x=<optimized
>> out>) at ./boost/gil/channel_algorithm.hpp:332
>> #4  boost::gil::channel_converter<int, int>::operator() (this=<optimized
>> out>, src=@0x555555755014: 2147483647) at ./boost/gil/channel_algorithm.
>> hpp:368
>> #5  boost::gil::channel_convert<int, int> (src=@0x555555755014: 2147483647)
>> at ./boost/gil/channel_algorithm.hpp:377
>> #6  main (argc=<optimized out>, argv=<optimized out>) at test_gil.cpp:32
>>
>> channel_algorithm.hpp:332 is this line:
>>
>>     type operator()(bits32s x) const { return
>> static_cast<bits32>(x+(1<<31)); }
>>
>> This shift is UB because it overflows (until C++17, I think?). The
>> addition will also overflow unless x is 0.
>>
>> I would suggest casting to unsigned first and then performing the math.
>
>
> I assume this is what you meant:
>
> template <> struct channel_convert_to_unsigned<bits32s> : public
> std::unary_function<bits32s,bits32> {
>     typedef bits32 type;
>     type operator()(bits32s x) const
>     {
>         uint32_t a = static_cast<uint32_t>(x);
>         a += (1 << 31);
>
>         return static_cast<bits32>(a);
>     }
> };

Or simply replace `static_cast<bits32>(x+(1<<31))` by
`static_cast<bits32>(x)+(1<<31)` (i.e., cast `x` rather than the full
expression).

Stefan

--

      ...ich hab' noch einen Koffer in Berlin...
   


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
On 03/28/18 22:10, Stefan Seefeld via Boost wrote:

> On 28.03.2018 14:48, Christian Henning via Boost wrote:
>>>
>>> I would suggest casting to unsigned first and then performing the math.
>>
>>
>> I assume this is what you meant:
>>
>> template <> struct channel_convert_to_unsigned<bits32s> : public
>> std::unary_function<bits32s,bits32> {
>>      typedef bits32 type;
>>      type operator()(bits32s x) const
>>      {
>>          uint32_t a = static_cast<uint32_t>(x);
>>          a += (1 << 31);
>>
>>          return static_cast<bits32>(a);
>>      }
>> };
>
> Or simply replace `static_cast<bits32>(x+(1<<31))` by
> `static_cast<bits32>(x)+(1<<31)` (i.e., cast `x` rather than the full
> expression).

Rather:

   static_cast<bits32>(x)+(static_cast<bits32>(1)<<31)

to make it compatible with older C++ versions.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
On 28 March 2018 at 21:23, Andrey Semashev via Boost
<[hidden email]> wrote:

> On 03/28/18 22:10, Stefan Seefeld via Boost wrote:
>> On 28.03.2018 14:48, Christian Henning via Boost wrote:
>>>>
>>>> I would suggest casting to unsigned first and then performing the math.
>>>
>>> I assume this is what you meant:
>>>
>>> template <> struct channel_convert_to_unsigned<bits32s> : public
>>> std::unary_function<bits32s,bits32> {
>>>      typedef bits32 type;
>>>      type operator()(bits32s x) const
>>>      {
>>>          uint32_t a = static_cast<uint32_t>(x);
>>>          a += (1 << 31);
>>>
>>>          return static_cast<bits32>(a);
>>>      }
>>> };
>>
>>
>> Or simply replace `static_cast<bits32>(x+(1<<31))` by
>> `static_cast<bits32>(x)+(1<<31)` (i.e., cast `x` rather than the full
>> expression).
>
>
> Rather:
>
>   static_cast<bits32>(x)+(static_cast<bits32>(1)<<31)
>
> to make it compatible with older C++ versions.


I think Andrey's version also sticks out and documents the danger better,
than 'cosmetic' shuffle of one bracket.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Christian Henning wrote:

>        a += (1 << 31);

1u << 31 to be on the safe side; 1 << 31 (shifting into the sign bit) is
undefined by itself. (It was accidentally made defined by C++17 but will
probably revert to undefined.)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] gcc fail with simple code

Boost - Dev mailing list
Thanks Peter!

Our fix is based on Andrey's suggestions. So we are good.

type operator()(bits32s x) const { return
static_cast<bits32>(x)+(static_cast<bits32>(1)<<31); }




On Wed, Mar 28, 2018 at 3:47 PM, Peter Dimov via Boost <
[hidden email]> wrote:

> Christian Henning wrote:
>
>        a += (1 << 31);
>>
>
> 1u << 31 to be on the safe side; 1 << 31 (shifting into the sign bit) is
> undefined by itself. (It was accidentally made defined by C++17 but will
> probably revert to undefined.)
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman
> /listinfo.cgi/boost
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost