r186925 - Revert "[analyzer] Add very limited support for temporary destructors"
Manuel Klimek
klimek at google.com
Wed Jul 24 10:07:43 PDT 2013
On Wed, Jul 24, 2013 at 7:03 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> Oops, no. Thanks for keeping me honest. Will add soon.
>
Thanks! And if you have any ideas that can help Pavel fix this, it would be
super appreciated :) I think that this will be pretty important for C++ for
the analyzer going forwards (and thx for all your help so far, too!)
Cheers,
/Manuel
>
> On Jul 24, 2013, at 8:33 , Manuel Klimek <klimek at google.com> wrote:
>
> Jordan, did you add a regression test for this?
>
>
> On Tue, Jul 23, 2013 at 4:29 AM, Jordan Rose <jordan_rose at apple.com>
> wrote:
>
>> I did get around to reverting this, Pavel. Apart from PR16664<http://llvm.org/bugs/show_bug.cgi?id=16664>,
>> I realized that neither the binary operators nor their subexpressions are
>> still live when the analyzer goes back through the chain of logical
>> expressions to see which destructors to run. (You can see how this is
>> working using the debug.DumpCFG checker.) Without the values previously
>> computed for the expression, the analyzer won't actually be consistent
>> about which branches to take.
>>
>> I don't have an immediate solution in mind. It's sort of non-trivial to
>> teach the liveness analysis that an expression is going to be revisited
>> later, but the only alternative I can think of is crawling backwards
>> through the evaluated blocks to find out which branch we took, which is a
>> horrible idea because it could include inlined calls. If you come up with
>> anything, please send it up for review!
>>
>> Jordan
>>
>>
>> On Jul 22, 2013, at 19:15 , Jordan Rose <jordan_rose at apple.com> wrote:
>>
>> Author: jrose
>> Date: Mon Jul 22 21:15:11 2013
>> New Revision: 186925
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=186925&view=rev
>> Log:
>> Revert "[analyzer] Add very limited support for temporary destructors"
>>
>> The analyzer doesn't currently expect CFG blocks with terminators to be
>> empty, but this can happen when generating conditional destructors for
>> a complex logical expression, such as (a && (b || Temp{})). Moreover,
>> the branch conditions for these expressions are not persisted in the
>> state. Even for handling noreturn destructors this needs more work.
>>
>> This reverts r186498.
>>
>> Modified:
>> cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
>> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>> cfe/trunk/test/Analysis/analyzer-config.c
>> cfe/trunk/test/Analysis/analyzer-config.cpp
>> cfe/trunk/test/Analysis/dtor.cpp
>> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Mon Jul 22
>> 21:15:11 2013
>> @@ -119,7 +119,7 @@ bool AnalyzerOptions::getBooleanOption(O
>> bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
>> return getBooleanOption(IncludeTemporaryDtorsInCFG,
>> "cfg-temporary-dtors",
>> - /* Default = */ true);
>> + /* Default = */ false);
>> }
>>
>> bool AnalyzerOptions::mayInlineCXXStandardLibrary() {
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Jul 22 21:15:11
>> 2013
>> @@ -590,15 +590,7 @@ void ExprEngine::ProcessMemberDtor(const
>>
>> void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
>> ExplodedNode *Pred,
>> - ExplodedNodeSet &Dst) {
>> -
>> - QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
>> -
>> - // FIXME: Inlining of temporary destructors is not supported yet
>> anyway, so we
>> - // just put a NULL region for now. This will need to be changed later.
>> - VisitCXXDestructor(varType, NULL, D.getBindTemporaryExpr(),
>> - /*IsBase=*/ false, Pred, Dst);
>> -}
>> + ExplodedNodeSet &Dst) {}
>>
>> void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
>> ExplodedNodeSet &DstTop) {
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>> (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jul
>> 22 21:15:11 2013
>> @@ -807,12 +807,6 @@ bool ExprEngine::shouldInlineCall(const
>> AnalysisDeclContextManager &ADCMgr =
>> AMgr.getAnalysisDeclContextManager();
>> AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
>>
>> - // Temporary object destructor processing is currently broken, so we
>> never
>> - // inline them.
>> - // FIME: Remove this once temp destructors are working.
>> - if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
>> - return false;
>> -
>> // The auto-synthesized bodies are essential to inline as they are
>> // usually small and commonly used. Note: we should do this check early
>> on to
>> // ensure we always inline these calls.
>>
>> Modified: cfe/trunk/test/Analysis/analyzer-config.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/analyzer-config.c (original)
>> +++ cfe/trunk/test/Analysis/analyzer-config.c Mon Jul 22 21:15:11 2013
>> @@ -6,7 +6,7 @@ void foo() { bar(); }
>>
>> // CHECK: [config]
>> // CHECK-NEXT: cfg-conditional-static-initializers = true
>> -// CHECK-NEXT: cfg-temporary-dtors = true
>> +// CHECK-NEXT: cfg-temporary-dtors = false
>> // CHECK-NEXT: faux-bodies = true
>> // CHECK-NEXT: graph-trim-interval = 1000
>> // CHECK-NEXT: ipa = dynamic-bifurcate
>>
>> Modified: cfe/trunk/test/Analysis/analyzer-config.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/analyzer-config.cpp (original)
>> +++ cfe/trunk/test/Analysis/analyzer-config.cpp Mon Jul 22 21:15:11 2013
>> @@ -17,7 +17,7 @@ public:
>> // CHECK-NEXT: c++-stdlib-inlining = true
>> // CHECK-NEXT: c++-template-inlining = true
>> // CHECK-NEXT: cfg-conditional-static-initializers = true
>> -// CHECK-NEXT: cfg-temporary-dtors = true
>> +// CHECK-NEXT: cfg-temporary-dtors = false
>> // CHECK-NEXT: faux-bodies = true
>> // CHECK-NEXT: graph-trim-interval = 1000
>> // CHECK-NEXT: ipa = dynamic-bifurcate
>>
>> Modified: cfe/trunk/test/Analysis/dtor.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/dtor.cpp (original)
>> +++ cfe/trunk/test/Analysis/dtor.cpp Mon Jul 22 21:15:11 2013
>> @@ -416,19 +416,4 @@ namespace NoReturn {
>> f(&x);
>> *x = 47; // no warning
>> }
>> -
>> - void g2(int *x) {
>> - if (! x) NR();
>> - *x = 47; // no warning
>> - }
>> -
>> - void f3(int **x) {
>> - NR();
>> - }
>> -
>> - void g3() {
>> - int *x;
>> - f3(&x);
>> - *x = 47; // no warning
>> - }
>> }
>>
>> Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=186925&r1=186924&r2=186925&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original)
>> +++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Mon Jul 22
>> 21:15:11 2013
>> @@ -108,24 +108,6 @@ TestCtorInits::TestCtorInits()
>> : a(int(A()) + int(B()))
>> , b() {}
>>
>> -class NoReturn {
>> -public:
>> - ~NoReturn() __attribute__((noreturn));
>> - void f();
>> -};
>> -
>> -void test_noreturn1() {
>> - int a;
>> - NoReturn().f();
>> - int b;
>> -}
>> -
>> -void test_noreturn2() {
>> - int a;
>> - NoReturn(), 47;
>> - int b;
>> -}
>> -
>> // CHECK: [B1 (ENTRY)]
>> // CHECK: Succs (1): B0
>> // CHECK: [B0 (EXIT)]
>> @@ -864,36 +846,3 @@ void test_noreturn2() {
>> // CHECK: [B0 (EXIT)]
>> // CHECK: Preds (1): B1
>>
>> -// CHECK: [B3 (ENTRY)]
>> -// CHECK: Succs (1): B2
>> -// CHECK: [B1]
>> -// CHECK: 1: int b;
>> -// CHECK: Succs (1): B0
>> -// CHECK: [B2]
>> -// CHECK: 1: int a;
>> -// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)
>> -// CHECK: 3: [B2.2] (BindTemporary)
>> -// CHECK: 4: [B2.3].f
>> -// CHECK: 5: [B2.4]()
>> -// CHECK: 6: ~NoReturn() (Temporary object destructor)
>> -// CHECK: Preds (1): B3
>> -// CHECK: Succs (1): B0
>> -// CHECK: [B0 (EXIT)]
>> -// CHECK: Preds (2): B1 B2
>> -
>> -// CHECK: [B3 (ENTRY)]
>> -// CHECK: Succs (1): B2
>> -// CHECK: [B1]
>> -// CHECK: 1: int b;
>> -// CHECK: Succs (1): B0
>> -// CHECK: [B2]
>> -// CHECK: 1: int a;
>> -// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)
>> -// CHECK: 3: [B2.2] (BindTemporary)
>> -// CHECK: 4: 47
>> -// CHECK: 5: ... , [B2.4]
>> -// CHECK: 6: ~NoReturn() (Temporary object destructor)
>> -// CHECK: Preds (1): B3
>> -// CHECK: Succs (1): B0
>> -// CHECK: [B0 (EXIT)]
>> -// CHECK: Preds (2): B1 B2
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130724/271d89b2/attachment.html>
More information about the cfe-commits
mailing list