[cfe-dev] [GSoC] Re: CSA constraint solver improvements

[Valeriy Savchenko via cfe-dev]

On 12 Apr 2021, at 03:30, Manas <manas18244 at iiitd.ac.in> wrote:
> I went through the code in RangeConstraintManager.cpp and comprehended
> about the constraint manager overall. canReasonAbout() (line: 1948)
> shows which all operations are left being implemented (namely, Mul, Div, 
> Shl, Shr, and also the Xor operator). I have added these into the
> proposal.
There is also an elephant in the room, - Add and Sub.
> 
>> However, we have found another application for z3 - refute produced warnings. When we are about to report a new warning, we can check that constraints are sat/unsat and discard the warning in the latter case.  This happens way less frequently and good in terms of performance.  Alas, the majority of users have static analyzer build without z3, and some false positives warnings sneak in.
>> 
>> So, there are two main directions that I see:
>> Figure out cases when z3 refutation works better than the built-in fast solver.  
>> Run the analyzer in both modes and analyze the difference.  Usually it’s under 10 warnings, so it won’t be very tedious.
> I have added this into the proposal too.
> 
>> Add reasoning about range-based binary operations.  
>> If we know ranges for symbols or symbolic expressions x and y, we can often reason about possible ranges for x OP y, where OP is some binary operator.  At the moment, we have support for &, |, and % (it’s a bit weird set of operators, but it was driven by reported false positives).
> Going through D80117, D79434, and D79336, one of the first things was to
> bisect the condition based on the sign of symbols. Followed by
> appropriate handling of upper/lower bounds for the expression.
Correct, we should always care about the bit vector nature of integers.

> 
> Regarding the TODO #1 (L:931) and TODO #2 (L:936) in RangeConstraintManager.cpp, 
> should I put these into the proposal too? (this to be done for other binary 
> operations as well apart from {rem, and, or} operations)
It didn’t seem to cause any performance problems so far, and I’d say this is not a priority right now.

> 
> These todos are related to lazy implemetation for VisitBinaryOperator and
> caching mechanism for analysis of nested expressions.
> 
>> It’s a logical continuation of my work:
>> D86465, D82445, D83286, D82381, D80117, D79434, and D79336
>> 
> I went through these patches as well in order to understand the workflow
> and things which have been done so far.
> 
> The description of project also mentions about a unit-test framework.
> Can I get more details regarding this?
As you looked through those patches, you probably noticed that we usually add more checks into constant-folding.c test.
It is pretty good, but it would be much better to have a dedicated section of unit-tests especially because the solver is pretty isolated and doesn’t really depend on AST (only on APSInt’s actually, but this is easy to overcome, you can see how it is done in RangeSetTest.cpp).

I guess my idea of perfect unit tests for the solver would be to have a special class for test symbols that we can construct like this TestSymbol x = {{0, 10}, {20, 40}} with overloaded operators that call solver, so we can do stuff like this: TestSymbol z = x + y; /* check for expected ranges in z */
This way the actual tests are very straightforward, easy to add new ones, easy to read and maintain.  And all the nitty gritty should be hidden inside of the test class and the test symbol class. The main challenge here is to design this testing framework, so it is as invisible as possible.
> 
> I have drafted the initial version of my proposal here.
> (https://docs.google.com/document/d/1ts_jxi1eAPGgeSzy5cs1dwZDcrmCHao8skZT8yWtOPI/edit?usp=sharing)
Awesome, I’ll take a look and respond in comments.

> 
> Thank you
> -- 
> Manas
> CSAM Undergraduate | 2022
> IIIT-Delhi, India

Cheers,
Valeriy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210412/649c45ab/attachment.html>