diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index 6d98aba58..5594451a8 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -54,6 +54,8 @@ BENCHMARK_EXPORT void SetBenchmarkFilter(std::string value); BENCHMARK_EXPORT int32_t GetBenchmarkVerbosity(); +// Creates the display reporter selected by --benchmark_format. Returns a +// fresh instance on every call; the caller owns and frees it. BENCHMARK_EXPORT BenchmarkReporter* CreateDefaultDisplayReporter(); BENCHMARK_EXPORT size_t RunSpecifiedBenchmarks(); diff --git a/src/benchmark.cc b/src/benchmark.cc index 91280295e..e5ab9dfdf 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -586,11 +586,11 @@ ConsoleReporter::OutputOptions GetOutputOptions(bool force_no_color) { } // end namespace internal BenchmarkReporter* CreateDefaultDisplayReporter() { - static auto* default_display_reporter = - internal::CreateReporter(FLAGS_benchmark_format, - internal::GetOutputOptions()) - .release(); - return default_display_reporter; + // Each caller owns the returned reporter, so never cache it in a static: + // the first caller would free it and later calls would dangle (#2240). + return internal::CreateReporter(FLAGS_benchmark_format, + internal::GetOutputOptions()) + .release(); } size_t RunSpecifiedBenchmarks() { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index fe88841dd..534eaf110 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -213,6 +213,9 @@ benchmark_add_test(NAME user_counters_thousands_test COMMAND user_counters_thous compile_output_test(memory_manager_test) benchmark_add_test(NAME memory_manager_test COMMAND memory_manager_test --benchmark_min_time=0.01s) +compile_benchmark_test(default_display_reporter_test) +benchmark_add_test(NAME default_display_reporter_test COMMAND default_display_reporter_test) + compile_output_test(profiler_manager_test) benchmark_add_test(NAME profiler_manager_test COMMAND profiler_manager_test --benchmark_min_time=0.01s) diff --git a/test/default_display_reporter_test.cc b/test/default_display_reporter_test.cc new file mode 100644 index 000000000..f4891f938 --- /dev/null +++ b/test/default_display_reporter_test.cc @@ -0,0 +1,28 @@ +// Regression test for https://github.com/google/benchmark/issues/2240: +// CreateDefaultDisplayReporter() cached its result in a function-local static +// even though each caller owns (and frees) the returned pointer, so the +// second call returned a dangling pointer. It must hand out a fresh reporter +// on every call. + +#include +#include + +#include "benchmark/benchmark.h" + +int main(int argc, char** argv) { + benchmark::MaybeReenterWithoutASLR(argc, argv); + benchmark::Initialize(&argc, argv); + + std::unique_ptr first( + benchmark::CreateDefaultDisplayReporter()); + std::unique_ptr second( + benchmark::CreateDefaultDisplayReporter()); + + if (first == nullptr || second == nullptr || first.get() == second.get()) { + std::cerr << "CreateDefaultDisplayReporter must return a fresh, " + "caller-owned reporter on every call\n"; + return 1; + } + + return 0; +}