lkml.org 
[lkml]   [2023]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] KVM: selftests: Zero-initialize entire test_result in memslot perf test
From
On 5.10.2023 02:29, Sean Christopherson wrote:
> Zero-initialize the entire test_result structure used by memslot_perf_test
> instead of zeroing only the fields used to guard the pr_info() calls.
>
> gcc 13.2.0 is a bit overzealous and incorrectly thinks that rbestslottim's
> slot_runtime may be used uninitialized.
>
> In file included from memslot_perf_test.c:25:
> memslot_perf_test.c: In function ‘main’:
> include/test_util.h:31:22: error: ‘rbestslottime.slot_runtime.tv_nsec’ may be used uninitialized [-Werror=maybe-uninitialized]
> 31 | #define pr_info(...) printf(__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~
> memslot_perf_test.c:1127:17: note: in expansion of macro ‘pr_info’
> 1127 | pr_info("Best slot setup time for the whole test area was %ld.%.9lds\n",
> | ^~~~~~~
> memslot_perf_test.c:1092:28: note: ‘rbestslottime.slot_runtime.tv_nsec’ was declared here
> 1092 | struct test_result rbestslottime;
> | ^~~~~~~~~~~~~
> include/test_util.h:31:22: error: ‘rbestslottime.slot_runtime.tv_sec’ may be used uninitialized [-Werror=maybe-uninitialized]
> 31 | #define pr_info(...) printf(__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~
> memslot_perf_test.c:1127:17: note: in expansion of macro ‘pr_info’
> 1127 | pr_info("Best slot setup time for the whole test area was %ld.%.9lds\n",
> | ^~~~~~~
> memslot_perf_test.c:1092:28: note: ‘rbestslottime.slot_runtime.tv_sec’ was declared here
> 1092 | struct test_result rbestslottime;
> | ^~~~~~~~~~~~~
>
> That can't actually happen, at least not without the "result" structure in
> test_loop() also being used uninitialized, which gcc doesn't complain
> about, as writes to rbestslottime are all-or-nothing, i.e. slottimens can't
> be non-zero without slot_runtime being written.
>
> if (!data->mem_size &&
> (!rbestslottime->slottimens ||
> result.slottimens < rbestslottime->slottimens))
> *rbestslottime = result;
>
> Zero-initialize the structures to make gcc happy even though this is
> likely a compiler bug. The cost to do so is negligible, both in terms of
> code and runtime overhead. The only downside is that the compiler won't
> warn about legitimate usage of "uninitialized" data, e.g. the test could
> end up consuming zeros instead of useful data. However, given that the
> test is quite mature and unlikely to see substantial changes, the odds of
> introducing such bugs are relatively low, whereas being able to compile
> KVM selftests with -Werror detects issues on a regular basis.
>
> Cc: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>
> I don't like papering over compiler bugs, but this is causing me quite a bit of
> pain, and IMO the long-term downsides are quite minimal. And I already spent
> way too much time trying to figure out if there is some bizarre edge case that
> gcc is detecting :-/
>

Weird, but as you say, the downsides of papering over this (probable) compiler
issue are small, so:
Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Thanks,
Maciej

\
 
 \ /
  Last update: 2023-10-05 16:47    [W:0.065 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site