<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>If you destroy the two intermediate layers i wouldn't mind.</div></blockquote><div>Then I will just extend the abstract interface and implement this in that way.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>On the other hand, our constraint manager is very much pushed to its <br>
limits where it is almost impossible to reason about the correctness and <br>
algorithmic complexity of the code we're writing.</div></blockquote><div>Definitely an issue.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>We should really do something about this whole technical debt thing and come</div><div> up with a better structure for our implementation before landing more large features.</div></blockquote><div>Unfortunately, I'm not qualified enough to guide this :|</div><div>The only thing I see is that really hard to strike the balance between accuracy and (time) complexity.<br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Everybody keeps saying "oh but it's just a tiny feature and <br>
it'll immediately make my use case better" but...</blockquote></div><div>It seems reasonable, but I think we really need to be able to compare symbols to symbols</div><div>(For now, only implemented for comparing pointers - I guess for modeling iterators).</div><div><br></div><div>In one of our previous <a href="http://lists.llvm.org/pipermail/cfe-dev/2020-March/064881.html">emails</a> you proposed the: <span style="font-family:monospace">ANALYZER_ASSERT(ANALYZER_EXTENT(buff) == size);</span></div><div>expressing hidden precondition assumptions to the analyzer.<br></div><div>Turns out the next obstacle is the constraint manager, which can not utilize this assumption, like in these cases:</div><div><span style="font-family:monospace">void symbol_constraints1(char *src, int n) {<br> assert(clang_analyzer_getExtent(src) > 10);<br> if (0 <= n && n < 10) {<br> // should be always true<br> clang_analyzer_eval(n < clang_analyzer_getExtent(src));<br> }<br>}<br>void symbol_constraints2(char *src, int len, int n) {<br> assert(clang_analyzer_getExtent(src) == len);<br> if (0 <= n && n < len) {<br> // should be always true<br> clang_analyzer_eval(n < clang_analyzer_getExtent(src));<br> }<br>}</span><br></div><div><br></div><div>As my plan was to hoist the diagnostic parts of the <span style="font-family:monospace">GenericTaintChecker</span> into eg. the <span style="font-family:monospace">CStringChecker</span>, I can not do it in a backward-compatible way.</div><div>Since <span style="font-family:monospace">CStringChecker</span> would warn for more cases, it would be fair to provide a way to suppress the new warnings. I think asserts like these, are the way forward.</div><div>But constraints are useless if the constraint manager can not utilize those.</div><div><br></div><div>Balazs<br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Artem Dergachev <<a href="mailto:noqnoqneo@gmail.com">noqnoqneo@gmail.com</a>> ezt írta (időpont: 2020. ápr. 7., K, 19:35):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">These layers never made much sense to me. We only really need two: <br>
`ConstraintManager` as the abstract base class and <br>
`RangeConstraintManager` as the particular implementation that we use. <br>
If you destroy the two intermediate layers i wouldn't mind.<br>
<br>
On the other hand, our constraint manager is very much pushed to its <br>
limits where it is almost impossible to reason about the correctness and <br>
algorithmic complexity of the code we're writing. We should really do <br>
something about this whole technical debt thing and come up with a <br>
better structure for our implementation before landing more large <br>
features. Everybody keeps saying "oh but it's just a tiny feature and <br>
it'll immediately make my use case better" but after a few dozen <br>
iterations it ends up being a mess that's almost impossible to debug <br>
when it causes a sudden performance drop.<br>
<br>
On 4/7/20 6:48 PM, Balázs Benics via cfe-dev wrote:<br>
> The constraint manager has limited functionality in comparing symbols <br>
> to symbols.<br>
> Though, it wouldn't be too hard to make it a bit smarter.<br>
><br>
> I would like to implement the following logic in the constraint <br>
> manager class hierarchy:<br>
> Given `a` and `b` well-constrained symbols.<br>
> We should know that `a` less than `b` if the possible maximum value of <br>
> `a` is still less than the minimum value of `b`.<br>
> In other words: `maxOf(a) < minOf(b) => a < b`<br>
><br>
> AFAIK `RangedConstraintManager` provides the interface to compare <br>
> symbols to constants.<br>
> The `RangeConstraintManager` class only implements that interface.<br>
><br>
> Should I extend the `RangedConstraintManager` interface to be able to <br>
> compare symbols to symbols?<br>
> Or, I could create another interface layer on top of <br>
> `RangedConstraintManager` interface to provide the necessary functions.<br>
><br>
> Which approach should I prefer?<br>
><br>
> Balazs<br>
><br>
> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br>
</blockquote></div>