Fix default reporter use after free

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();

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() {

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)
 

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 <iostream>
+#include <memory>
+
+#include "benchmark/benchmark.h"
+
+int main(int argc, char** argv) {
+  benchmark::MaybeReenterWithoutASLR(argc, argv);
+  benchmark::Initialize(&argc, argv);
+
+  std::unique_ptr<benchmark::BenchmarkReporter> first(
+      benchmark::CreateDefaultDisplayReporter());
+  std::unique_ptr<benchmark::BenchmarkReporter> 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;
+}