get rid of confusing `ACTIVATION_FRAME_COST` constant

[StackOverflowExcept1on]

Hi, I'm working on adding a custom stack limiter for our protocol. It will provide API like that: (e.g. you can inject something instead of unreachable instruction)

wasm_instrument::inject_custom_stack_limiter(module, 20_000, |signature| vec![
    Instruction::Foo,
    Instruction::Bar,
    ...,
])

And I think I understood what the real problem was at #1

wasm-instrument/src/stack_limiter/mod.rs

Lines 10 to 19 in 6307588

	/// Macro to generate preamble and postamble.
	macro_rules! instrument_call {
	($callee_idx: expr, $callee_stack_cost: expr, $stack_height_global_idx: expr, $stack_limit: expr) => {{
	use $crate::parity_wasm::elements::Instruction::*;
	[
	// stack_height += stack_cost(F)
	GetGlobal($stack_height_global_idx),
	I32Const($callee_stack_cost),
	I32Add,
	SetGlobal($stack_height_global_idx),

So we have an instrumented_call! with a maximum stack height of 2, the same as the ACTIVATION_FRAME_COST constant.

// stack_height += stack_cost(F) 
GetGlobal($stack_height_global_idx), // push #1
I32Const($callee_stack_cost), // push #2
// max_height = 2

Backing to problem #1: the function with index 0 has no arguments and no return value, so its max_height=0

(module
  (type (;0;) (func))
  (func (;0;) (type 0)
    call 0)
  (func (;1;) (type 0)
    call 0)
  (export "main" (func 1)))

We also have a main export that will call function 0 and it will not be instrumented due to the condition:

wasm-instrument/src/stack_limiter/thunk.rs

Lines 44 to 48 in 6307588

	let callee_stack_cost = ctx.stack_cost(func_idx).ok_or("function index isn't found")?;
	// Don't generate a thunk if stack_cost of a callee is zero.
	if callee_stack_cost != 0 {
	replacement_map.insert(

What is the best way to solve this problem? When the Call(idx) opcode is found, we need to check that idx is not an import and simply do:

let import_count = module.import_count(elements::ImportCountType::Function);
let mut max_height: u32 = 0;
let mut overhead_of_instrumented_call = 0;

...

match opcode {
    Call(idx) => {
        if idx >= import_count {
            overhead_of_instrumented_call = 2;
        }
    }
}

Ok(max_height + overhead_of_instrumented_call)

or we can start passing instrument_call! instructions instead of Call(idx) to match opcode

[athei]

But even with non-imports the problem can arise if they have a zero cost without the constant?

Also, you still need the constant if the callee is an import?

[StackOverflowExcept1on]

But even with non-imports the problem can arise if they have a zero cost without the constant?

It should not because we need to charge for overhead of the instrumented call instructions. For example, instead of calculating the stack height for Call(0) we will replace the Call(0) with info[0] (sequence that we got from instrumented call! macro)

In this case the Call(idx), which does not change the stack (0 pops and 0 pushes), will still cost 2.

#![feature(if_let_guard)]

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Instruction {
    Foo,
    Bar,
    Baz,
    Call(usize),
}

fn process(opcode: &Instruction) {
    println!("{opcode:?}");
}

fn main() {
    use Instruction::*;

    let info = vec![vec![Foo, Bar, Baz]];
    let xs = vec![Foo, Bar, Baz, Call(0), Foo];

    for opcode in xs.iter() {
        match opcode {
            Call(i) if let Some(slice) = info.get(*i) => {
                for opcode in slice {
                    process(opcode);
                }
            }
            _ => process(opcode),
        }
    }
}

[StackOverflowExcept1on]

Also, you still need the constant if the callee is an import?

Why do I need this constant if instrument_function will not process imports (since they have zero cost)?

wasm-instrument/src/stack_limiter/mod.rs

Lines 164 to 165 in 0c739d9

	// We can't calculate stack_cost of the import functions.
	Ok(0)

wasm-instrument/src/stack_limiter/mod.rs

Lines 260 to 266 in 0c739d9

	ctx.stack_cost(*callee).and_then(|cost| {
	if cost > 0 {
	Some(InstrumentCall { callee: *callee, offset, cost })
	} else {
	None
	}
	})

For example, we have module

(module
  (import "env" "nop" (func $nop))
  (func $f1 (export "f1")
    call $nop
  )
)

After instrumentation:

(module
  (type (;0;) (func))
  (import "env" "nop" (func $nop (;0;) (type 0))) ;; cost of import = 0
  (func $f1 (;1;) (type 0) ;; cost of func = 0
    call $nop ;; it will not be processed by the `instrument_function`
  )
  (global (;0;) (mut i32) i32.const 0)
  (export "f1" (func $f1)) ;; thunk function is not created here, bcz we can safely call to original
)

[athei]

What I am saying is that this is not only about imports is it? You can just call an zero cost function recursively without a constant cost for the activation.

[StackOverflowExcept1on]

I think you mean that example with empty functions, right? If you look at the PR, the tests pass and I'll explain why.

So we have function 0, which actually costs 0 (no params, nor results)

(func (;0;)
  call 0
)

but after the injection stack limiter it will look like this:

(func (;0;) (type 0)
  global.get 0
  i32.const 2
  i32.add
  global.set 0
  global.get 0
  i32.const 1024
  i32.gt_u
  if ;; label = @1
    unreachable
  end
  call 0
  global.get 0
  i32.const 2
  i32.sub
  global.set 0
)

Instead of counting the cost of Call(idx=0) we will count cost of instrument_call!(idx, 0, 0, 0). Therefore this function will have a maximum stack height of 2, not 0.