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

Fix unsound contract equality fast check #1703

Merged
merged 6 commits into from
Oct 29, 2023
Merged

Conversation

yannham
Copy link
Member

@yannham yannham commented Oct 28, 2023

Closes #1700.

For performance reason, the contract equality checker performs a quick pointer check to see if the two terms to compare are physically equal. In this case, it used to return true directly, eschewing more costly recursive checks.

However, this is unsound, because the same term (physically) might be in two different environments, and thus evaluate in fine to two different values. This is typically the case with function application: when evaluating f 1 and f 2, both terms will physically point to the body of f, but in a different environment. Thus, we also need to ensure that environment are equals as well in this fast check.

This commit adds a fast ptr_eq check to the environment, and now checks that both terms and their respective environments are pairwise physically equals.

Passing by, this commits also fix unrelated clippy warning, after the udpate to clippy 1.73.

For performance reason, the contract equality checker performs a quick
pointer check to see if the two terms to compare are physically equal.
In this case, it used to return `true` directly, eschewing more costly
recursive checks.

However, this is unsound, because the same term (physically) might be in two
different environments, and have two different values. This is typically
the case with function application: when evaluating `f 1` and `f 2`,
both terms will physically point to the body of `f`, but in a different
environment. Thus, we also need to ensure that environment are equals as
well in this quick check.

This commit adds a fast `ptr_eq` check to the environment as well, and
now checks that both terms and their respective environments are
pairwise physically equals.

Passing by, this commits also fix unrelated clippy warning, after the
udpate to clippy 1.73.
@github-actions github-actions bot temporarily deployed to pull request October 28, 2023 15:06 Inactive
core/src/environment.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request October 29, 2023 12:02 Inactive
@yannham yannham added this pull request to the merge queue Oct 29, 2023
Merged via the queue into master with commit 11c8fab Oct 29, 2023
5 checks passed
@yannham yannham deleted the fix/unsound-contract-eq branch October 29, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contracts applied sequentially can shadow each other
2 participants