Date: Thu, 3 Mar 2016 13:55:42 +0100
From: Jakub Jelinek <jakub@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Colin King <colin.king@canonical.com>, Ingo Molnar <mingo@redhat.com>,
        linux-kernel@vger.kernel.org, Richard Henderson <rth@twiddle.net>,
        Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: Q: why didn't GCC warn about this uninitialized variable? (was:
 Re: [PATCH] perf tests: initialize sa.sa_flags)
Message-ID: <20160303125542.GD3017@tucnak.redhat.com>
Reply-To: Jakub Jelinek <jakub@redhat.com>
References: <1456923322-29697-1-git-send-email-colin.king@canonical.com>
 <20160302125901.GF6356@twins.programming.kicks-ass.net>
 <20160302130350.GO3604@kernel.org>
 <20160302132127.GG6356@twins.programming.kicks-ass.net>
 <20160302132323.GP3604@kernel.org>
 <20160303121944.GB2484@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160303121944.GB2484@gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2106
Lines: 43

On Thu, Mar 03, 2016 at 01:19:44PM +0100, Ingo Molnar wrote:
>         struct sigaction sa;
> 
> 	...
> 
>         sigfillset(&sa.sa_mask);
>         sa.sa_sigaction = segfault_handler;
>         sigaction(SIGSEGV, &sa, NULL);
> 
> ... which uninitialized sa.sa_flags field GCC merrily accepted as proper C code, 
> despite us turning on essentially _all_ GCC warnings for the perf build that exist 
> under the sun:

GCC -W{,maybe-}uninitialized warning works only on SSA form, so in order to
get that warning, either it needs to be some scalar that is uninitialized,
or Scalar Replacement of Aggregates needs to be able to turn the structure
into independent scalars.  Neither is the case here, as you take address of
the struct when passing its address to sigaction, and that call can't be
inlined nor is in any other way visible to the compiler, so that it could
optimize both the caller and sigaction itself at the same time.

Even if GCC added infrastructure for tracking which bits/bytes in
aggregates are or might be uninitialized at which point, generally,
  struct XYZ abc;
  foo (&abc);
is so common pattern that warning here that the fields are uninitialized
would have extremely high false positive ratio.
Even if as somebody mentioned that the argument is const struct sigaction *
rather than struct sigaction *, that doesn't change really anything,
you can cast away the constness and still write into it in the other
function.
Furthermore, in many APIs, only a subset of fields need to be initialized
unconditionally, and other fields might need to be initialized only
conditionally, depending on those always initialized fields, other
parameters to functions, etc.
So, in order to warn here, we'd need some assurance (new attribute on
sigaction function?) that when it is called, it has to have all named fields in
the pointed to structures initialized (perhaps attribute like nonnull, which
can either apply to all pointer arguments, or selected ones) at the entry of
the function and not initializing them all is a bug.
So, this really isn't as trivial as you might think it is.

	Jakub