-
Notifications
You must be signed in to change notification settings - Fork 88
Affine Equalities #592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Affine Equalities #592
Conversation
I am immediately confused by this. If the new affeq domain/analysis uses Apron environments, variables and expressions, then it's not really independent from Apron. If Apron isn't installed, then the affeq domain cannot even be compiled. To me, this is a particularly weird choice of abstraction, because it would be more logical to use non-Apron variables, expressions and environments (like the base analysis does) for the domain, which intends to be separate from Apron's. I would've expected the common interface between Apron domains and this affeq domain to use By still requiring the presence of Apron, the point of having this domain is defeated, because if I already have Apron, I might as well use the equivalent equalities domain provided by it. That's very likely more efficient and more reliable. |
@sim642 |
Currently (on A combination like octagon and equalities wouldn't fall under that, so isn't currently directly supported. But achieving that doesn't require implementing one of the two from scratch! Thanks to the genericity of Alternatively, Apron has (reduced) products of its domains, which could achieve the same. |
True, there is indeed no affeq on master at the moment but adding it required just a few more lines of code. Having a completely apron-free implementation also didn't make sense for us since we wanted to keep the apron abstract syntax tree plus its conversion functions. For comparing our affeq implementation with the apron one this was somewhat crucial as we didn't want ours to suffer from wrong/incomplete CIL expression handling. Thus, keeping it also gave us a better common basis for the comparison. Do you perhaps have some remarks on the code as well? I would be really glad to get as much feedback as possible since the implementation is an integral part of my thesis. |
|
Why does this PR update |
|
This is a huge diff (by necessity) and that makes it very hard to review. Could you maybe provide an overview over the different modules (and module types) that are now used (there was a lot of them before already), and I think we now have more that are even more involved? This would be similar in spirit to the overview over the different modules you give in the description of the PR, but at a higher level of detail. Also, it would be very helpful to indicate which files contain significant new code, and which contain code that has been moved from somewhere else. I think that would make it easier (for all of us) to review the PR in detail in detail and also offer more specific suggestions for some of the parts. |
I've added a more detailed description to my initial comment. |
But the Apron analyzer/src/cdomains/apron/apronDomain.apron.ml Lines 158 to 200 in 4c7576a
All it would've taken to not depend on Apron was in the affeq implementation to pattern match on CIL's BinOp (PlusA, ...) instead of Apron's Binop (Add, ...).
I guess there's not much to be done about it at this point, but it's a weird early design decision that now is deeply integrated in this. It would require major refactoring now to change. |
This point is easily missed when looking at the diffs because a rename is also involved, but this is a no-go. The in-place functions of Apron are a feature and we very intentionally use them to achieve better performance. Because the functional equivalents always involve a memory copy and then do that in-place operation on the new memory. There's a reason Apron provides these. By reverting that optimization you're (maybe unintentionally) shooting the Apron domains in the foot and crippling their performance, which invalidates the core goal of these changes to compare the two. |
This is also a major point when it comes to performance and its comparison. Functional lists are infamously inefficient for the kinds of operations vectors and matrices are used for here (indexing). If I were to guess, these list manipulations make up a major bottleneck. |
f93e316 to
ff2b331
Compare
That makes sense. We've added the in-place functions to relationAnalysis again and indicated by a suffix "_pt_with" that the functions could have potential side-effects (which is the case for all "_pt_with" labeled functions in the apron domain but not for the ones inside affeq) 236b387 |
4e31e16 to
0d8374d
Compare
Okay a quick update on this. We have added a matrix data structure that relies on the |
I've manually reduced one the smallest SV-Comp test where the mistake occured. The test fails one commit before the fix. After the fix commit it works perfectly fine of course. |
|
@michael-schwarz I think I've now done all the tinkering on this that I intended, so you can also have a final look. Just a reminder here at the end again: this is to be squash merged. |
michael-schwarz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @MartinWehking and thanks @sim642 for the improvements!
|
I guess we'll also need to make changes in https://github.com/goblint/bench now that this is merged. |
CHANGES: * Add `setjmp`/`longjmp` analysis (goblint/analyzer#887, goblint/analyzer#970, goblint/analyzer#1015, goblint/analyzer#1019). * Refactor race analysis to lazy distribution (goblint/analyzer#1084, goblint/analyzer#1089, goblint/analyzer#1136, goblint/analyzer#1016). * Add thread-unsafe library function call analysis (goblint/analyzer#723, goblint/analyzer#1082). * Add mutex type analysis and mutex API analysis (goblint/analyzer#800, goblint/analyzer#839, goblint/analyzer#1073). * Add interval set domain and string literals domain (goblint/analyzer#901, goblint/analyzer#966, goblint/analyzer#994, goblint/analyzer#1048). * Add affine equalities analysis (goblint/analyzer#592). * Add use-after-free analysis (goblint/analyzer#1050, goblint/analyzer#1114). * Add dead code elimination transformation (goblint/analyzer#850, goblint/analyzer#979). * Add taint analysis for partial contexts (goblint/analyzer#553, goblint/analyzer#952). * Add YAML witness validation via unassume (goblint/analyzer#796, goblint/analyzer#977, goblint/analyzer#1044, goblint/analyzer#1045, goblint/analyzer#1124). * Add incremental analysis rename detection (goblint/analyzer#774, goblint/analyzer#777). * Fix address sets unsoundness (goblint/analyzer#822, goblint/analyzer#967, goblint/analyzer#564, goblint/analyzer#1032, goblint/analyzer#998, goblint/analyzer#1031). * Fix thread escape analysis unsoundness (goblint/analyzer#939, goblint/analyzer#984, goblint/analyzer#1074, goblint/analyzer#1078). * Fix many incremental analysis issues (goblint/analyzer#627, goblint/analyzer#836, goblint/analyzer#835, goblint/analyzer#841, goblint/analyzer#932, goblint/analyzer#678, goblint/analyzer#942, goblint/analyzer#949, goblint/analyzer#950, goblint/analyzer#957, goblint/analyzer#955, goblint/analyzer#954, goblint/analyzer#960, goblint/analyzer#959, goblint/analyzer#1004, goblint/analyzer#558, goblint/analyzer#1010, goblint/analyzer#1091). * Fix server mode for abstract debugging (goblint/analyzer#983, goblint/analyzer#990, goblint/analyzer#997, goblint/analyzer#1000, goblint/analyzer#1001, goblint/analyzer#1013, goblint/analyzer#1018, goblint/analyzer#1017, goblint/analyzer#1026, goblint/analyzer#1027). * Add documentation for configuration JSON schema and OCaml API (goblint/analyzer#999, goblint/analyzer#1054, goblint/analyzer#1055, goblint/analyzer#1053). * Add many library function specifications (goblint/analyzer#962, goblint/analyzer#996, goblint/analyzer#1028, goblint/analyzer#1079, goblint/analyzer#1121, goblint/analyzer#1135, goblint/analyzer#1138). * Add OCaml 5.0 support (goblint/analyzer#1003, goblint/analyzer#945, goblint/analyzer#1162).
This draft PR introduces an analysis for affine equalities (ref: Michael Karr , 1976) that can be run independently from the already existing apron analyses.
Abstract states in the newly added domain are represented by structs containing a matrix and an apron environment.
Matrices are modeled as proposed by Karr: Each variable is assigned to a column and each row represents a linear affine relationship that must hold at the corresponding program point.
The apron environment is hereby used to organize the order of columns and variables.
Furthermore, the apron analysis and its respective privatization modules have been modified to fit the needs for apron-independent relational analyses.
A short list giving a quick (and perhaps not full) overview of what's been added/modified:
If an analysis returns bounds that are unequal to min and max of ikind , we can exclude the possibility that an overflow occurs and the abstract effect of the expression assignment can be used, i.e. we do not have to set the variable's value to top.
Additionally, we now also refine after positive guards when overflows might occur and there is only one variable inside the expression and the expression is an equality constraint check (==). We check after the refinement if the new value of the variable is outside its integer bounds and if that is the case, either revert to the old state or set it to bottom.
The new analysis can be activated by the following parameter:
--set ana.activated[+] affeqIn order to compare affeq with the apron affine equalities implementation, the affeq analysis inside apron can be selected by the following parameter (when apron is activated):
--set ana.apron.domain "affeq"--set ana.affeq.matrix list/arrayA more detailed descriptions of all files/modules and interfaces:
-affineEqualityDomain (contains mostly new code)**:
Modules
-VarManagement: functor parameterized by
AbstractMatrixandAbstractVector. It defines the type t of the affine equality domain (a struct that contains an optional matrix and an apron environment) and provides the functions needed for handling variables (which are defined byRelationDomain.D2) such asadd_varsremove_vars. Furthermore, it provides the functionget_coeff_vecthat parses an apron expression into a vector of coefficients if the apron expression has an affine form.-ExpressionBounds: functor that takes the same arguments as VarManagement. It's interface is
SharedFunctions.ConvBoundsand calculates the bounds of anapron expression. As it is specifically used for the new affine equality domain, it can only provide bounds if the expression contains known constants only and in that case, min and max are the same.RelationDomain.D2(including the lattice operations such asmeet,join, etc.) and the main code for the new affine equality domain.vectorMatrix
Interfaces/module types
Mpqfmodule for rationals from Apron that implements multi-precision rationals.One could later exchange "Mpqf" with a different module that provides the functions specified by this interface.
Vectorand takesA:RatOpsas argument .It substitutes the abstract type num (short for "numerical") ofVectorwith type t of moduleA. Some functions inside have the suffix _pt_with,which means that the function could have potential side effects
A:RatOpsandV:AbstractVector. Similarly to AbstractVector, AbstractMatrix substitutesnumofMatrixwith type t ofA:RatOpsbut also the abstract type vec with type t of a moduleV:AbstractVector. Some functions inside have the suffix _pt_with,which means that the function could have potential side effects
Modules
RatOps. It provides more readable infix operators for the functions ofRatOps. It is designed to be included by modules that make use ofRatOps's functionsAbstractVectorwith type tA.t List.t. Reuses most of the already existingListfunctions such asmap2,rev, etc. and adds some for vector manipulations (e.g.insert_valremove_val...). None of its functions has side-effectsAbstractMatrixwith typeV.t list. In contrast toListVector, it does not include the functions ofListdirectly but uses them to provide unique ones. It also includes variousnormalizefunctions that transform a matrix into reduced row echelon form. These are in general not efficient as they perform a full Gauss-Jordan elimination that is slow for lists.AbstractVectorwith type tA.t array(from theBatterieslibrary). Provides more efficient side-effecting functions thanListVector. Provides a copy function since the array data structure it contains is mutable.AbstractMatrixwith typeA.t array array(from theBatterieslibrary). Provides more efficient side-effecting functions thanListMatrix. Similar toListMatrix, it provides a normalization function to reduce a matrix into reduced row echelon form. However, its normalization functions are faster not only because they use faster side-effect functions but also because they exploit that the input matrix/matrices are in reduced row echelon form alreadyApronDomain (Contains most of its old code. Some parts have been moved. Not much new has been added)
Changes:
affineEqualityDomainaffineEqualityDomainovparameter has been added to functions where functions of the Convert module are used. This is to also to make used of the improved overflow handling.assert_inv,eval_interval_exprandeval_inthave been moved into a newAssertionModuleinsideSharedFunctionsas they are also used byaffineEqualityDomainRelComponentand moved to RelationDomainRelationDomain.D2).SharedFunctions.AssertionModuleand D2Complete. It contains all functions specified byRelationDomain.RelD2. AD2Complete is required byApronPrecCompareUtilRelationDomain (Contains some of ApronDomain's code and new interfaces)
New module types/ Interfaces
Varthat was insideapronDomain(Moved tosharedFunctions).RelationAnalysis. It includes for instance lattice functions such asjoin,meet...Some of the functions end with the suffix _pt_with which indicates that the function could have potential side effects.
In that case, the functions are supposed to return their modified input.
AssertionModulein `sharedFunctions'RelationAnalysis.SpecFunctor. The 3 modules that it contains must be implemented for each analysis and bundled into a module of this type. It is intended to group the implementation for variables and the domain.Moved modules + module types/ interfaces
ApronDomainintoRelationDomain. Functor arguments for RelVar have been added.ApronDomainand accepts RelD2 now as functor argument instead of ApronDomain.S2SharedFunctions (Contains mostly code from
apronDomainwhich has been adapted to fit to affineEqualityDomain`)Moved modules + module types/interfaces
apronDomainapronDomain. It takes a module of type ConvBounds as functor argument now. Used to convert CIL expressions to apron expressions.New module types/interfaces
RelationDomain.RelD2with ConvBounds integrated and various apron elements. It is designed to be the interface for the D2 modules inaffineEqualityDomainandapronDomainand serves as a functor argument for AssertionModuleNew modules
add_vars,remove_varsetc. inapronDomainandaffineEqualityDomainRelationAnalysisand defined byRelationDomain.RelD2and are exactly the same forapronDomainandaffineEqualityDomain, i.e. they don`t have to be defined twice inside both domains. It uses the functions of its functor argument AssertionRelD2RelationAnalysis (Contains the
SpecFunctorofapronDomain)RelationDomain.RDnow instead ofApronDomain.S2and uses the domain and variable provided by it.It also accepts
RelationPrecCompareUtil.Utilnow as functor argument asApronPrecCompareUtilusages inside have been removed.Some of the definitely side-effecting functions ending with _with are now expected to only potentially have side effects and
their return values are therefore utilized.
ApronAnalysis
RelationAnalysisand extends it by data storing methods etc. It also passesApronPrecCompareUtiltoRelationAnalysis.SpecFunctor.spec_module,get-spec) for apron analyses are also included and taken from the oldapronAnalysis. They were slightly adapted to the new module structure (e.g. module RD with apron domain etc. are initialized inside)AffineEqualityAnalysis
ApronAnalysis, initialization methods for the new affine equality analysis were added. However, it does not add specific data storing methods toRelationAnalysis.SpecFunctorand passesRelationPrecCompareUtil.DummyUtilinstead of an apron-specific implementation to theSpecFunctorRelationPrecCompareUtil and ApronPrecCompareUtil