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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to