[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

Fri Jun 14 17:05:01 PDT 2019

Szelethus added a comment.

In D62883#1544283 <https://reviews.llvm.org/D62883#1544283>, @NoQ wrote:

> However, this heuristic breaks when the same code appears in an inlined stack frame. Because given the context, we need to prove that this check makes sense *in this context*. I strongly suspect that all of the heuristics that you're talking about depend significantly on the surrounding call stacks.

I don't follow you here, unfortunately, could you please elaborate? Also, I'll come out with this right now: I never really understood these contexts, is there a handy doc or a paper I could take a look at?

> Like, i suspect it so strongly that i'm ready to admit that i've no idea what we're doing anymore, and it makes sense to take a pen and a sheet of paper and try to write down the tracking rules, and then come up with an easily editable and extensible architecture for tweaking those rules as we discover more and more of them.

That sounds great. The should-not-have-happened and the must-have-happened cases will all rely on this (since in both cases, at least for now, we'd like to use the extra information to track conditions), so maybe it's due to refresh `trackExpressionValue`. It would also make publishing and reviewing my works a lot easier.

In D62883#1544302 <https://reviews.llvm.org/D62883#1544302>, @NoQ wrote:

> On the other hand, all of these problems seem to be examples of the problem of D62978 <https://reviews.llvm.org/D62978>. Might it be that it's the only thing that we're missing? Like, we need to formulate a rule that'll give us an understanding of when do we track the value past the point where it has been constrained to true or false - do we care about the origins of the value or do we care about its constraints only? In case of `flag` in the test examples, we clearly do. In case of these bools that come from boolean conversions and assertions and such, we clearly don't. What's the difference?
>
> How about we track the condition value past its collapse point only if it involves an overwrite of a variable (or other region) from which the value is loaded? Like, if there are no overwrites, stop at the collapse point. If there are overwrites, stop at the oldest overwrite point (i.e., the value was loaded from 'x' which was previously overwritten by the value of value 'y' which was previously overwritten by the initial value of variable 'z' => stop at the overwrite point of 'y').
>
> (then, again, not sure what happens if more nested stack frames are around)

For now, I'd like to restrict my efforts to simply "we should track this"  or "we shouldn't track this". I think simply not tracking cases where the condition is a single variable assignment is a good initial approach. Let how the tracking is done be a worry for another day. Also, I'm a little confused, isn't this comment about how D62978 <https://reviews.llvm.org/D62978> could be solved?

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

https://reviews.llvm.org/D62883