[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-17 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351499: [analyzer] Make sure base-region and its sub-regions are either all alive or… (authored by dergachev, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D56632#1359215 , @NoQ wrote: > In D56632#1356163 , > @baloghadamsoftware wrote: > > > Did you measure decrease in the false-positive rate or an increase in the > > true-positive rate on rea

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D56632#1359576 , @xazax.hun wrote: > If you find out the reason why we need `markElementIndicesLive`, documenting > that would also be useful. But it is also independent of this change. > Maybe something like we could learn new i

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/symbol-reaper.cpp:31 + clang_analyzer_eval(glob); // expected-warning{{TRUE}} + // expected-warning@-1{{SYMBOL DEAD}} +} NoQ wrote: > Szelethus wrote: > > N00b question: What

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-16 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. Thanks, LGTM! It is interesting to see if we need to traverse all the super regions in `scanReachableSymbols`, but if we need to change something there, I would prefer that to be in a separate patch. If visiting the whole super region

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:390 - // FIXME: Remove when we migrate over to just using ValueManager. SymbolManager &getSymbolManager() { return SymMgr; } baloghadamsoftware wrote: > Is t

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 181961. NoQ marked 12 inline comments as done. NoQ added a comment. Fix stuff. In D56632#1356163 , @baloghadamsoftware wrote: > Did you measure decrease in the false-positive rate or an increase in the > true-positive r

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-14 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment. Hi Artem, This looks perfect, just some stylish issues. Comment at: test/Analysis/symbol-reaper.cpp:13 + void foo() { +// Ugh, maybe just let clang_analyzer_eval() work within callees already? +// The glob variable shouldn't keep our symbol a

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-14 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I really like all this detective work and it would be sad to have it forgotten. I would love to see some of your comments in the documentation of symbol reaper. More specifically: > When a memory region is live, all its sub-regions and super-regions are > automaticall

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Awesome detective work! I glanced over the code, it looks great. I'd love to dedicate more time to your liveness-related patches, but university is a thing, so finding typos and the like

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-14 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment. This seems to be an important fix. Thank you! Did you measure decrease in the false-positive rate or an increase in the true-positive rate on real code? I expect some. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 181419. NoQ marked an inline comment as done. NoQ added a comment. Prettify the unittest a bit, especially the `ASTMatcher` part of it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56632/new/ https://reviews.llvm.org/D56632 Files: include/clang/Stat

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet, mgorny. This is a follow-up to D56042