Clang ToT builds failing with new -Winteger-overflow warnings |
||||
Issue descriptionAll the bots are red. Example from https://ci.chromium.org/buildbot/chromium.clang/ToTLinux/1964: FAILED: obj/base/base_unittests/safe_numerics_unittest.o ../../base/safe_numerics_unittest.cc:965:51: error: overflow in expression; result is -131071 with type 'int' [-Werror,-Winteger-overflow] Dst(SrcLimits::max()) * SrcLimits::max()); ^ ../../base/safe_numerics_unittest.cc:982:57: error: overflow in expression; result is -131071 with type 'int' [-Werror,-Winteger-overflow] TEST_EXPECTED_RANGE(RANGE_VALID, SrcLimits::max() * static_cast<Src>(-1)); ^ ../../base/safe_numerics_unittest.cc:965:51: error: overflow in expression; result is 1 with type 'int' [-Werror,-Winteger-overflow] Dst(SrcLimits::max()) * SrcLimits::max()); ^ ../../base/safe_numerics_unittest.cc:965:51: error: overflow in expression; result is 1 with type 'long' [-Werror,-Winteger-overflow] ../../base/safe_numerics_unittest.cc:965:51: error: overflow in expression; result is -8589934591 with type 'long' [-Werror,-Winteger-overflow] 5 errors generated. This is most likely due to r328671 which landed last night.
,
Mar 28 2018
https://chromium-review.googlesource.com/c/chromium/src/+/984876 is a CL to address this.
,
Mar 28 2018
If I understand correctly, both these warnings are in dead code, so they are in a sense false positives. line 965 and line 982 are guarded by checks that are meant to check if the code will overflow at runtime:
if (MaxExponent<Dst>::value >= MaxExponent<Src>::value * 2 - 1) {
// At least twice larger type.
TEST_EXPECTED_SUCCESS(SrcLimits::max() * checked_dst);
TEST_EXPECTED_VALUE(SrcLimits::max() * clamped_dst,
Dst(SrcLimits::max()) * SrcLimits::max());
} else { // Larger, but not at least twice as large.
TEST_EXPECTED_FAILURE(SrcLimits::max() * checked_dst);
...
if (SrcLimits::is_iec559) {
TEST_EXPECTED_RANGE(RANGE_VALID, SrcLimits::max() * static_cast<Src>(-1));
TEST_EXPECTED_RANGE(RANGE_OVERFLOW, SrcLimits::infinity());
TEST_EXPECTED_RANGE(RANGE_UNDERFLOW, SrcLimits::infinity() * -1);
TEST_EXPECTED_RANGE(RANGE_INVALID, SrcLimits::quiet_NaN());
} else if (numeric_limits<Src>::is_signed) {
My understanding is that 'int' is not "iec559": negating INT_MAX will not be representable.
,
Mar 28 2018
,
Mar 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c1339ee7eb5eb1090699661e938b63f595b00ca commit 2c1339ee7eb5eb1090699661e938b63f595b00ca Author: Mostyn Bramley-Moore <mostynb@vewd.com> Date: Wed Mar 28 21:10:04 2018 disable clang integer overflow warnings for safe_numerics unittests Some of these tests intentionally cause overflow, and it seems that clang ToT is now able to recognise and warn about this. Let's disable these warnings as we do for MSVC. Bug: 826656 Change-Id: I7a455e4d5b901c2638154da9d8d7ef99531286b0 Reviewed-on: https://chromium-review.googlesource.com/984876 Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#546598} [modify] https://crrev.com/2c1339ee7eb5eb1090699661e938b63f595b00ca/base/safe_numerics_unittest.cc
,
Mar 29 2018
Thanks! The bots look much better now.
,
Mar 29 2018
I blame the test author. |
||||
►
Sign in to add a comment |
||||
Comment 1 by h...@chromium.org
, Mar 28 2018