[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen: Use -Wuninitialized and -Winit-self
On 2024-01-05 07:56, Jan Beulich wrote:
On 04.01.2024 21:43, Roberto Bagnara wrote:
On 2024-01-04 15:33, Andrew Cooper wrote:
On 04/01/2024 1:41 pm, Jan Beulich wrote:
On 28.12.2023 20:39, Andrew Cooper wrote:
The use of uninitialised data is undefined behaviour. At -O2 with trivial
examples, both Clang and GCC delete the variable, and in the case of a
function return, the caller gets whatever was stale in %rax prior to the call.
Clang includes -Wuninitialized within -Wall, but GCC only includes it in
-Wextra, which is not used by Xen at this time.
Furthermore, the specific pattern of assigning a variable to itself in its
declaration is only diagnosed by GCC with -Winit-self. Clang does diagnoise
simple forms of this pattern with a plain -Wuninitialized, but it fails to
diagnose the instances in Xen that GCC manages to find.
GCC, with -Wuninitialized and -Winit-self notices:
arch/x86/time.c: In function ‘read_pt_and_tsc’:
arch/x86/time.c:297:14: error: ‘best’ is used uninitialized in this
function [-Werror=uninitialized]
297 | uint32_t best = best;
| ^~~~
arch/x86/time.c: In function ‘read_pt_and_tmcct’:
arch/x86/time.c:1022:14: error: ‘best’ is used uninitialized in this
function [-Werror=uninitialized]
1022 | uint64_t best = best;
| ^~~~
and both have logic paths where best can be returned while uninitalised.
I disagree. In both cases the variables are reliably set during the first
loop iteration.
I suggest you pay attention to the precision of the integers.
It is hard (likely prohibitively hard) to avoid entering the if(), but
it is not impossible.
The compiler really has emitted logic paths where stack rubble is returned.
Furthermore this initialize-to-self is a well known pattern to suppress the
-Wuninitialized induced warnings, originally used by Linux'es
uninitialized_var().
I'm glad you cited this, because it proves my point.
Notice how it was purged from Linux slowly over the course of 8 years
because it had been shown to create real bugs, by hiding real uses of
uninitialised variables.
There is a worse problem for initialize-to-self: it is undefined behavior
per se. If this is done to suppress a warning, then what happens is
paradoxical: in order to suppress a warning about a potential undefined
behavior (the variable might indeed be always written before being read)
one introduces a definite undefined behavior.
I don't think so - aiui this is another of the many gcc extensions to the
language (no code is generated for this type of initialization, iirc).
Well, it is undefined behavior for the C Standard: "undefined behavior"
is a technical term (see, e.g., C18 3.4.3p1) that is not dependent on
any compiler or any implementation technique for the C language.
Any implementation can deal with undefined behavior with absolute
freedom, which includes, among infinite other possibilities, doing what
you believe it does (iyrc ;-)
In addition, the compiler is typically not the only element of the toolchain
that needs to reliably interpret and process C code: so, even if we could reach
absolute certainty about what a specific version of one compiler does in
response
to one instance of undefined behavior (and I think we cannot), then we would
need to obtain similar guarantees for other tools.
|
