[cfe-dev] CFG and CXXDefaultInitExpr

[Enrico P]

Let me see if I understand your idea:

1. Don't skip initializers, and handle CXXDefaultInitExprs by visiting
the wrapped expression even if it's outside of the CFG.
2. When Sema::ParseLexedMemberInitializers is called, we treat each
CXXDefaultInitExpr as if it were a function, but marking all the
earlier-declared fields as "Initialized", and run the
UninitializedValues on it.

The first part is straightforward: if the long-term plan is to remove
the DiagnosedUninitializedFields, the only option is to have a
recursive visitor on the CXXDefaultInitExprs that visits the wrapped
expr. I already have it working.

About the second part: are you suggesting to create a temporary
FunctionDecl with just the CXXDefaultInitExpr as a body, and calling
clang::runUninitializedVariablesAnalysis() on it?

On Tue, Oct 22, 2013 at 9:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> What we want here long-term is to extend the UninitializedValues checker to understand initializers (as you have done) and to remove the DiagnoseUninitializedFields check, rather than to skip the initializers in the CFG-based check.
>
> For default initializers, we should analyze them at the point where they're declared, looking for uses of later-declared fields, and also consider them when analyzing a constructor (this time looking for uses of earlier-declared fields that weren't initialized). Perhaps the easiest way to do this would be to treat them as if they were an entire function body, perform an UninitializedValues check on them, and suppress any warnings for earlier-declared fields, then when we run the UninitializedValues check on the constructor, look for earlier fields where we wanted to warn and check whether those fields are in fact initialized.
>
>
>
> On Tue, Oct 22, 2013 at 11:48 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>>
>> Ah, I see. Thanks for the clarification. Yes, skipping the initializers in the CFG is one way to do this; another would be to traverse the CXXDefaultInitExpr yourself, to make sure the expression inside doesn't reference any uninitialized fields. The last answer would be to clone the AST, but I'm a bit wary of trying to do that. Sorry there's not a cleaner solution in the meantime.
>>
>> Jordan
>>
>>
>> On Oct 22, 2013, at 10:17, Enrico P <epertoso at google.com> wrote:
>>
>> Yes, the presence of a CXXDefaultInitExpr is enough to consider a field initialized.
>>
>> I forgot to mention that I'm trying to use clang::runUninitializedVariablesAnalysis() to detect the use of an uninitialized field in the constructor's body.
>>
>> In the following case:
>>
>> 1:  struct A {
>> 2:   int a;
>> 3:   int b = a;
>> 4:   int c, d;
>> 5:   A() : c(d) {
>> 6:     a = d;
>> 7:   }
>> 8:  };
>>
>> A()'s constructor initializers will be (in order): a CXXCtorInitializer constructed with the FieldDecl of 'b' and a CXXDefaultInitExpr, and one with the FieldDecl of 'y' and an ImplicitCastExpr.
>>
>> If UninitializedValues.cpp is modified to also track uninitialized uses of fields, it will issue a warning about 'd' on line 5 and one on line 6.
>> It will not warn about 'a' on line 3 because the expression wrapped by CXXDefaultInitExpr doesn't appear in the CFG. Hence my initial question.
>>
>> Sema::ActOnMemInitializers (calling 'DiagnoseUnitializedFields' defined in SemaDeclCXX.cpp) will correctly warn about 'a' on line 3 and 'd' on line 5, but of course not on line 6.
>>
>> As we've established that the expression for which a CXXDefaultInitExpr acts as a placeholder should not be appended to the CFG, the easiest way to warn on lines 3, 5, and 6 is to use
>> both SemaDeclCXX.cpp's DiagnoseUnitializedFields and clang::runUninitializedVariablesAnalysis, but care must be taken so that only one warning is generated for line 5.
>>
>> I'm sorry to have caused confusion.
>> Enrico
>>
>>
>> On Tue, Oct 22, 2013 at 5:59 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>>>
>>> CXXDefaultInitExprs only appear if the field has an in-class initializer, so just the presence of the expr should be enough to consider the field initialized. Or am I misunderstanding something?
>>>
>>> Jordan
>>>
>>>
>>> On Oct 21, 2013, at 10:10 , Enrico P <epertoso at google.com> wrote:
>>>
>>> Right, I can see the CXXDefaultInitExpr appearing multiple times when initializing (C++14) aggregates with braces.
>>>
>>> Verifying that a field has an in-class initializer (or that the constructor has a CXXCtorInitializer for that field) is what I'm currently doing.
>>>
>>> Sema::ActOnMemInitializers already takes care of the initialization order. If the fields appearing in the constructor body aren't ignored until all of the CXXCtorInitializers have been visited, duplicate warnings may be issued, hence I was wondering whether the analysis could be done in just one place.
>>>
>>> I guess that for now it would be better to just visit the CFG of a constructor starting from the statement that follows the last CFGInitializer, and let Sema::ActOnMemInitializers take care of the rest.
>>>
>>> Thanks for the answer!
>>> Enrico
>>>
>>> On Fri, Oct 18, 2013 at 6:40 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>>>>
>>>> Unfortunately yes: a CXXDefaultInitExpr is a placeholder for whichever expression was written as the initial value of the member you've left out, and that expression doesn't belong to the current context. If you used list initialization (braces) to initialize two different objects, you'd end up including the initializer expression twice, and the CFG doesn't work right if the same Stmt* appears more than once.
>>>>
>>>> That said, I would think that the presence of a CXXDefaultInitExpr would be enough to prove that a member variable is initialized. If you're worried about initialization order problems, you could either traverse the CXXDefaultInitExpr manually or just make a non-flow-sensitive check that the initializer expression doesn't reference any member variables defined later in the class.
>>>>
>>>> Does that help?
>>>> Jordan
>>>>
>>>>
>>>> On Oct 18, 2013, at 9:19 , Enrico P <epertoso at google.com> wrote:
>>>>
>>>> > Hello,
>>>> >
>>>> > I'm doing some work on -Wuninitialized to make it detect the use of uninitialized fields within C++ constructor bodies.
>>>> >
>>>> > When the CFGBuilder visits a CXXDefaultInitExpr, it won't visit its children because CXXDefaultInitExpr::children() returns an empty child_range.
>>>> >
>>>> > Is there any reason for this (maybe related to the comment at line 1108 of CFG.cpp?), or is it safe to have it return a non-empty child_range?
>>>> >
>>>> > Thanks,
>>>> > Enrico
>>>> > _______________________________________________
>>>> > cfe-dev mailing list
>>>> > cfe-dev at cs.uiuc.edu
>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>>
>>>
>>>
>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>