Skip to content
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

Updates aws_hash_table stubs for CBMC proofs #714

Closed

Conversation

tegansb
Copy link
Contributor

@tegansb tegansb commented Oct 14, 2020

Description of changes:
Updates the hash_iterator_string_string_generator and hash_find_string_string_generator functions with non-NULL assumptions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@feliperodri feliperodri added the cbmc Anything related to CBMC proofs. label Oct 15, 2020
@feliperodri feliperodri changed the title Updates to generators for CBMC hash table stubs Updates aws_hash_table stubs for CBMC proofs Oct 15, 2020
Comment on lines +23 to +25
__CPROVER_assume(new_iter->element.key != NULL);
new_iter->element.value = ensure_string_is_allocated_nondet_length();
__CPROVER_assume(new_iter->element.value != NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we guarantee these assumptions in this function as well? What happens with the actual function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there is no matching function corresponding to this stub. It's a generator to generate items in a hash table when it's iterated over (it's the HASH_ITER_ELEMENT_GENERATOR from the aws_hash_iter_override stubs). So whether it's guaranteed is I guess an assumption over the hash_table we're iterating over. But in general, some functions might require hash tables with this guarantee and some might not, so maybe we could have two functions? I'll also think of a way to add this as a precondition to the function itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create a GitHub issue to track whether we should map these as a precondition? @tegansb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made here #715. Could you add the cbmc label?

Copy link
Contributor

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

@feliperodri feliperodri requested a review from danielsn October 15, 2020 15:59
@feliperodri feliperodri marked this pull request as ready for review October 16, 2020 01:36
@feliperodri
Copy link
Contributor

This changes has been merged in PR #716.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cbmc Anything related to CBMC proofs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants