-
Notifications
You must be signed in to change notification settings - Fork 265
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
CONTRACTS: refactor DFCC code for loop contracts #7551
CONTRACTS: refactor DFCC code for loop contracts #7551
Conversation
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.
I have not reviewed the contracts/dynamic-frames
part. @remi-delmas-3000 I can if you want. The rest looks good but I notice the overlap with #7550 so it would be good to have this rebased when it is merged.
c78a3cc
to
bb8e8e8
Compare
ed8ac89
to
ad9e93f
Compare
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.
// TODO specialise the library functions for the max size of | ||
// loop and function contracts |
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.
Shall we create a GitHub issue about this and add the link here? It might be better to track this later.
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.
this is still work in progress this will already be addressed in the final version
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.
DELETED
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.
But then ... haven't you resolved the "TODO" and should remove this comment?
This has to be implemented in the forthcoming loop contracts PR, this TODO is just a marker that tells you where to do it
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.
Sorry this one is a TODO for @qinheping , my comment above applied to two other TODOs I've turned into reqs.
src/goto-instrument/contracts/dynamic-frames/dfcc_contract_clauses_codegen.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_contract_clauses_codegen.cpp
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_spec_functions.cpp
Outdated
Show resolved
Hide resolved
bc8071e
to
961bc8c
Compare
961bc8c
to
58e87a3
Compare
58e87a3
to
f7ae59e
Compare
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.
Approving with a couple of comments that really need to be addressed.
src/goto-instrument/contracts/dynamic-frames/dfcc_contract_clauses_codegen.cpp
Outdated
Show resolved
Hide resolved
// TODO specialise the library functions for the max size of | ||
// loop and function contracts |
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.
But then ... haven't you resolved the "TODO" and should remove this comment?
This has to be implemented in the forthcoming loop contracts PR, this TODO is just a marker that tells you where to do it
std::set<irep_idt> function_pointer_contracts; | ||
instrument.instrument_function(function_id, function_pointer_contracts); | ||
INVARIANT( | ||
function_pointer_contracts.size() == 0, |
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.
You might want to use .empty()
instead of .size() == 0
.
src/goto-instrument/contracts/dynamic-frames/dfcc_spec_functions.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_spec_functions.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_spec_functions.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_wrapper_program.cpp
Outdated
Show resolved
Hide resolved
Allows to to inlining and loop detection directly on a goto program instead of a goto function
- Factor out some methods in dfcc_spec_functionst - Move the code generation methods from dfcc_contract_functionst to dfcc_contract_clauses_codegent. - Update Makefile with new class. - Propagate interface changes to top level class
Enable checking for the presence of dynamic allocations or deallocations in loop bodies and transitively in the functions called from loop bodies. - Adds `allow_allocate` and `allow_deallocate` flags in the write set struct - Modify `add_allocated` to fail an assertion when called on a write set with `allow_allocate` set to false. - Modify `check_deallocate` to return false when called on a write set with `allow_deallocate` set to false. - Don't treat replacement mode specially anymore, always store freeable targets in both the object map (for lookups) and in the append list (for inclusion checks).
1e354f7
to
fb35a17
Compare
This PR refactors the DFCC code to make it possible to handle loop assigns clauses in #7541.
We now have a standalone class
dfcc_contract_clauses_codegent
which provides methods to generate GOTO programs that initialize a dynamic frame instance from an assigns clause specification or havoc the targets of an assigns clause.Before, these code generation methods were encapsulated inside the class
dfcc_contract_functionst
which exclusively handles function contracts. This makes them available for loop contracts as well.