From fa6a55877de9168ce2cc2ebdc05bcd4a9f8604bb Mon Sep 17 00:00:00 2001 From: Gigon Bae Date: Wed, 15 Jan 2025 17:47:18 -0800 Subject: [PATCH] Fix primitives benchmark code (#812) The pointer variable is used after releasing memory with `free()`, which is a bug that needs to be addressed. Without this fix, the benchmark code may fail to build or run correctly, resulting in the following error ([link](https://github.com/rapidsai/cucim/actions/runs/12753155354/job/35544254443?pr=811)) ``` In function 'void benchmark::DoNotOptimize(Tp&) [with Tp = char*]', inlined from 'void string_strcpy(benchmark::State&)' at /opt/conda/conda-bld/work/benchmarks/primitives.cpp:110:33: /opt/conda/conda-bld/work/build-release/_deps/deps-googlebenchmark-src/src/../include/benchmark/benchmark.h:322:3: error: pointer used after 'void free(void*)' [-Werror=use-after-free] 322 | asm volatile("" : "+m,r"(value) : : "memory"); | ^~~ ``` It seems that the updated version of GCC is now capable of detecting this potential issue. This commit resolves the issue by ensuring the memory is released only after `benchmark::DoNotOptimize()` is called. This PR handles a build issue related to #811 - #811 Authors: - Gigon Bae (https://github.com/gigony) Approvers: - Bradley Dice (https://github.com/bdice) - https://github.com/jakirkham URL: https://github.com/rapidsai/cucim/pull/812 --- benchmarks/primitives.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/benchmarks/primitives.cpp b/benchmarks/primitives.cpp index d3dd2da54..0ff576d77 100644 --- a/benchmarks/primitives.cpp +++ b/benchmarks/primitives.cpp @@ -89,10 +89,10 @@ static void string_memcpy(benchmark::State& state) char * c_str = (char*) malloc(size + 1); memcpy(c_str, data.data(), size); c_str[size] = '\0'; - free(c_str); // Make sure the variable is not optimized away by compiler benchmark::DoNotOptimize(c_str); benchmark::DoNotOptimize(size); + free(c_str); } } BENCHMARK(string_memcpy); @@ -105,9 +105,9 @@ static void string_strcpy(benchmark::State& state) std::string data = "#########################################################################################################################################################################################"; char * c_str = (char*) malloc(data.size() + 1); strcpy(c_str, data.data()); - free(c_str); // Make sure the variable is not optimized away by compiler benchmark::DoNotOptimize(c_str); + free(c_str); } } BENCHMARK(string_strcpy); @@ -120,9 +120,9 @@ static void string_strdup(benchmark::State& state) { std::string data = "#########################################################################################################################################################################################"; char * c_str = strdup(data.data()); - free(c_str); // Make sure the variable is not optimized away by compiler benchmark::DoNotOptimize(c_str); + free(c_str); } } BENCHMARK(string_strdup); @@ -140,12 +140,12 @@ static void alloc_malloc(benchmark::State& state) arr[i] = (char*)malloc(10); arr[i][0] = i; } + // Make sure the variable is not optimized away by compiler + benchmark::DoNotOptimize(arr); for (int i = 0; i < 30000; i++) { free(arr[i]); } - // Make sure the variable is not optimized away by compiler - benchmark::DoNotOptimize(arr); } } BENCHMARK(alloc_malloc);//->Iterations(100); @@ -163,12 +163,12 @@ static void alloc_pmr(benchmark::State& state) arr[i] = static_cast(cucim_malloc(10)); arr[i][0] = i; } + // Make sure the variable is not optimized away by compiler + benchmark::DoNotOptimize(arr); for (int i = 0; i < 30000; i++) { cucim_free(arr[i]); } - // Make sure the variable is not optimized away by compiler - benchmark::DoNotOptimize(arr); } } BENCHMARK(alloc_pmr);//->Iterations(100);