Skip to content

CONTRACTS: loop assigns clause inference function #7629

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

Merged

Conversation

remi-delmas-3000
Copy link
Collaborator

@remi-delmas-3000 remi-delmas-3000 commented Mar 28, 2023

The first commit is from #7636, do not review.

Co-authored with @qinheping

Loop assigns clause inference function. Uses loop_utils.h to get a set of lvalue expressions modulo aliasing that may be assigned by the loop.
The lvalue expressions are then post processed.

  • If an inferred target depends on a loop-local variable (i.e. a variable that does not exist outside of the loop body) it is widened to its base object. If the base object still depends on a loop local, the target is discarded, otherwise __CPROVER_object_whole(&root_object) is kept as inferred target.
  • If the address of an inferred target is non-constant, then __CPROVER_object_whole(&target) is used as widened target.

Extracted from #7541, can only be tested once all features are in place.

  • 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.

@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (53b50bc) 78.51% compared to head (1b899b1) 78.51%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #7629   +/-   ##
========================================
  Coverage    78.51%   78.51%           
========================================
  Files         1674     1674           
  Lines       191935   191935           
========================================
  Hits        150704   150704           
  Misses       41231    41231           

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch from 2bd210a to fc1cd2a Compare March 29, 2023 01:04
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch 4 times, most recently from 025ff40 to 6303032 Compare April 3, 2023 17:13
@remi-delmas-3000
Copy link
Collaborator Author

@tautschnig @qinheping I just pushed a revised version that uses root objects to widen targets, should be stable now

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch from 6303032 to c231116 Compare April 3, 2023 17:44
@feliperodri feliperodri added aws Bugs or features of importance to AWS CBMC users aws-high Code Contracts Function and loop contracts labels Apr 3, 2023
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch from c231116 to 32ad1ea Compare April 3, 2023 19:25
@feliperodri feliperodri requested a review from tautschnig April 3, 2023 20:51
@feliperodri feliperodri requested a review from qinheping April 3, 2023 21:00
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch 2 times, most recently from 9f87703 to 62e0f74 Compare April 4, 2023 16:40
Copy link
Collaborator

@qinheping qinheping 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-infer-assigns-clause branch from 62e0f74 to aeaad9a Compare April 6, 2023 15:07
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch 2 times, most recently from 5def6b7 to 8285444 Compare April 6, 2023 20:38
@feliperodri
Copy link
Collaborator

@tautschnig if you have the time, could you do another round of review on this PR?

@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch from 8285444 to c802c2d Compare April 11, 2023 03:37
Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

Just minor cleanup, will be good to go once the depended-on PR is merged.

Comment on lines +92 to +96
// Target address is not constant, widening to the whole object
result.emplace(make_object_whole_call_expr(address_of_expr, ns));
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens behind the scenes with a pointer passed to object-whole? Would it be the same as the root_object case above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The meaning of object_whole(&expr) or object_whole(&root_object(expr)) is the same so we don't bother computing the root object. In the first case we compute the root_object to try and get rid of a dependency on loop locals or drop the target if the root object is still local to the loop. In the second case we have a non-constant expression that does not depend on loop locals, and we just widen it directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we have to use object_whole because this is the predicate that DFCC understands to specify byte ranges.

Remi Delmas added 2 commits April 14, 2023 17:15
Alternative implementation of `root_object`
supporting ternary operators in assignment or call
LHS expressions, and object slice expressions in
assigns clause targets.
@remi-delmas-3000 remi-delmas-3000 force-pushed the contracts-infer-assigns-clause branch from c802c2d to 1b899b1 Compare April 14, 2023 18:54
@remi-delmas-3000
Copy link
Collaborator Author

@tautschnig all suggestions implemented, could you please take a last look ?

@tautschnig tautschnig disabled auto-merge April 18, 2023 10:06
@tautschnig tautschnig merged commit aeda79a into diffblue:develop Apr 18, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants