[gil] gcc fail with simple code
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
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.
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.
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); } };
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...
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.
On 28 March 2018 at 21:23, Andrey Semashev via Boost <boost@lists.boost.org> 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
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 < boost@lists.boost.org> 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
participants (5)
-
Andrey Semashev -
Christian Henning -
Mateusz Loskot -
Peter Dimov -
Stefan Seefeld