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

[move] Rewrite verifier metering #19036

Merged
merged 23 commits into from
Aug 22, 2024
Merged
Prev Previous commit
Next Next commit
locals_safety
  • Loading branch information
tnowacki committed Aug 21, 2024
commit 4157b635d17848dfa0969a082460980bfc4ebe35
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<'env, 'a> CodeUnitVerifier<'env, 'a> {
) -> PartialVMResult<()> {
StackUsageVerifier::verify(verifier_config, self.module, &self.function_context, meter)?;
type_safety::verify(self.module, &self.function_context, ability_cache, meter)?;
locals_safety::verify(self.module, &self.function_context, meter)?;
locals_safety::verify(self.module, &self.function_context, ability_cache, meter)?;
reference_safety::verify(
self.module,
&self.function_context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub(crate) enum LocalState {
}
use LocalState::*;

use crate::ability_cache::AbilityCache;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: move use up to the other use statements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remember what we were discussing about autoimports in the IDE... yeah this is why its hard


pub(crate) const STEP_BASE_COST: u128 = 2;
pub(crate) const RET_PER_LOCAL_COST: u128 = 5;
pub(crate) const JOIN_BASE_COST: u128 = 2;
Expand All @@ -41,8 +43,10 @@ pub(crate) struct AbstractState {
impl AbstractState {
/// create a new abstract state
pub fn new(
module: &CompiledModule,
_module: &CompiledModule,
function_context: &FunctionContext,
ability_ache: &mut AbilityCache,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: spelling

meter: &mut (impl Meter + ?Sized),
) -> PartialVMResult<Self> {
let num_args = function_context.parameters().len();
let num_locals = num_args + function_context.locals().len();
Expand All @@ -55,7 +59,14 @@ impl AbstractState {
.0
.iter()
.chain(function_context.locals().0.iter())
.map(|st| module.abilities(st, function_context.type_parameters()))
.map(|st| {
ability_ache.abilities(
Scope::Function,
meter,
function_context.type_parameters(),
st,
)
})
.collect::<PartialVMResult<Vec<_>>>()?;

Ok(Self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@

mod abstract_state;

use crate::locals_safety::abstract_state::{RET_PER_LOCAL_COST, STEP_BASE_COST};
use crate::{
ability_cache::AbilityCache,
locals_safety::abstract_state::{RET_PER_LOCAL_COST, STEP_BASE_COST},
};
use abstract_state::{AbstractState, LocalState};
use move_abstract_interpreter::absint::{AbstractInterpreter, FunctionContext, TransferFunctions};
use move_binary_format::{
Expand All @@ -22,9 +25,10 @@ use move_core_types::vm_status::StatusCode;
pub(crate) fn verify<'a>(
module: &CompiledModule,
function_context: &'a FunctionContext<'a>,
ability_cache: &mut AbilityCache,
meter: &mut (impl Meter + ?Sized),
) -> PartialVMResult<()> {
let initial_state = AbstractState::new(module, function_context)?;
let initial_state = AbstractState::new(module, function_context, ability_cache, meter)?;
LocalsSafetyAnalysis().analyze_function(initial_state, function_context, meter)
}

Expand Down