diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index 6014809661..a3ee03f4ba 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -165,6 +165,7 @@ rtc_static_library("rtc_base_approved") { "safe_compare.h", "safe_conversions.h", "safe_conversions_impl.h", + "safe_minmax.h", "sanitizer.h", "scoped_ref_ptr.h", "string_to_number.cc", @@ -813,6 +814,7 @@ if (rtc_include_tests) { "ratetracker_unittest.cc", "refcountedobject_unittest.cc", "safe_compare_unittest.cc", + "safe_minmax_unittest.cc", "string_to_number_unittest.cc", "stringencode_unittest.cc", "stringutils_unittest.cc", diff --git a/webrtc/base/safe_minmax.h b/webrtc/base/safe_minmax.h new file mode 100644 index 0000000000..8d1fc8776f --- /dev/null +++ b/webrtc/base/safe_minmax.h @@ -0,0 +1,188 @@ +/* + * Copyright 2017 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Minimum and maximum +// =================== +// +// rtc::SafeMin(x, y) +// rtc::SafeMax(x, y) +// +// Accept two arguments of either any two integral or any two floating-point +// types, and return the smaller and larger value, respectively, with no +// truncation or wrap-around. If only one of the input types is statically +// guaranteed to be able to represent the result, the return type is that type; +// if either one would do, the result type is the smaller type. (One of these +// two cases always applies.) +// +// (The case with one floating-point and one integral type is not allowed, +// because the floating-point type will have greater range, but may not have +// sufficient precision to represent the integer value exactly.) +// +// Requesting a specific return type +// ================================= +// +// Both functions allow callers to explicitly specify the return type as a +// template parameter, overriding the default return type. E.g. +// +// rtc::SafeMin(x, y) // returns an int +// +// If the requested type is statically guaranteed to be able to represent the +// result, then everything's fine, and the return type is as requested. But if +// the requested type is too small, a static_assert is triggered. + +#ifndef WEBRTC_BASE_SAFE_MINMAX_H_ +#define WEBRTC_BASE_SAFE_MINMAX_H_ + +#include +#include + +#include "webrtc/base/checks.h" +#include "webrtc/base/safe_compare.h" +#include "webrtc/base/type_traits.h" + +namespace rtc { + +namespace safe_minmax_impl { + +// Make the range of a type available via something other than a constexpr +// function, to work around MSVC limitations. See +// https://blogs.msdn.microsoft.com/vcblog/2015/12/02/partial-support-for-expression-sfinae-in-vs-2015-update-1/ +template +struct Limits { + static constexpr T lowest = std::numeric_limits::lowest(); + static constexpr T max = std::numeric_limits::max(); +}; + +template ::value> +struct UnderlyingType; + +template +struct UnderlyingType { + using type = T; +}; + +template +struct UnderlyingType { + using type = typename std::underlying_type::type; +}; + +// Given two types T1 and T2, find types that can hold the smallest (in +// ::min_t) and the largest (in ::max_t) of the two values. +template ::value, + bool int2 = IsIntlike::value> +struct MType { + static_assert(int1 == int2, + "You may not mix integral and floating-point arguments"); +}; + +// Specialization for when neither type is integral (and therefore presumably +// floating-point). +template +struct MType { + using min_t = typename std::common_type::type; + static_assert(std::is_same::value || + std::is_same::value, + ""); + + using max_t = typename std::common_type::type; + static_assert(std::is_same::value || + std::is_same::value, + ""); +}; + +// Specialization for when both types are integral. +template +struct MType { + // The type with the lowest minimum value. In case of a tie, the type with + // the lowest maximum value. In case that too is a tie, the types have the + // same range, and we arbitrarily pick T1. + using min_t = typename std::conditional< + safe_cmp::Lt(Limits::lowest, Limits::lowest), + T1, + typename std::conditional< + safe_cmp::Gt(Limits::lowest, Limits::lowest), + T2, + typename std::conditional::max, + Limits::max), + T1, + T2>::type>::type>::type; + static_assert(std::is_same::value || + std::is_same::value, + ""); + + // The type with the highest maximum value. In case of a tie, the types have + // the same range (because in C++, integer types with the same maximum also + // have the same minimum). + static_assert(safe_cmp::Ne(Limits::max, Limits::max) || + safe_cmp::Eq(Limits::lowest, Limits::lowest), + "integer types with the same max should have the same min"); + using max_t = typename std:: + conditional::max, Limits::max), T1, T2>::type; + static_assert(std::is_same::value || + std::is_same::value, + ""); +}; + +// A dummy type that we pass around at compile time but never actually use. +// Declared but not defined. +struct DefaultType; + +// ::type is A, except we fall back to B if A is DefaultType. We static_assert +// that the chosen type can hold all values that B can hold. +template +struct TypeOr { + using type = typename std:: + conditional::value, B, A>::type; + static_assert(safe_cmp::Le(Limits::lowest, Limits::lowest) && + safe_cmp::Ge(Limits::max, Limits::max), + "The specified type isn't large enough"); + static_assert(IsIntlike::value == IsIntlike::value && + std::is_floating_point::value == + std::is_floating_point::value, + "float<->int conversions not allowed"); +}; + +} // namespace safe_minmax_impl + +template ::min_t>::type>::type> +constexpr R2 SafeMin(T1 a, T2 b) { + static_assert(IsIntlike::value || std::is_floating_point::value, + "The first argument must be integral or floating-point"); + static_assert(IsIntlike::value || std::is_floating_point::value, + "The second argument must be integral or floating-point"); + return safe_cmp::Lt(a, b) ? static_cast(a) : static_cast(b); +} + +template ::max_t>::type>::type> +constexpr R2 SafeMax(T1 a, T2 b) { + static_assert(IsIntlike::value || std::is_floating_point::value, + "The first argument must be integral or floating-point"); + static_assert(IsIntlike::value || std::is_floating_point::value, + "The second argument must be integral or floating-point"); + return safe_cmp::Gt(a, b) ? static_cast(a) : static_cast(b); +} + +} // namespace rtc + +#endif // WEBRTC_BASE_SAFE_MINMAX_H_ diff --git a/webrtc/base/safe_minmax_unittest.cc b/webrtc/base/safe_minmax_unittest.cc new file mode 100644 index 0000000000..6fcbf00332 --- /dev/null +++ b/webrtc/base/safe_minmax_unittest.cc @@ -0,0 +1,141 @@ +/* + * Copyright 2017 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include + +#include "webrtc/base/safe_minmax.h" +#include "webrtc/test/gtest.h" + +namespace rtc { + +namespace { + +// Functions that check that SafeMin() and SafeMax() return the specified type. +// The functions that end in "R" use an explicitly given return type. + +template +constexpr bool TypeCheckMinMax() { + return std::is_same(), std::declval())), + Tmin>::value && + std::is_same(), std::declval())), + Tmax>::value; +} + +template +constexpr bool TypeCheckMinR() { + return std::is_same< + decltype(SafeMin(std::declval(), std::declval())), R>::value; +} + +template +constexpr bool TypeCheckMaxR() { + return std::is_same< + decltype(SafeMax(std::declval(), std::declval())), R>::value; +} + +// clang-format off + +// SafeMin/SafeMax: Check that all combinations of signed/unsigned 8/64 bits +// give the correct default result type. +static_assert(TypeCheckMinMax< int8_t, int8_t, int8_t, int8_t>(), ""); +static_assert(TypeCheckMinMax< int8_t, uint8_t, int8_t, uint8_t>(), ""); +static_assert(TypeCheckMinMax< int8_t, int64_t, int64_t, int64_t>(), ""); +static_assert(TypeCheckMinMax< int8_t, uint64_t, int8_t, uint64_t>(), ""); +static_assert(TypeCheckMinMax< uint8_t, int8_t, int8_t, uint8_t>(), ""); +static_assert(TypeCheckMinMax< uint8_t, uint8_t, uint8_t, uint8_t>(), ""); +static_assert(TypeCheckMinMax< uint8_t, int64_t, int64_t, int64_t>(), ""); +static_assert(TypeCheckMinMax< uint8_t, uint64_t, uint8_t, uint64_t>(), ""); +static_assert(TypeCheckMinMax< int64_t, int8_t, int64_t, int64_t>(), ""); +static_assert(TypeCheckMinMax< int64_t, uint8_t, int64_t, int64_t>(), ""); +static_assert(TypeCheckMinMax< int64_t, int64_t, int64_t, int64_t>(), ""); +static_assert(TypeCheckMinMax< int64_t, uint64_t, int64_t, uint64_t>(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); + +// SafeMin/SafeMax: Check that we can use enum types. +enum DefaultE { kFoo = -17 }; +enum UInt8E : uint8_t { kBar = 17 }; +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); + +using ld = long double; + +// SafeMin/SafeMax: Check that all floating-point combinations give the +// correct result type. +static_assert(TypeCheckMinMax< float, float, float, float>(), ""); +static_assert(TypeCheckMinMax< float, double, double, double>(), ""); +static_assert(TypeCheckMinMax< float, ld, ld, ld>(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax(), ""); +static_assert(TypeCheckMinMax< ld, float, ld, ld>(), ""); +static_assert(TypeCheckMinMax< ld, double, ld, ld>(), ""); +static_assert(TypeCheckMinMax< ld, ld, ld, ld>(), ""); + +// clang-format on + +// SafeMin/SafeMax: Check some cases of explicitly specified return type. The +// commented-out lines give compilation errors due to the requested return type +// being too small or requiring an int<->float conversion. +static_assert(TypeCheckMinR(), ""); +// static_assert(TypeCheckMinR(), ""); +static_assert(TypeCheckMinR(), ""); +// static_assert(TypeCheckMaxR(), ""); +// static_assert(TypeCheckMaxR(), ""); +static_assert(TypeCheckMaxR(), ""); +// static_assert(TypeCheckMaxR(), ""); + +template +constexpr bool CheckMinMax(T1 a, T2 b, Tmin min, Tmax max) { + return TypeCheckMinMax() && SafeMin(a, b) == min && + SafeMax(a, b) == max; +} + +// SafeMin/SafeMax: Check a few values. +static_assert(CheckMinMax(int8_t{1}, int8_t{-1}, int8_t{-1}, int8_t{1}), ""); +static_assert(CheckMinMax(uint8_t{1}, int8_t{-1}, int8_t{-1}, uint8_t{1}), ""); +static_assert(CheckMinMax(uint8_t{5}, uint64_t{2}, uint8_t{2}, uint64_t{5}), + ""); +static_assert(CheckMinMax(std::numeric_limits::min(), + std::numeric_limits::max(), + std::numeric_limits::min(), + std::numeric_limits::max()), + ""); +static_assert(CheckMinMax(std::numeric_limits::min(), + std::numeric_limits::max(), + std::numeric_limits::min(), + int32_t{std::numeric_limits::max()}), + ""); +// static_assert(CheckMinMax(1.f, 2, 1.f, 2.f), ""); +static_assert(CheckMinMax(1.f, 0.0, 0.0, 1.0), ""); + +} // namespace + +// clang-format off + +// These functions aren't used in the tests, but it's useful to look at the +// compiler output for them, and verify that (1) the same-signedness *Safe +// functions result in exactly the same code as their *Ref counterparts, and +// that (2) the mixed-signedness *Safe functions have just a few extra +// arithmetic and logic instructions (but no extra control flow instructions). +int32_t TestMinRef( int32_t a, int32_t b) { return std::min(a, b); } +uint32_t TestMinRef( uint32_t a, uint32_t b) { return std::min(a, b); } +int32_t TestMinSafe( int32_t a, int32_t b) { return SafeMin(a, b); } +int32_t TestMinSafe( int32_t a, uint32_t b) { return SafeMin(a, b); } +int32_t TestMinSafe(uint32_t a, int32_t b) { return SafeMin(a, b); } +uint32_t TestMinSafe(uint32_t a, uint32_t b) { return SafeMin(a, b); } + +// clang-format on + +} // namespace rtc diff --git a/webrtc/common_video/h264/sps_vui_rewriter.cc b/webrtc/common_video/h264/sps_vui_rewriter.cc index 2e118d5233..534e75c865 100644 --- a/webrtc/common_video/h264/sps_vui_rewriter.cc +++ b/webrtc/common_video/h264/sps_vui_rewriter.cc @@ -18,6 +18,7 @@ #include "webrtc/base/bitbuffer.h" #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" +#include "webrtc/base/safe_minmax.h" #include "webrtc/common_video/h264/h264_common.h" #include "webrtc/common_video/h264/sps_parser.h" @@ -351,8 +352,7 @@ bool CopyRemainingBits(rtc::BitBuffer* source, COPY_BITS(source, destination, bits_tmp, misaligned_bits); } while (source->RemainingBitCount() > 0) { - size_t count = std::min(static_cast(32u), - static_cast(source->RemainingBitCount())); + auto count = rtc::SafeMin(32u, source->RemainingBitCount()); COPY_BITS(source, destination, bits_tmp, count); } // TODO(noahric): The last byte could be all zeroes now, which we should just diff --git a/webrtc/modules/audio_coding/neteq/merge.cc b/webrtc/modules/audio_coding/neteq/merge.cc index 299682f60d..d50b66588a 100644 --- a/webrtc/modules/audio_coding/neteq/merge.cc +++ b/webrtc/modules/audio_coding/neteq/merge.cc @@ -16,6 +16,8 @@ #include // min, max #include +#include "webrtc/base/safe_conversions.h" +#include "webrtc/base/safe_minmax.h" #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" #include "webrtc/modules/audio_coding/neteq/audio_multi_vector.h" #include "webrtc/modules/audio_coding/neteq/cross_correlation.h" @@ -209,8 +211,8 @@ size_t Merge::GetExpandedSignal(size_t* old_length, size_t* expand_period) { int16_t Merge::SignalScaling(const int16_t* input, size_t input_length, const int16_t* expanded_signal) const { // Adjust muting factor if new vector is more or less of the BGN energy. - const size_t mod_input_length = - std::min(static_cast(64 * fs_mult_), input_length); + const auto mod_input_length = rtc::SafeMin( + 64 * rtc::dchecked_cast(fs_mult_), input_length); const int16_t expanded_max = WebRtcSpl_MaxAbsValueW16(expanded_signal, mod_input_length); int32_t factor = (expanded_max * expanded_max) / diff --git a/webrtc/modules/audio_processing/audio_processing_unittest.cc b/webrtc/modules/audio_processing/audio_processing_unittest.cc index 814eea9967..c56a57b4c3 100644 --- a/webrtc/modules/audio_processing/audio_processing_unittest.cc +++ b/webrtc/modules/audio_processing/audio_processing_unittest.cc @@ -21,6 +21,7 @@ #include "webrtc/base/gtest_prod_util.h" #include "webrtc/base/ignore_wundef.h" #include "webrtc/base/protobuf_utils.h" +#include "webrtc/base/safe_minmax.h" #include "webrtc/common_audio/include/audio_util.h" #include "webrtc/common_audio/resampler/include/push_resampler.h" #include "webrtc/common_audio/resampler/push_sinc_resampler.h" @@ -678,7 +679,7 @@ void ApmTest::ProcessDelayVerificationTest(int delay_ms, int system_delay_ms, // Calculate expected delay estimate and acceptable regions. Further, // limit them w.r.t. AEC delay estimation support. const size_t samples_per_ms = - std::min(static_cast(16), frame_->samples_per_channel_ / 10); + rtc::SafeMin(16u, frame_->samples_per_channel_ / 10); int expected_median = std::min(std::max(delay_ms - system_delay_ms, delay_min), delay_max); int expected_median_high = std::min( diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc index 71415c77a8..e645090b35 100644 --- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -15,6 +15,7 @@ #include #include "webrtc/base/checks.h" +#include "webrtc/base/safe_minmax.h" #include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" @@ -250,8 +251,8 @@ uint32_t AimdRateControl::MultiplicativeRateIncrease( int64_t now_ms, int64_t last_ms, uint32_t current_bitrate_bps) const { double alpha = 1.08; if (last_ms > -1) { - int time_since_last_update_ms = std::min(static_cast(now_ms - last_ms), - 1000); + auto time_since_last_update_ms = + rtc::SafeMin(now_ms - last_ms, 1000); alpha = pow(alpha, time_since_last_update_ms / 1000.0); } uint32_t multiplicative_increase_bps = std::max(