[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-02-01 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. In D95307#2533259 , @vsavchenko wrote: > I don't know if this is useful or not, but pointer-to-members used to be > `void *` up until very recently. Here is my change on that topic (and it > probably where the bug you're trying

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-02-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2532669 , @RedDocMD wrote: > I clearly am taking the wrong approach to this problem. I will need to > reconsider how PointerToMember is actually modelled before solving this > problem I don't know if this is useful

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-31 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD abandoned this revision. RedDocMD added a comment. I clearly am taking the wrong approach to this problem. I will need to reconsider how PointerToMember is actually modelled before solving this problem Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-28 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. @kromanenkov I guess it won't work then. Thanks for the idea! Now that I think about it removing duplicates is probably not the right thing to do. The problem arises because the call PTMD->getCXXBaseList() should ideally return the list of previous casts, but instead r

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-27 Thread Kirill Romanenkov via Phabricator via cfe-commits
kromanenkov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:199 - for (const auto &I : llvm::reverse(PathRange)) -PathList = prependCXXBase(I, PathList); + llvm::SmallPtrSet BaseTypes; + for (const auto &BaseSpec : PathList) ---

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-27 Thread Kirill Romanenkov via Phabricator via cfe-commits
kromanenkov added a comment. Looks like you run formatter on the whole file, maybe narrow down its scope a little? For example, only for the touched function? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95307/new/ https://reviews.llvm.org/D95307

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-27 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 319607. RedDocMD added a comment. Removing duplication of CXXBaseSpecifiers when they are created for PointerToMember Removed hackish weakening of assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95307/new

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-26 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. On further digging as to how pointer-to-members are handled in StaticAnalyzer, I discovered that in `BasicValueFactory::accumCXXBase() for the example given in my test produces the two `CXXBaseSpecifier` for the pointer-to-member. At line 195, one base specifier is add

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. Thanks @NoQ for the tip on the right test command! My own belief is that this patch is just a hack that squelches the problem, instead of solving it. I am limited by my own inexperience with the codebase and am trying to learn more to help better. That said, couple of

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D95307#2519597 , @vsavchenko wrote: > In D95307#2519309 , @RedDocMD wrote: > >> Funnily enough, when I run `ninja clang-check` I don't get any errors. > > I believe that `ninja check-clang` i

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D95307#2519283 , @vsavchenko wrote: > In D95307#2518592 , @NoQ wrote: > >> This patch shoots the messenger but someone still needs to conduct a proper >> investigation. The assertion is losi

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2519309 , @RedDocMD wrote: > Funnily enough, when I run `ninja clang-check` I don't get any errors. I believe that `ninja check-clang` is the right command (clang-check is a tool) :-) Repository: rG LLVM Github M

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. Funnily enough, when I run `ninja clang-check` I don't get any errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95307/new/ https://reviews.llvm.org/D95307 ___ cfe-commits m

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2519300 , @RedDocMD wrote: > In D95307#2519283 , @vsavchenko > wrote: > >> In D95307#2518592 , @NoQ wrote: >> >>> This patch shoots the

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment. In D95307#2519283 , @vsavchenko wrote: > In D95307#2518592 , @NoQ wrote: > >> This patch shoots the messenger but someone still needs to conduct a proper >> investigation. The assertion is

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2518592 , @NoQ wrote: > This patch shoots the messenger but someone still needs to conduct a proper > investigation. The assertion is losing a lot more of its bug-finding power > than necessary to uncover the current

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-24 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added reviewers: vsavchenko, xazax.hun, martong, kromanenkov. NoQ added a comment. Herald added a subscriber: rnkovacs. This patch shoots the messenger but someone still needs to conduct a proper investigation. The assertion is losing a lot more of its bug-finding power than necessary to unc

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Looks fine to me. Thank you for addressing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95307/new/ https://reviews.llvm.org/D95307 ___ cfe-commits mailing list cfe-commit

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-24 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD created this revision. Herald added subscribers: steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, a.sidorin, szepet, baloghadamsoftware. RedDocMD requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the function isVa