Jordan, did you add a regression test for this?
On Tue, Jul 23, 2013 at 4:29 AM, Jordan Rose <[email protected]> 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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
