From c0597fb382c575172ccc252517266294f16e2ac4 Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Mon, 16 Apr 2018 18:36:00 +0000 Subject: [PATCH] Revert "Reland "Floating-point exception observer for unit tests"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit aaa85ae565989f42b811c9a4858bb087319ba214. Reason for revert: Breaks iOS64 Debug trybot: https://uberchromegw.corp.google.com/i/internal.client.webrtc/builders/iOS64%20Debug/builds/14014 The failure being at: ../../test/fpe_observer_unittest.cc:93: Failure Expected equality of these values: 0x009f Which is: 159 all_flags Which is: 31 It looks like the missing flag may be "FE_FLUSHTOZERO"? Original change's description: > Reland "Floating-point exception observer for unit tests" > > This reverts commit e3d522dd6b52025191bacfab241f130e9870941f. > > Reason for revert: Disabling test failing in downstream projects. > > Original change's description: > > Revert "Floating-point exception observer for unit tests" > > > > This reverts commit 3fb3939896f6270d48aff34eee2946bd7661bd63. > > > > Reason for revert: Downstream projects failures. > > > > Original change's description: > > > Floating-point exception observer for unit tests > > > > > > This CL adds a simple tool that let a unit test fail if a floating > > > point exception occurs. It is possible to focus on specific exceptions. > > > Note that FloatingPointExceptionObserver is only effective in debug > > > mode. For this reason, the related unit tests only run in debug mode. > > > Plus, due to some platform-specific limitations, not all the floating > > > point exceptions are available on Android. > > > > > > Bug: webrtc:8948 > > > Change-Id: I0956e27f2f3aa68771dd647169fba7968ccbd771 > > > Reviewed-on: https://webrtc-review.googlesource.com/58097 > > > Commit-Queue: Alessio Bazzica > > > Reviewed-by: Patrik Höglund > > > Cr-Commit-Position: refs/heads/master@{#22768} > > > > TBR=phoglund@webrtc.org,alessiob@webrtc.org,kwiberg@webrtc.org > > > > Change-Id: I0fd3d114ab4a348fd46339e98273e19c1ac1c6dc > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Bug: webrtc:8948 > > Reviewed-on: https://webrtc-review.googlesource.com/67380 > > Reviewed-by: Alessio Bazzica > > Commit-Queue: Alessio Bazzica > > Cr-Commit-Position: refs/heads/master@{#22769} > > TBR=phoglund@webrtc.org,alessiob@webrtc.org,kwiberg@webrtc.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: webrtc:8948 > Change-Id: I7584d941b227277a271323b47bc70945af999758 > Reviewed-on: https://webrtc-review.googlesource.com/69060 > Reviewed-by: Karl Wiberg > Reviewed-by: Alessio Bazzica > Commit-Queue: Alessio Bazzica > Cr-Commit-Position: refs/heads/master@{#22848} TBR=phoglund@webrtc.org,alessiob@webrtc.org,kwiberg@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:8948 Change-Id: Ia377cea165211a0fad8f7ab29baae3eee64395c3 Reviewed-on: https://webrtc-review.googlesource.com/70280 Reviewed-by: Taylor Brandstetter Commit-Queue: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#22886} --- test/BUILD.gn | 13 ---- test/fpe_observer.h | 60 --------------- test/fpe_observer_unittest.cc | 140 ---------------------------------- 3 files changed, 213 deletions(-) delete mode 100644 test/fpe_observer.h delete mode 100644 test/fpe_observer_unittest.cc diff --git a/test/BUILD.gn b/test/BUILD.gn index f091d00f64..7a26e6384e 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -201,17 +201,6 @@ rtc_source_set("test_support") { } if (rtc_include_tests) { - rtc_source_set("floating_point_except_observer") { - testonly = true - sources = [ - "fpe_observer.h", - ] - deps = [ - ":test_support", - "../rtc_base:rtc_base_approved", - ] - } - rtc_source_set("test_main") { visibility = [ "*" ] testonly = true @@ -322,7 +311,6 @@ if (rtc_include_tests) { rtc_test("test_support_unittests") { deps = [ ":fileutils", - ":floating_point_except_observer", ":perf_test", ":rtp_test_utils", ":test_main", @@ -337,7 +325,6 @@ if (rtc_include_tests) { "//testing/gtest", ] sources = [ - "fpe_observer_unittest.cc", "frame_generator_unittest.cc", "rtp_file_reader_unittest.cc", "rtp_file_writer_unittest.cc", diff --git a/test/fpe_observer.h b/test/fpe_observer.h deleted file mode 100644 index 6244a91137..0000000000 --- a/test/fpe_observer.h +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright (c) 2018 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. - */ - -#ifndef TEST_FPE_OBSERVER_H_ -#define TEST_FPE_OBSERVER_H_ - -#include -#include "test/gtest.h" - -namespace webrtc { -namespace test { - -// Class that let a unit test fail if floating point exceptions are signaled. -// Usage: -// { -// FloatingPointExceptionObserver fpe_observer; -// ... -// } -class FloatingPointExceptionObserver { - public: - FloatingPointExceptionObserver(int mask = FE_DIVBYZERO | FE_INVALID | - FE_OVERFLOW | FE_UNDERFLOW) - : mask_(mask) { -#ifdef NDEBUG - EXPECT_LE(0, mask_); // Avoid compile time errors in release mode. -#else - EXPECT_EQ(0, std::feclearexcept(mask_)); -#endif - } - ~FloatingPointExceptionObserver() { -#ifndef NDEBUG - const int occurred = std::fetestexcept(mask_); - EXPECT_FALSE(occurred & FE_INVALID) - << "Domain error occurred in a floating-point operation."; - EXPECT_FALSE(occurred & FE_DIVBYZERO) << "Division by zero."; - EXPECT_FALSE(occurred & FE_OVERFLOW) - << "The result of a floating-point operation was too large."; - EXPECT_FALSE(occurred & FE_UNDERFLOW) - << "The result of a floating-point operation was subnormal with a loss " - << "of precision."; - EXPECT_FALSE(occurred & FE_INEXACT) - << "Inexact result: rounding during a floating-point operation."; -#endif - } - - private: - const int mask_; -}; - -} // namespace test -} // namespace webrtc - -#endif // TEST_FPE_OBSERVER_H_ diff --git a/test/fpe_observer_unittest.cc b/test/fpe_observer_unittest.cc deleted file mode 100644 index bf82fb2f03..0000000000 --- a/test/fpe_observer_unittest.cc +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright (c) 2018 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 -#include -#include - -#include "rtc_base/logging.h" -#include "test/fpe_observer.h" -#include "test/gtest.h" - -namespace webrtc { -namespace test { - -namespace { - -std::map GetExceptionCodes() { - static const std::map codes = { - {FE_INVALID, "FE_INVALID"}, -// TODO(bugs.webrtc.org/8948): Some floating point exceptions are not signaled -// on Android. -#ifndef WEBRTC_ANDROID - {FE_DIVBYZERO, "FE_DIVBYZERO"}, {FE_OVERFLOW, "FE_OVERFLOW"}, - {FE_UNDERFLOW, "FE_UNDERFLOW"}, -#endif - {FE_INEXACT, "FE_INEXACT"}, - }; - return codes; -} - -// Define helper functions as a trick to trigger floating point exceptions at -// run-time. -float MinusOne() { - return -std::cos(0.f); -} - -float PlusOne() { - return std::cos(0.f); -} - -float PlusTwo() { - return 2.f * std::cos(0.f); -} - -// Triggers one or more exception according to the |trigger| mask while -// observing the floating point exceptions defined in the |observe| mask. -void TriggerObserveFloatingPointExceptions(int trigger, int observe) { - FloatingPointExceptionObserver fpe_observer(observe); - float tmp = 0.f; - if (trigger & FE_INVALID) - tmp = std::sqrt(MinusOne()); - if (trigger & FE_DIVBYZERO) - tmp = 1.f / (MinusOne() + PlusOne()); - if (trigger & FE_OVERFLOW) - tmp = std::numeric_limits::max() * PlusTwo(); - if (trigger & FE_UNDERFLOW) { - // TODO(bugs.webrtc.org/8948): Check why FE_UNDERFLOW is not triggered with - // . - tmp = std::numeric_limits::min() / PlusTwo(); - } - if (trigger & FE_INEXACT) { - tmp = std::sqrt(2.0); - } -} - -} // namespace - -TEST(FloatingPointExceptionObserverTest, CheckTestConstants) { - // Check that the constants used in the test suite behave as expected. - ASSERT_EQ(0.f, MinusOne() + PlusOne()); -#ifndef WEBRTC_ANDROID - // Check that all the floating point exceptions are exercised. - int all_flags = 0; - for (const auto v : GetExceptionCodes()) { - RTC_LOG(LS_INFO) << v.second << " = " << v.first; - all_flags |= v.first; - } -#ifdef WEBRTC_MAC -#ifndef FE_UNNORMAL -#define FE_UNNORMAL 2 -#endif - all_flags |= FE_UNNORMAL; // Non standard OS specific flag. -#endif - ASSERT_EQ(FE_ALL_EXCEPT, all_flags); -#endif -} - -// TODO(bugs.webrtc.org/8948): NDEBUG is not reliable on downstream projects, -// keep false positive/negative tests disabled until fixed. -// #ifdef NDEBUG -#define MAYBE_CheckNoFalsePositives DISABLED_CheckNoFalsePositives -#define MAYBE_CheckNoFalseNegatives DISABLED_CheckNoFalseNegatives -// #else -// #define MAYBE_CheckNoFalsePositives CheckNoFalsePositives -// #define MAYBE_CheckNoFalseNegatives CheckNoFalseNegatives -// #endif - -// The floating point exception observer only works in debug mode. -// Trigger each single floating point exception while observing all the other -// exceptions. It must not fail. -TEST(FloatingPointExceptionObserverTest, MAYBE_CheckNoFalsePositives) { - for (const auto exception_code : GetExceptionCodes()) { - SCOPED_TRACE(exception_code.second); - const int trigger = exception_code.first; - int observe = FE_ALL_EXCEPT & ~trigger; - // Over/underflows also trigger FE_INEXACT; hence, ignore FE_INEXACT (which - // would be a false positive). - if (trigger & (FE_OVERFLOW | FE_UNDERFLOW)) - observe &= ~FE_INEXACT; - TriggerObserveFloatingPointExceptions(trigger, observe); - } -} - -// Trigger each single floating point exception while observing it. Check that -// this fails. -TEST(FloatingPointExceptionObserverTest, MAYBE_CheckNoFalseNegatives) { - for (const auto exception_code : GetExceptionCodes()) { - SCOPED_TRACE(exception_code.second); - const int trigger = exception_code.first; -#ifdef WEBRTC_ANDROID - // TODO(bugs.webrtc.org/8948): FE_INEXACT is not triggered on Android. - if (trigger == FE_INEXACT) - continue; -#endif - EXPECT_NONFATAL_FAILURE( - TriggerObserveFloatingPointExceptions(trigger, trigger), ""); - } -} - -} // namespace test -} // namespace webrtc