Skip to content

Commit 660fe65

Browse files
committed
fix(bench): unblock Linux/Windows bench builds
Two independent failures in the Benchmark workflow after the Stage 10/12 tests landed: Linux bench (gcc-14, -O3 -Werror) -Wfree-nonheap-object fires a false positive inside domain_hash_sha256's std::vector<uint8_t>::push_back + insert chain. GCC 14's -O3 optimizer inlines _M_realloc_append's _Guard destructor, loses track of the allocation's provenance, and flags the allocator's deallocate call as "called on pointer with nonzero offset". Fix by rewriting the buffer construction as a single sized-vector constructor + memcpy fills. One allocation, no push_back / insert intermediates for the optimizer to lose track of. Functionally identical; all 22 CborStrict tests stay green on MSVC Debug, including the three domain-hash determinism checks. Windows bench (MSVC, LNK2038 CRT mismatch) The Windows job passed only `-A x64` at configure time — no CMAKE_BUILD_TYPE. That made the Botan ExternalProject rule in common/crypto/src/CMakeLists.txt:78-84 match its "multi-config + CMAKE_BUILD_TYPE unset" branch and default Botan to --msvc-runtime=MDd (Debug CRT). The later `cmake --build --config Release` step compiled bench_main with MD (Release CRT), producing a link-time CRT mismatch against the Debug Botan archive. Not a cache issue — deleting the cache reproduced the same wrong Botan config on every fresh configure. Fix at the workflow level (the correct idiom anyway): pass CMAKE_BUILD_TYPE=Release on the Windows bench so Botan's configure-time choice aligns with the Release binary that links against it. No behavior change on the test / release / debug paths that build with explicit CMAKE_BUILD_TYPE; this only addresses the Benchmark workflow's Windows path.
1 parent 30da51d commit 660fe65

2 files changed

Lines changed: 17 additions & 8 deletions

File tree

.github/workflows/benchmark.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ jobs:
3737
platform: macos
3838
- os: windows-latest
3939
generator: "Visual Studio 17 2022"
40-
cmake_args: "-A x64"
40+
# CMAKE_BUILD_TYPE is set explicitly even though VS is a
41+
# multi-config generator: the Botan ExternalProject picks
42+
# --msvc-runtime at configure time, and without this flag
43+
# common/crypto/src/CMakeLists.txt defaults to MDd (Debug),
44+
# which then LNK2038s against the --config Release bench.
45+
cmake_args: "-A x64 -DCMAKE_BUILD_TYPE=Release"
4146
build_args: "--config Release"
4247
bin: ./build/bin/Release/vmpilot_bench.exe
4348
platform: windows

common/src/cbor/strict.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,18 @@ domain_hash_sha256(std::string_view domain_label,
295295
if (domain_label.empty() || domain_label.size() > 0xff) {
296296
std::abort();
297297
}
298-
std::vector<std::uint8_t> buf;
299-
buf.reserve(1 + domain_label.size() + size);
300-
buf.push_back(static_cast<std::uint8_t>(domain_label.size()));
301-
buf.insert(buf.end(),
302-
reinterpret_cast<const std::uint8_t*>(domain_label.data()),
303-
reinterpret_cast<const std::uint8_t*>(domain_label.data()) + domain_label.size());
298+
// Single-allocation layout avoids push_back/insert chains that trip
299+
// GCC 14 -O3 -Wfree-nonheap-object provenance tracking (the
300+
// compiler loses track of _M_realloc_append's pointer through its
301+
// own inlining and emits a false-positive "called on pointer with
302+
// nonzero offset" error under -Werror).
303+
const std::size_t total = 1 + domain_label.size() + size;
304+
std::vector<std::uint8_t> buf(total);
305+
buf[0] = static_cast<std::uint8_t>(domain_label.size());
306+
std::memcpy(buf.data() + 1, domain_label.data(), domain_label.size());
304307
if (size > 0) {
305-
buf.insert(buf.end(), canonical_bytes, canonical_bytes + size);
308+
std::memcpy(buf.data() + 1 + domain_label.size(),
309+
canonical_bytes, size);
306310
}
307311

308312
const auto digest = VMPilot::Crypto::SHA256(buf, /*salt=*/{});

0 commit comments

Comments
 (0)