From 94f5ad613ac43709222f28ca23acf8bbdd53bc55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20=C5=81oskot?= Date: Sun, 17 Jun 2018 21:52:53 +0200 Subject: [PATCH] Use promote_integral in channel_invert algorithm This should help to avoid UB due to possible signed integer overflows, for minimum/maximum of input channel domain. Fixes #89 --- include/boost/gil/channel_algorithm.hpp | 85 ++++++++++++++----------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/include/boost/gil/channel_algorithm.hpp b/include/boost/gil/channel_algorithm.hpp index 706bf210f..20cc8a78a 100644 --- a/include/boost/gil/channel_algorithm.hpp +++ b/include/boost/gil/channel_algorithm.hpp @@ -1,6 +1,6 @@ /* Copyright 2005-2007 Adobe Systems Incorporated - + Use, modification and distribution are subject to the Boost Software License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt). @@ -13,7 +13,7 @@ #define GIL_CHANNEL_ALGORITHM_HPP //////////////////////////////////////////////////////////////////////////////////////// -/// \file +/// \file /// \brief Channel algorithms /// \author Lubomir Bourdev and Hailin Jin \n /// Adobe Systems Incorporated @@ -31,6 +31,7 @@ #include "gil_config.hpp" #include "channel.hpp" +#include "promote_integral.hpp" #include "typedefs.hpp" #include @@ -90,13 +91,13 @@ struct unsigned_integral_num_bits > \brief Converting from one channel type to another \ingroup ChannelAlgorithm -Conversion is done as a simple linear mapping of one channel range to the other, +Conversion is done as a simple linear mapping of one channel range to the other, such that the minimum/maximum value of the source maps to the minimum/maximum value of the destination. One implication of this is that the value 0 of signed channels may not be preserved! When creating new channel models, it is often a good idea to provide specializations for the channel conversion algorithms, for -example, for performance optimizations. If the new model is an integral type that can be signed, it is easier to define the conversion -only for the unsigned type (\p channel_converter_unsigned) and provide specializations of \p detail::channel_convert_to_unsigned +example, for performance optimizations. If the new model is an integral type that can be signed, it is easier to define the conversion +only for the unsigned type (\p channel_converter_unsigned) and provide specializations of \p detail::channel_convert_to_unsigned and \p detail::channel_convert_from_unsigned to convert between the signed and unsigned type. Example: @@ -111,7 +112,7 @@ assert(dst_channel == 255); // max value goes to max value \endcode */ -/** +/** \defgroup ChannelConvertUnsignedAlgorithm channel_converter_unsigned \ingroup ChannelConvertAlgorithm \brief Convert one unsigned/floating point channel to another. Converts both the channel type and range @@ -144,7 +145,7 @@ struct channel_converter_unsigned_impl { typedef DstChannelV result_type; DstChannelV operator()(SrcChannelV src) const { return DstChannelV(channel_traits::min_value() + - (src - channel_traits::min_value()) / channel_range() * channel_range()); + (src - channel_traits::min_value()) / channel_range() * channel_range()); } private: template @@ -154,7 +155,7 @@ private: }; // When both the source and the destination are integral channels, perform a faster conversion -template +template struct channel_converter_unsigned_impl : public channel_converter_unsigned_integral,unsigned_integral_max_value >::value > {}; @@ -164,12 +165,12 @@ struct channel_converter_unsigned_impl //// channel_converter_unsigned_integral ////////////////////////////////////// -template +template struct channel_converter_unsigned_integral : public channel_converter_unsigned_integral_impl::value % unsigned_integral_max_value::value) > {}; -template +template struct channel_converter_unsigned_integral : public channel_converter_unsigned_integral_impl::value % unsigned_integral_max_value::value) > {}; @@ -179,24 +180,24 @@ struct channel_converter_unsigned_integral //// channel_converter_unsigned_integral_impl ////////////////////////////////////// -// Both source and destination are unsigned integral channels, +// Both source and destination are unsigned integral channels, // the src max value is less than the dst max value, // and the dst max value is divisible by the src max value -template +template struct channel_converter_unsigned_integral_impl { - DstChannelV operator()(SrcChannelV src) const { + DstChannelV operator()(SrcChannelV src) const { typedef typename unsigned_integral_max_value::value_type integer_t; static const integer_t mul = unsigned_integral_max_value::value / unsigned_integral_max_value::value; return DstChannelV(src * mul); } }; -// Both source and destination are unsigned integral channels, +// Both source and destination are unsigned integral channels, // the dst max value is less than (or equal to) the src max value, // and the src max value is divisible by the dst max value -template +template struct channel_converter_unsigned_integral_impl { - DstChannelV operator()(SrcChannelV src) const { + DstChannelV operator()(SrcChannelV src) const { typedef typename unsigned_integral_max_value::value_type integer_t; static const integer_t div = unsigned_integral_max_value::value / unsigned_integral_max_value::value; static const integer_t div2 = div/2; @@ -205,9 +206,9 @@ struct channel_converter_unsigned_integral_impl +template struct channel_converter_unsigned_integral_impl { - DstChannelV operator()(uintmax_t src) const { + DstChannelV operator()(uintmax_t src) const { static const uintmax_t div = unsigned_integral_max_value::value / unsigned_integral_max_value::value; static const uintmax_t div2 = div/2; if (src > unsigned_integral_max_value::value - div2) @@ -216,11 +217,11 @@ struct channel_converter_unsigned_integral_impl -struct channel_converter_unsigned_integral_impl +template +struct channel_converter_unsigned_integral_impl : public channel_converter_unsigned_integral_nondivisible,unsigned_integral_num_bits >, @@ -228,11 +229,11 @@ struct channel_converter_unsigned_integral_impl::value> {}; -// Both source and destination are unsigned integral channels, +// Both source and destination are unsigned integral channels, // the src max value is less than the dst max value, // and the dst max value is not divisible by the src max value // The expression (src * dst_max) / src_max fits in an integer -template +template struct channel_converter_unsigned_integral_nondivisible { DstChannelV operator()(SrcChannelV src) const { typedef typename base_channel_type::type dest_t; @@ -240,11 +241,11 @@ struct channel_converter_unsigned_integral_nondivisible +template struct channel_converter_unsigned_integral_nondivisible { DstChannelV operator()(SrcChannelV src) const { static const double mul = unsigned_integral_max_value::value / double(unsigned_integral_max_value::value); @@ -252,17 +253,17 @@ struct channel_converter_unsigned_integral_nondivisible +template struct channel_converter_unsigned_integral_nondivisible { - DstChannelV operator()(SrcChannelV src) const { + DstChannelV operator()(SrcChannelV src) const { typedef typename detail::unsigned_integral_max_value< SrcChannelV >::value_type src_integer_t; typedef typename detail::unsigned_integral_max_value< DstChannelV >::value_type dst_integer_t; - static const double div = unsigned_integral_max_value::value + static const double div = unsigned_integral_max_value::value / static_cast< double >( unsigned_integral_max_value::value ); static const src_integer_t div2 = static_cast< src_integer_t >( div / 2.0 ); @@ -325,10 +326,10 @@ template <> struct channel_converter_unsigned { } }; -/// @} +/// @} namespace detail { -// Converting from signed to unsigned integral channel. +// Converting from signed to unsigned integral channel. // It is both a unary function, and a metafunction (thus requires the 'type' nested typedef, which equals result_type) template // Model ChannelValueConcept struct channel_convert_to_unsigned : public detail::identity { @@ -416,13 +417,13 @@ struct channel_converter { /// \ingroup ChannelConvertAlgorithm /// \brief Converting from one channel type to another. template // Model ChannelConcept (could be channel references) -inline typename channel_traits::value_type channel_convert(const SrcChannel& src) { +inline typename channel_traits::value_type channel_convert(const SrcChannel& src) { return channel_converter::value_type, - typename channel_traits::value_type>()(src); + typename channel_traits::value_type>()(src); } /// \ingroup ChannelConvertAlgorithm -/// \brief Same as channel_converter, except it takes the destination channel by reference, which allows +/// \brief Same as channel_converter, except it takes the destination channel by reference, which allows /// us to move the templates from the class level to the method level. This is important when invoking it /// on heterogeneous pixels. struct default_channel_converter { @@ -506,10 +507,10 @@ struct channel_multiplier { /// \brief A function multiplying two channels. result = a * b / max_value template // Models ChannelConcept (could be a channel reference) -inline typename channel_traits::value_type channel_multiply(Channel a, Channel b) { +inline typename channel_traits::value_type channel_multiply(Channel a, Channel b) { return channel_multiplier::value_type>()(a,b); } -/// @} +/// @} /** \defgroup ChannelInvertAlgorithm channel_invert @@ -528,8 +529,16 @@ assert(inv == 0); /// \brief Default implementation. Provide overloads for performance /// \ingroup ChannelInvertAlgorithm channel_invert template // Models ChannelConcept (could be a channel reference) -inline typename channel_traits::value_type channel_invert(Channel x) { - return channel_traits::max_value()-x + channel_traits::min_value(); +inline typename channel_traits::value_type channel_invert(Channel x) { + + using base_t = typename base_channel_type::type; + using promoted_t = typename promote_integral::type; + promoted_t const promoted_x = x; + promoted_t const promoted_max = channel_traits::max_value(); + promoted_t const promoted_min = channel_traits::min_value(); + promoted_t const promoted_inverted_x = promoted_max - promoted_x + promoted_min; + auto const inverted_x = static_cast(promoted_inverted_x); + return inverted_x; } //#ifdef _MSC_VER