Add checks that <Resume|Pause>Timing functions are not called outside of the benchmark. Fixes #204

include/benchmark/benchmark_api.h

@@ -242,15 +242,17 @@ public:
   // returned false.
   bool KeepRunning() {
     if (BENCHMARK_BUILTIN_EXPECT(!started_, false)) {
-        ResumeTiming();
+        assert(!finished_);
         started_ = true;
+        ResumeTiming();
     }
     bool const res = total_iterations_++ < max_iterations;
     if (BENCHMARK_BUILTIN_EXPECT(!res, false)) {
-        assert(started_);
+        assert(started_ && !finished_);
         PauseTiming();
         // Total iterations now is one greater than max iterations. Fix this.
         total_iterations_ = max_iterations;
+        finished_ = true;
     }
     return res;
   }
@@ -371,6 +373,7 @@ public:
 
 private:
   bool started_;
+  bool finished_;
   size_t total_iterations_;
 
   bool has_range_x_;

src/benchmark.cc

@@ -815,7 +815,7 @@ void RunBenchmark(const benchmark::internal::Benchmark::Instance& b,
 
 State::State(size_t max_iters, bool has_x, int x, bool has_y, int y,
              int thread_i, int n_threads)
-    : started_(false), total_iterations_(0),
+    : started_(false), finished_(false), total_iterations_(0),
       has_range_x_(has_x), range_x_(x),
       has_range_y_(has_y), range_y_(y),
       bytes_processed_(0), items_processed_(0),
@@ -830,11 +830,13 @@ State::State(size_t max_iters, bool has_x, int x, bool has_y, int y,
 void State::PauseTiming() {
   // Add in time accumulated so far
   CHECK(running_benchmark);
+  CHECK(started_ && !finished_);
   timer_manager->StopTimer();
 }
 
 void State::ResumeTiming() {
   CHECK(running_benchmark);
+  CHECK(started_ && !finished_);
   timer_manager->StartTimer();
 }
 

src/check.h

@@ -10,6 +10,18 @@
 namespace benchmark {
 namespace internal {
 
+typedef void(AbortHandlerT)();
+
+inline AbortHandlerT*& get_abort_handler() {
+    static AbortHandlerT* handler = &std::abort;
+    return handler;
+}
+
+BENCHMARK_NORETURN inline void abort_handler() {
+    get_abort_handler()();
+    std::abort(); // fallback to enforce noreturn
+}
+
 // CheckHandler is the class constructed by failing CHECK macros. CheckHandler
 // will log information about the failures and abort when it is destructed.
 class CheckHandler {
@@ -25,13 +37,13 @@ public:
     return log_;
   }
 
-  BENCHMARK_NORETURN ~CheckHandler() {
+  BENCHMARK_NORETURN ~CheckHandler() noexcept(false) {
       log_ << std::endl;
-      std::abort();
+      abort_handler();
   }
 
-  CheckHandler & operator=(const CheckHandler&) = delete;
-  CheckHandler(const CheckHandler&) = delete;
+  CheckHandler & operator=(const CheckHandler&) = delete;
+  CheckHandler(const CheckHandler&) = delete;
   CheckHandler() = delete;
 private:
   std::ostream& log_;

test/CMakeLists.txt

@@ -36,6 +36,9 @@ add_test(options_benchmarks options_test --benchmark_min_time=0.01)
 compile_benchmark_test(basic_test)
 add_test(basic_benchmark basic_test --benchmark_min_time=0.01)
 
+compile_benchmark_test(diagnostics_test)
+add_test(diagnostics_test diagnostics_test --benchmark_min_time=0.01)
+
 compile_benchmark_test(fixture_test)
 add_test(fixture_test fixture_test --benchmark_min_time=0.01)
 

test/diagnostics_test.cc

@@ -0,0 +1,43 @@
+
+#include "benchmark/benchmark_api.h"
+#include "../src/check.h"
+
+void test_handler() {
+  throw std::logic_error("");
+}
+
+void try_invalid_pause_resume(benchmark::State& state) {
+#ifndef NDEBUG
+  try {
+    state.PauseTiming();
+    std::abort();
+  } catch (std::logic_error const&) {}
+  try {
+    state.ResumeTiming();
+    std::abort();
+  } catch (std::logic_error const&) {}
+#else
+  (void)state; // avoid unused warning
+#endif
+}
+
+void BM_diagnostic_test(benchmark::State& state) {
+  static bool called_once = false;
+
+  if (called_once == false) try_invalid_pause_resume(state);
+
+  while (state.KeepRunning()) {
+    benchmark::DoNotOptimize(state.iterations());
+  }
+
+  if (called_once == false) try_invalid_pause_resume(state);
+
+  called_once = true;
+}
+BENCHMARK(BM_diagnostic_test);
+
+int main(int argc, char** argv) {
+  benchmark::internal::get_abort_handler() = &test_handler;
+  benchmark::Initialize(&argc, argv);
+  benchmark::RunSpecifiedBenchmarks();
+}