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

Setup verification #486

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Setup verification #486

wants to merge 8 commits into from

Conversation

ziqiaozhou
Copy link

@ziqiaozhou ziqiaozhou commented Oct 16, 2024

  • Set up verification tool and add verification CI.
    Change the minimum rust version to 1.77 since the verification tool still needs some efforts to support 1.80. This will not change the default version in non-verification mode, since rust-toolchain.toml specifies 1.80.
  • cargo verify in kernel folder will run verification for functions that annotated with #[verus_verify]
  • Added new crates verify_proof and verify_external, which will import the minimum required spec and proofs to help SVSM to do efficient proofs and stub out external crates.
  • Add some proofs for address.rs as a basic example to show how verification works.
  • ./script/vfmt.sh will do enhanced formatting for code ending with .verus.rs

verify_proof/src/bits.verus.rs Outdated Show resolved Hide resolved
verify_proof/src/bits.verus.rs Show resolved Hide resolved
@ziqiaozhou ziqiaozhou marked this pull request as ready for review October 31, 2024 20:59
@ziqiaozhou
Copy link
Author

I have updated tool to support Rust-1.82, improved the verification document, and marked the verify.yml as manually triggered CI so that I can still run CI in my forked repo before sending my change here. I can remove it if you prefer to not keeping the manually-triggered CI.

Add verus tool setup scripts + CI + dependencies + import some proofs from verismo

Signed-off-by: Ziqiao Zhou <[email protected]>
Verify more in address mod & Split verismo to verify_proof and verify_external.

* Verify more in address mod.
* Rename spec and improve the definition of canonical address
* Most functions for VirtAddr are verified.
* Move broadcast proofs into address_inner.verus.rs
* Split verismo lib into verify_proof and verify_external.
* Improve verification.md
* Add verify_external to define spec for externals
* No dep verify by default
* Fix CI
* Revert changes in existing ci
* change name in verify.yml
* Use verus main branch
* Upgrade verus lib to latest main branch

Signed-off-by: ziqiao zhou <[email protected]>

---------

Signed-off-by: ziqiao zhou <[email protected]>
Signed-off-by: Ziqiao Zhou <[email protected]>
Simplify the seq_macros
Refine the specification for Option::map

Signed-off-by: Ziqiao Zhou <[email protected]>
* Upgrade verus to support v1.82.

* Cleanup build.rs
* Set verus lib and tool to the rev supporting v1.82

Signed-off-by: Ziqiao Zhou <[email protected]>

* Move vstd.verus.rs to verus vstd & mark memory-related as external

* memory related verification needs to add a memory permission and will
  do it later

Signed-off-by: Ziqiao Zhou <[email protected]>

* Revert unnecessary change after verus upgrade

* revert rust version
* revert some code due to version change

Signed-off-by: Ziqiao Zhou <[email protected]>

* fmt

---------

Signed-off-by: Ziqiao Zhou <[email protected]>
Add example & links about verus

Signed-off-by: Ziqiao Zhou <[email protected]>
Do not run CI for verification.

Signed-off-by: Ziqiao Zhou <[email protected]>
Copy link
Member

@joergroedel joergroedel left a comment

Choose a reason for hiding this comment

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

I followed the documentation and ran a successful verification, but the process generated a few warnings which I'd like to see fixed:

warning: verus-related attribute has no effect because Verus is already ignoring this item. You may need to mark it as `#[verifier::verify]`.
  --> verify_proof/src/lib.rs:16:1
   |
16 | global size_of usize == 8;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
347 | bit_shl_values! {u64, u64, 1u64, lemma_bit_u64_shl_values}
    | ---------------------------------------------------------- in this macro invocation
    |
    = note: this warning originates in the macro `bit_shl_values` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
351 | bit_shl_values! {usize, u64, 1usize, lemma_bit_usize_shl_values}
    | ---------------------------------------------------------------- in this macro invocation
    |
    = note: this warning originates in the macro `bit_shl_values` (in Nightly builds, run with -Z macro-backtrace for more info)

verification results:: 0 verified, 0 errors
warning: `verify_proof` (lib) generated 3 warnings
note: preparing crate for verification

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> /home/joro/src/svsm/verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Also, please look at the other review comments I left.

scripts/vpath.sh Outdated Show resolved Hide resolved
scripts/vinstall.sh Outdated Show resolved Hide resolved
scripts/vinstall.sh Outdated Show resolved Hide resolved
@joergroedel joergroedel added the in-review PR is under active review and not yet approved label Nov 12, 2024
* Upgrade verus to avoid warn in global size
* Install verusfmt via cargo install

Signed-off-by: Ziqiao Zhou <[email protected]>
* Remove vpath.sh since it is only used in verify.yml CI

Signed-off-by: Ziqiao Zhou <[email protected]>
@ziqiaozhou
Copy link
Author

I followed the documentation and ran a successful verification, but the process generated a few warnings which I'd like to see fixed:

warning: verus-related attribute has no effect because Verus is already ignoring this item. You may need to mark it as `#[verifier::verify]`.
  --> verify_proof/src/lib.rs:16:1
   |
16 | global size_of usize == 8;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
347 | bit_shl_values! {u64, u64, 1u64, lemma_bit_u64_shl_values}
    | ---------------------------------------------------------- in this macro invocation
    |
    = note: this warning originates in the macro `bit_shl_values` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
351 | bit_shl_values! {usize, u64, 1usize, lemma_bit_usize_shl_values}
    | ---------------------------------------------------------------- in this macro invocation
    |
    = note: this warning originates in the macro `bit_shl_values` (in Nightly builds, run with -Z macro-backtrace for more info)

verification results:: 0 verified, 0 errors
warning: `verify_proof` (lib) generated 3 warnings
note: preparing crate for verification

warning: broadcast functions should have explicit #[trigger] or #![trigger ...]
   --> /home/joro/src/svsm/verify_proof/src/bits.verus.rs:248:9
    |
248 |         pub broadcast proof fn $pname()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Also, please look at the other review comments I left.

The warnings are removed. You need to run cargo v prepare-verus before cargo verify since I upgraded the verus version to avoid one warning due to mismatched version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review PR is under active review and not yet approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants