NoQ added a comment. In D62883#1542802 <https://reviews.llvm.org/D62883#1542802>, @Szelethus wrote:
> Some conclusions: > > - Cases where the condition is also a variable initialization tend to expand > the bug path greatly. This isn't always bad, but should be noted. In general, > unless we have a good heuristic to figure out whether there is meaningful > information on the right hand side of the initialization, I think we just > shouldn't track these. A good example for this is this one > <http://cc.elte.hu:15001/Default/#is-unique=on&report-hash=&run=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&reportHash=20eb450bc76b250d0ffa40e0435d3823&report=2998&subtab=20eb450bc76b250d0ffa40e0435d3823>. > The 37th event contains pretty much every information we need, it's obvious > that the optional could either be None or non-None, since it's in a > condition. dyn_cast is a prime example too: in this case > <http://cc.elte.hu:15001/Default/#is-unique=on&report-hash=&run=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&reportHash=ae426512e10d1835e2b80b594d41226a&report=3126&subtab=ae426512e10d1835e2b80b594d41226a>, > note 9 is all we really need. > - We should probably believe that `operator bool()` is implemented correctly, > and shouldn't track the value all the way there (at least, when we're only > tracking the condition). Example > <http://cc.elte.hu:15001/Default/#is-unique=on&report-hash=&run=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&reportHash=cb40d620796d9fb5257657817766ef6c&report=3775&subtab=cb40d620796d9fb5257657817766ef6c> > - We shouldn't ever track assert-like conditions. Example > <http://cc.elte.hu:15001/Default/#is-unique=on&report-hash=&run=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%2FClang-tools-extra%20AFTER%20tracking%20conditions&reportHash=4a863a6cf99be5676bc7d1d9aedccfb1&report=3087&subtab=4a863a6cf99be5676bc7d1d9aedccfb1> > (note 38-41) > - When the report didn't suffer from any of the above issues, I found the > extra notes to be helpful! :D 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? ================ Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:66 +void foo() { + // TODO: It makes no sense at all for bar to have been assigned here. + flag = coin(); // expected-note {{Value assigned to 'flag'}} ---------------- It's invalidation of globals by `coin()` which potentially writes to `bar`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62883/new/ https://reviews.llvm.org/D62883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits