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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 25 12:48:21 PST 2021


NoQ added a comment.

In D95307#2519283 <https://reviews.llvm.org/D95307#2519283>, @vsavchenko wrote:

> In D95307#2518592 <https://reviews.llvm.org/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 cornercase; i really hope to preserve it instead.
>
> You mean the failing assertion from the tests?

I mean, `isValidBaseClass()` is a function that's only ever used as part of an assert condition; it has no other purpose. The only thing this patch does is partially disable the assertion. The patch also disables a lot more of the assertion than necessary for it to not fail on the test.

Disabling the assertion has relatively little value on its own because assertions are already disabled in release/production builds of clang. We need to understand whether it is the assertion that's incorrect, or it's the surrounding code that's incorrect. Given that we're performing a derived-to-base cast when such cast is not necessary, that's a good indication that the problem is in the surrounding code, i.e. the behavior implemented in the transfer function isn't a correct abstraction over the actual runtime behavior of the program under analysis. It might be hard to judge because typed pointer values are a pretty questionable abstraction to begin with but i think at least some investigation is justified.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95307/new/

https://reviews.llvm.org/D95307



More information about the cfe-commits mailing list