bench: Remove incorrect __LINE__ in BENCHMARK macro
What the developer wrote
Authored by MarcoFalke
bench: Remove incorrect __LINE__ in BENCHMARK macro
Duplicate benchmarks with the same name are not supported. Expanding the
name with __LINE__ is confusing and brittle, because it makes duplication
bugs silent.
Fix this twofold:
* By enforcing unique benchmarks at compile-time and link-time. For
example, a link failure may now look like:
"mold: error: duplicate symbol: bench_runner_AddrManAdd"
* By enforcing unique benchmarks at run-time. This should never happen,
due to the build-failure, but a failure may look like:
"Assertion `benchmarks().try_emplace(std::move(name), std::move(func)).second' failed."
What changed, and why it matters
This commit is a code-quality fix for Bitcoin Core's internal benchmark test harness. It removes a confusing use of line numbers in benchmark names and adds checks to prevent duplicate benchmark names. It does not affect the live Bitcoin network, wallet funds, consensus rules, or node security.
No security action required. This is a benign internal test-harness improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change modifies the BENCHMARK macro in src/bench/bench.h so that generated benchmark registration symbols use a stable name (bench_runner_
Changed components
src/bench/bench.hsrc/bench/bench.cppInspect captured patch +4 / −3
diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp
index 9b74af51..0b2ee6e3 100644
--- a/src/bench/bench.cpp
+++ b/src/bench/bench.cpp
@@ -5,6 +5,7 @@
#include <bench/bench.h>
#include <test/util/setup_common.h> // IWYU pragma: keep
+#include <util/check.h>
#include <util/fs.h>
#include <chrono>
@@ -71,7 +72,7 @@ BenchRunner::BenchmarkMap& BenchRunner::benchmarks()
BenchRunner::BenchRunner(std::string name, BenchFunction func)
{
- benchmarks().insert(std::make_pair(name, func));
+ Assert(benchmarks().try_emplace(std::move(name), std::move(func)).second);
}
void BenchRunner::RunAll(const Args& args)
diff --git a/src/bench/bench.h b/src/bench/bench.h
index 591ea8bc..14f0c4b0 100644
--- a/src/bench/bench.h
+++ b/src/bench/bench.h
@@ -64,8 +64,8 @@ public:
};
} // namespace benchmark
-// BENCHMARK(foo) expands to: benchmark::BenchRunner bench_11foo{"foo", foo};
+// BENCHMARK(foo) expands to: benchmark::BenchRunner bench_runner_foo{"foo", foo};
#define BENCHMARK(n) \
- benchmark::BenchRunner PASTE2(bench_, PASTE2(__LINE__, n)){STRINGIZE(n), n};
+ benchmark::BenchRunner PASTE2(bench_runner_, n){STRINGIZE(n), n};
#endif // BITCOIN_BENCH_BENCH_H
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.