[PATCH] c++: Implement -Wuninitialized for mem-initializers [PR19808]

[Martin Sebor]

On 11/19/20 11:41 AM, Jason Merrill wrote:
> On 11/17/20 3:44 AM, Jan Hubicka wrote:
>>> On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via 
>>> Gcc-patches wrote:
>>>>>> Why doesn't the middle-end warning work for inline functions?
>>>>>
>>>>> It does but only when they're called (and, as usual, also unless
>>>>> the uninitialized use is eliminated).
>>>>
>>>> Yes, but why?  I assume because we don't bother going through all 
>>>> the phases
>>>> of compilation for unused inlines, but couldn't we change that when 
>>>> we're
>>>> asking for (certain) warnings?
>>>
>>> CCing Richard and Honza on this.
>>>
>>> I think for unused functions we don't even gimplify unused functions, 
>>> the
>>> cgraph code just throws them away.  Even trying just to run the first 
>>> few
>>> passes (gimplification up to uninit1) would have several high costs,
>> Note that uninit1 is a late pass so it is not just few passes we speak
>> about.  Late passes are run only on cocde that really lands in .s file
>> so enabling them would mean splitting the pass queue and running another
>> unreachable code somewhere.  That would confuse inliner and other IPA
>> passes since they will have to somehow deal with dead code in their
>> program size estimate and also affect LTO.
>>
>> Even early passes are run only on reachable portion of program, since
>> functions are analyzed by cgraphunit on demand (only if they are
>> analyzed by someone else). Simlar logic is also done be C++ FE to decide
>> what templates.  Changling this would also have quite some compile
>> time/memory use impact.
>>
>> There is -fkeep-inline-functions.
> 
> OK, thanks for the explanation.  -fkeep-inline-functions seems like an 
> acceptable answer for people who want a warning audit of their library 
> header inlines.
> 
> Martin, I notice that the middle-end warning doesn't currently catch this:
> 
> struct B { int i,j; };
> 
> struct A
> {
>    B b;
>    A(): b({b.i}) { }
> };
> 
> A a;
> 
> It does warn if B only has one member; adding the second wrongly 
> silences the warning.

The GIMPLE for A's ctor looks like this:

A::A (struct A * const this)
{
   *this = {CLOBBER};
   {
     this->b = {};
     _1 = this->b.i;
     this->b.i = _1;
   }
}

so the b member is cleared first before it's read from and there's
nothing to warn about.  This happens for C structs too:

void f (void*);

struct A { int i, j; };

void g (void)
{
   struct A a = { a.i };
   f (&a);
}

The uninitialized read is in the original dump (below) so the zero
initialization happens in the gimplifier and could be diagnosed
there.

;; enabled by -tree-original


{
   struct A a = {.i=a.i};

     struct A a = {.i=a.i};
   f ((void *) &a);
}

Martin

PS I think the C++ original dump (below) shows the same problem
despite the Unknown trees that obscure it:

;; Function A::A() (null)
;; enabled by -tree-original


<<cleanup_point <<< Unknown tree: expr_stmt
   *(struct A *) this = {CLOBBER} >>>>>;
{
   <<cleanup_point <<< Unknown tree: expr_stmt
   (void) (((struct A *) this)->b = TARGET_EXPR <D.2389, {.i=((struct A 
*) this)->b.i}>) >>>>>;
}