Skip to content

CONTRACTS: Front-end: frees clause improvements #7087

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 Aug 25, 2022

Depends on #7086.

The first commit in this PR is from #7086, please do not read.

The second commit adds the following to the front-end:

  • add __CPROVER_freeable_t built-in type
  • add __CPROVER_freeable built-in function
  • allow calls to __CPROVER_freeable_t functions in frees clauses
  • add __CPROVER_is_freeable built-in predicate
  • add __CPROVER_is_freed built-in predicate

The predicates are not yet supported in the back-end and behave as nondet functions

The build fails with ubuntu 18.04 and I am not sure why, since no error is visible on the output.

  • 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).
  • [n/a] 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 Aug 25, 2022

Codecov Report

Merging #7087 (eeb0728) into develop (9bbce17) will decrease coverage by 0.00%.
The diff coverage is 79.52%.

@@             Coverage Diff             @@
##           develop    #7087      +/-   ##
===========================================
- Coverage    77.85%   77.84%   -0.01%     
===========================================
  Files         1574     1574              
  Lines       181235   181435     +200     
===========================================
+ Hits        141099   141246     +147     
- Misses       40136    40189      +53     
Impacted Files Coverage Δ
src/ansi-c/ansi_c_convert_type.h 100.00% <ø> (ø)
src/ansi-c/ansi_c_internal_additions.cpp 90.12% <ø> (ø)
src/ansi-c/c_typecheck_base.h 100.00% <ø> (ø)
...c/solvers/smt2_incremental/convert_expr_to_smt.cpp 82.35% <ø> (ø)
..._incremental/smt2_incremental_decision_procedure.h 75.00% <ø> (ø)
src/util/replace_symbol.cpp 76.89% <0.00%> (-1.32%) ⬇️
...o-instrument/contracts/instrument_spec_assigns.cpp 96.92% <42.85%> (-2.44%) ⬇️
src/ansi-c/c_typecheck_expr.cpp 76.77% <48.97%> (-0.58%) ⬇️
src/util/rename_symbol.cpp 76.33% <60.00%> (-0.50%) ⬇️
src/ansi-c/parser.y 80.34% <70.83%> (-0.14%) ⬇️
... and 16 more

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

@remi-delmas-3000 remi-delmas-3000 force-pushed the frees-clause-improvements branch 2 times, most recently from 7edced4 to 62a0609 Compare August 26, 2022 04:10
Add new functions to specify assignable
targets in assigns clauses:
- add type __CPROVER_assignable_t
- add builtin __CPROVER_assignable
- add builtin __CPROVER_whole_object
- add builtin __CPROVER_object_upto
- add builtin __CPROVER_typed_target
- allow function call expressions as assigns
  clause targets as long as they have the
  return type __CPROVER_assignable_t and
  are one of the built-ins.

Add support for `__CPROVER_frees()` clauses
to the contract language to let users specify
the pointers a function (or loop) is allowed
to free.
@remi-delmas-3000 remi-delmas-3000 force-pushed the frees-clause-improvements branch 2 times, most recently from 626fac5 to 133116d Compare August 26, 2022 04:31
Add the following to the front-end:
- add `__CPROVER_freeable_t` built-in type
- add `__CPROVER_freeable` built-in function
- allow calls to `__CPROVER_freeable_t` functions
  in frees clauses
- add `__CPROVER_is_freeable` built-in predicate
- add `__CPROVER_is_freed` built-in predicate

The predicates are not yet supported in the back-end.
@remi-delmas-3000 remi-delmas-3000 force-pushed the frees-clause-improvements branch from 133116d to eeb0728 Compare August 26, 2022 05:10
@feliperodri feliperodri added aws Bugs or features of importance to AWS CBMC users aws-high Code Contracts Function and loop contracts labels Aug 29, 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.

Left a few comments, but I will get another review after merging previous PR (or moving all frees clauses code over here and kill dependency).

Comment on lines +793 to +795
error().source_location = expr.source_location();
error() << id2string(id) + " is not allowed in preconditions." << eom;
throw 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess the previous PR contains this test case, right?

if(code.find(ID_C_spec_frees).is_not_nil())
{
typecheck_spec_frees(
static_cast<unary_exprt &>(code.add(ID_C_spec_frees)).op().operands());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no coverage here?

{
error().source_location = target.source_location();
error() << "function pointer calls not allowed in frees clauses" << eom;
throw 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no coverage?

Comment on lines +1029 to +1032
bool has_freeable_type =
(type.id() == ID_empty) &&
(type.get(ID_C_typedef) == CPROVER_PREFIX "freeable_t");
if(!has_freeable_type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool has_freeable_type =
(type.id() == ID_empty) &&
(type.get(ID_C_typedef) == CPROVER_PREFIX "freeable_t");
if(!has_freeable_type)
if((type.id() == ID_empty) &&
(type.get(ID_C_typedef) == CPROVER_PREFIX "freeable_t"))

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 like naming what is being checked to make the intent more explicit.

const std::function<void(exprt &)> typecheck_target = [&](exprt &target) {
typecheck_spec_frees_target(target);
};
typecheck_conditional_targets(targets, typecheck_target, "frees");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should "frees" and "assigns" be a macro?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what is wrong with std::function ?

{
$$=$1;
set($$, ID_C_spec_frees);
parser_stack($$).add_to_operands(exprt(ID_target_list));
Copy link
Collaborator

Choose a reason for hiding this comment

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

No coverage?

@remi-delmas-3000
Copy link
Collaborator Author

Superseded by #7091

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.

3 participants