If I’m not able to reproduce the issue, how do you expect me to see if I fixed it?
Please run clang -E in your environment that has all headers that options.c requires, creating a single isolated source file that shows the problem. A fix would need a regression test to go in, anyway. From: Alexander Kornienko [mailto:[email protected]] Sent: 04 August 2014 14:03 To: Artyom Skrobov Cc: Hal Finkel; Ted Kremenek; [email protected] Commits Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses On Mon, Aug 4, 2014 at 2:31 PM, Artyom Skrobov <[email protected]> wrote: May I ask you for an isolated source file that would reproduce this issue? options.c seems to require a whole tree of headers, including event_strings.h which is not present in the repository at all. I think, the issue is obvious: you've taken two different versions of the code and dropped one of them on the floor. Apparently, they were different for a reason. The fix could be as easy as adding back the version from LiveVariables.cpp (as another derived class, for example, once you already have inheritance there), and using it in LiveVariables. If you want to go further and understand the reason why they should be different, and what exactly breaks, the code I pointed at is probably the best I can provide. You can try looking at revision 2229: https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c?r=2229, it should be the closest one to the version I found the issue on. But please first send a patch to fix the issue in the simpliest manner. Thank you! From: Alexander Kornienko [mailto:[email protected]] Sent: 04 August 2014 13:25 To: Artyom Skrobov Cc: Hal Finkel; Ted Kremenek; [email protected] Commits Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses On Mon, Aug 4, 2014 at 2:22 PM, Alexander Kornienko <[email protected]> wrote: This patch makes the deadcode.DeadStores analyzer hang on this file: https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c The relevant part of stack trace looks like this: 0x00e189ed: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::balanceTree(llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*, clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*) 0x00e18d2e: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add_internal(clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*) 0x00e18ec5: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add_internal(clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*) 0x00e1b292: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add(llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*, clang::Stmt const*) 0x00e1f2d1: clang::LiveVariables::computeLiveness(clang::AnalysisDeclContext&, bool) 0x00673969: void clang::ento::check::ASTCodeBody::_checkBody<(anonymous namespace)::DeadStoresChecker>(void*, clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) Did you notice that the DataflowWorklist class was a bit different in these two classes? I meant, "files" (UninitializedValues.cpp and LiveVariables.cpp), not "classes". Notably, the dequeue method and initialization of the enqueuedBlocks bit-vector. Please fix or revert the patch. Thank you! On Tue, Jul 29, 2014 at 6:27 PM, Artyom Skrobov <[email protected]> wrote: Hal, thank you for the suggestion! I've expanded that comment four-fold in r214183. Ted, thank you for reviewing the original patch, and no worries it took a while. You might want to also check that the comments I've added when committing r214064 are correct -- although they're essentially a rephrasing of comments from your own emails. -----Original Message----- From: Hal Finkel [mailto:[email protected]] Sent: 28 July 2014 13:12 To: Artyom Skrobov Cc: [email protected] Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses We should have a description here of what this code does, not just where it's used. One can get some idea by reading the comments above DataflowWorklist::enqueueSuccessors in the source file, but it is not clear that gives a complete picture. -Hal _______________________________________________ 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
