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

Interpreter handles opaque inconsistently #944

Open
parno opened this issue Dec 15, 2023 · 4 comments
Open

Interpreter handles opaque inconsistently #944

parno opened this issue Dec 15, 2023 · 4 comments

Comments

@parno
Copy link
Collaborator

parno commented Dec 15, 2023

This program

use vstd::prelude::*;

verus!{

fn main() { }

pub proof fn bar() {
    assert(vstd::std_specs::bits::u64_leading_zeros(0) == 64) by (compute_only);
}

}

uses u64_leading_zeros, which is defined as public and open:

pub open spec fn u64_leading_zeros(i: u64) -> int

Oddly, when the interpreter tries to run, when it reaches the point of trying to apply u64_leading_zeros, it does a lookup for the function's body and the lookup fails, specifically we get None here:

match ctx.fun_ssts.get(fun) {

where fun_ssts is passed into the interpreter from the state object in the ast_to_sst conversion.

If I inline the definition in the same file, there's no trouble finding and using the definition.

use vstd::prelude::*;

verus!{

fn main() { }

#[verifier::opaque]
pub open spec fn u64_leading_zeros(i: u64) -> int
    decreases i
{
    if i == 0 {
        64
    } else {
        u64_leading_zeros(i / 2) - 1
    }
}

pub proof fn bar() {
    assert(u64_leading_zeros(0) == 64) by (compute_only);
}

}
}

Is this expected behavior? Or is something going wrong in the collection of fun_ssts?

@tjhance
Copy link
Collaborator

tjhance commented Jan 29, 2024

A result of this is that this passes:

pub proof fn other_function() {
    reveal(vstd::std_specs::bits::u64_leading_zeros);
}

pub proof fn bar() {
    assert(vstd::std_specs::bits::u64_leading_zeros(0) == 64) by (compute_only);
}

whereas this does not:

pub proof fn bar() {
    assert(vstd::std_specs::bits::u64_leading_zeros(0) == 64) by (compute_only);
}

This is obviously unexpected behavior.

Because of the way "pruning" works, and because u64_leading_zeros is opaque, we generate SST for it iff we find that it is revealed anywhere in the module. Then as Bryan points out, the interpreter determines whether or not to unfold the definition of u64_leading_zeros based on whether or not the SST exists.

To make a fix, we need to decide: should assert-by-compute unfold opaque functions by default, or should it require
them to be revealed?

@tjhance tjhance changed the title Access to spec function bodies defined in vstd Interpreter handles opaque inconsistently Jan 29, 2024
@parno
Copy link
Collaborator Author

parno commented Jan 29, 2024

I would tend to think that opaque is designed to control SMT-level processing, rather than proofs-by-compute. Indeed, it seems like we often use opaque for recursive functions to keep the SMT solver from getting too excited, but then those functions seem to often be the ones we can to compute on for specific concrete values. Hence, I'm tempted to say that the interpreter should unfold opaque definitions. OTOH, maybe there's value in consistently enforcing opaqueness, so that user's aren't confused by the different possible meanings. If we go that way, we should certainly do it consistently, not based on whether there's an ambient reveal elsewhere in the module.

@parno
Copy link
Collaborator Author

parno commented Jan 30, 2024

Based on discussions in today's Verus meeting, the consensus was that the interpreter should default to opening opaque definitions and that eventually we could add support for restricting this, via syntax along the lines of assert (...) by (compute, opaque=foo).

@parno
Copy link
Collaborator Author

parno commented Jun 28, 2024

Based on a discussion with @Chris-Hawblitzel, the current plan is to add assert-by-compute to the list of assertions that cause us to spinoff a separate query, and in that context we can tell the pruning phase to not remove opaque functions (or their transitive dependencies).

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

No branches or pull requests

2 participants