Skip to content

CONTRACTS: Support bounded user defined memory-predicates #7401

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

Conversation

remi-delmas-3000
Copy link
Collaborator

@remi-delmas-3000 remi-delmas-3000 commented Nov 29, 2022

Fixes #7194.

Users can now write their own pointer predicates in terms of:

  • __CPROVER_is_fresh(ptr, size)
  • __CPROVER_pointer_in_range(lb, ptr, ub)
  • __CPROVER_obeys_contract(ptr, target)

A rewriting pass lifts these user-defined pointer predicates
into predicates that take pointer by reference and can
make the predicates hold in assumption contexts
using side-effects, and check the predicates in assertion
contexts.

Limitiations:
User-defined predicates can be self-recursive but not mutually
recursive, and their evaluation needs to be terminate, i.e. they
can only describe bounded data structures.

The doxygen user and developer manuals and has been updated to describe the feature and its implementation.
Tests were added, and existing tests cleaned-up.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@remi-delmas-3000 remi-delmas-3000 added new feature aws Bugs or features of importance to AWS CBMC users aws-high Code Contracts Function and loop contracts labels Nov 29, 2022
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch 4 times, most recently from 030f804 to bf18bd4 Compare November 29, 2022 21:50
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch 7 times, most recently from f84fdc9 to 1ace3d4 Compare November 30, 2022 08:07
@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Base: 78.38% // Head: 78.41% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (3618d8a) compared to base (bb9d8e3).
Patch coverage: 91.69% of modified lines in pull request are covered.

❗ Current head 3618d8a differs from pull request most recent head c144993. Consider uploading reports for the commit c144993 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7401      +/-   ##
===========================================
+ Coverage    78.38%   78.41%   +0.03%     
===========================================
  Files         1651     1655       +4     
  Lines       190045   190264     +219     
===========================================
+ Hits        148961   149201     +240     
+ Misses       41084    41063      -21     
Impacted Files Coverage Δ
...contracts/dynamic-frames/dfcc_contract_handler.cpp 100.00% <ø> (ø)
...t/contracts/dynamic-frames/dfcc_contract_handler.h 100.00% <ø> (ø)
...strument/contracts/dynamic-frames/dfcc_library.cpp 95.54% <ø> (ø)
...t/contracts/dynamic-frames/dfcc_spec_functions.cpp 89.54% <ø> (+15.88%) ⬆️
...ent/contracts/dynamic-frames/dfcc_spec_functions.h 100.00% <ø> (ø)
src/libcprover-cpp/api.h 100.00% <ø> (ø)
...cts/dynamic-frames/dfcc_lift_memory_predicates.cpp 89.60% <89.60%> (ø)
src/ansi-c/c_typecheck_expr.cpp 75.68% <100.00%> (+0.08%) ⬆️
.../goto-instrument/contracts/dynamic-frames/dfcc.cpp 97.68% <100.00%> (+0.12%) ⬆️
...ument/contracts/dynamic-frames/dfcc_instrument.cpp 81.44% <100.00%> (+0.15%) ⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@remi-delmas-3000 remi-delmas-3000 self-assigned this Nov 30, 2022
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch 2 times, most recently from 3815192 to 0d242c2 Compare December 2, 2022 21:56
@feliperodri feliperodri changed the title CONTRACTS: support bounded user defined memory-predicates CONTRACTS: Support bounded user defined memory-predicates Dec 5, 2022
Copy link
Collaborator

@feliperodri feliperodri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some minor changes before approval.

@remi-delmas-3000
Copy link
Collaborator Author

remi-delmas-3000 commented Dec 7, 2022

A reader might believe that __CPROVER_is_in_range ought to apply to types other than pointers, e.g., integers, or floating-point numbers. It may make sense to make that predicate polymorphic. Can you simply use __CPROVER_pointer_in_range?

Will fix.

EDIT:
My instrumentation pass maps predicate calls to built-in predicates to special implementations that takes the dynamic frame as parameter and to make it context-aware (assert vs. assume).

Right now the front-end maps __CPROVER_pointer_in_range calls to pointer_in_range_exprt expressions that get lowered during goto-conversion, so when my pass happens pointer_in_range_exprt is not there for me to match and replace anymore.

@tautschnig is currently working on moving the lowering of built-ins to the back end #7395, which would solve my problem by keeping pointer_in_range_exprt intact out of goto-conversion.

The best I can do for the moment is to temporarily rename my __CPROVER_is_in_range predicate to __CPROVER_pointer_in_range_dfcc so it does not get lowered during goto conversion and my pass can still work.

I'll update my code to pointer_in_range_exprt when #7395 gets merged.

@kroening Would that work for you ?

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch from eda68f4 to cf6fd73 Compare December 7, 2022 21:38
Comment on lines 2337 to 2359
else if(identifier == CPROVER_PREFIX "is_in_range")
{
// same as pointer_in_range with experimental feature for DFCC contracts
// -- do not use
if(expr.arguments().size() != 3)
{
throw invalid_source_file_exceptiont{
"is_in_range expects three arguments", expr.source_location()};
}

for(const auto &arg : expr.arguments())
{
if(arg.type().id() != ID_pointer)
{
throw invalid_source_file_exceptiont{
"is_in_range expects pointer-typed arguments", arg.source_location()};
}
}
return nil_exprt();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this (and any other support for __CPROVER_is_in_range) please go in a commit of its own? #7395 should make it completely unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be in line with what's suggested in #7401 (comment) (except it would make the follow-up work very easy to identify).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved all pointer_in_range related code to a separate commit

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch 8 times, most recently from 7d57611 to e0643b4 Compare December 8, 2022 15:15
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch from e0643b4 to 3618d8a Compare December 8, 2022 16:00
Users can now write their own pointer predicates in terms of:
- __CPROVER_is_fresh(ptr, size)
- __CPROVER_obeys_contract(ptr, target)

A rewriting pass lifts these user-defined pointer predicates
into predicates that take pointers by reference and can
make the predicates hold in assumption contexts using
side-effects, and check the predicates in assertion contexts.

Limitiations:
User-defined predicates can be self-recursive but not mutually
recursive, and their evaluation needs to be terminate, i.e. they
can only describe bounded data structures.
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch from 3618d8a to df1551e Compare December 8, 2022 16:07
@remi-delmas-3000
Copy link
Collaborator Author

@feliperodri all requested changes were applied

Copy link
Collaborator

@feliperodri feliperodri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch from df1551e to 2653d78 Compare December 8, 2022 16:17
Adds support for pointer_in_range_dfcc in ensures and
requires clauses and user-defined predicates. This is a
temporary workaround to the fact that pointer_in_range
is lowered by the front-end.
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-lift-memory-predicates branch from 2653d78 to c144993 Compare December 8, 2022 16:24
@remi-delmas-3000 remi-delmas-3000 merged commit 8f8d169 into diffblue:develop Dec 8, 2022
TGWDB added a commit to TGWDB/cbmc that referenced this pull request Dec 8, 2022
NlightNFotis added a commit that referenced this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Bugs or features of importance to AWS CBMC users aws-high Code Contracts Function and loop contracts new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Predicate to describe overlapping pointers
4 participants