Skip to content

Introduce ThreadContext instead of execution depth #726

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

Closed
wants to merge 8 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 19 additions & 33 deletions bindings/rust/src/lib.rs
Original file line number Diff line number Diff line change
@@ -313,18 +313,11 @@ impl Instance {
///
/// An invalid index, invalid inputs, or invalid depth can cause undefined behaviour.
///
/// The `depth` argument sets the initial call depth. Should be set to `0`.
///
/// # Safety
/// This function expects a valid `func_idx`, appropriate number of `args`, and valid `depth`.
pub unsafe fn unsafe_execute(
&mut self,
func_idx: u32,
args: &[Value],
depth: i32,
) -> ExecutionResult {
/// This function expects a valid `func_idx` and appropriate number of `args`.
pub unsafe fn unsafe_execute(&mut self, func_idx: u32, args: &[Value]) -> ExecutionResult {
ExecutionResult {
0: sys::fizzy_execute(self.0.as_ptr(), func_idx, args.as_ptr(), depth),
0: sys::fizzy_execute(self.0.as_ptr(), func_idx, args.as_ptr()),
}
}

@@ -338,15 +331,8 @@ impl Instance {
///
/// An error is returned if the function can not be found, inappropriate number of arguments are passed,
/// or the supplied types are mismatching.
///
/// The `depth` argument sets the initial call depth. Should be set to `0`.
// TODO: consider different approach: Result<TypedValue, Error> where Error::Trap is used for traps
pub fn execute(
&mut self,
name: &str,
args: &[TypedValue],
depth: i32,
) -> Result<TypedExecutionResult, ()> {
pub fn execute(&mut self, name: &str, args: &[TypedValue]) -> Result<TypedExecutionResult, ()> {
let func_idx = self.find_exported_function_index(&name);
if func_idx.is_none() {
return Err(());
@@ -369,7 +355,7 @@ impl Instance {
// Translate to untyped raw values.
let args: Vec<Value> = args.iter().map(|v| v.into()).collect();

let ret = unsafe { self.unsafe_execute(func_idx, &args, depth) };
let ret = unsafe { self.unsafe_execute(func_idx, &args) };
Ok(TypedExecutionResult {
result: ret.0,
value_type: func_type.output,
@@ -651,29 +637,29 @@ mod tests {
assert!(instance.is_ok());
let mut instance = instance.unwrap();

let result = unsafe { instance.unsafe_execute(0, &[], 0) };
let result = unsafe { instance.unsafe_execute(0, &[]) };
assert!(!result.trapped());
assert!(!result.value().is_some());

let result = unsafe { instance.unsafe_execute(1, &[], 0) };
let result = unsafe { instance.unsafe_execute(1, &[]) };
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_i32(), 42);

// Explicit type specification
let result =
unsafe { instance.unsafe_execute(2, &[(42 as i32).into(), (2 as i32).into()], 0) };
unsafe { instance.unsafe_execute(2, &[(42 as i32).into(), (2 as i32).into()]) };
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_i32(), 21);

// Implicit i64 types (even though the code expects i32)
let result = unsafe { instance.unsafe_execute(2, &[42.into(), 2.into()], 0) };
let result = unsafe { instance.unsafe_execute(2, &[42.into(), 2.into()]) };
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_i32(), 21);

let result = unsafe { instance.unsafe_execute(3, &[], 0) };
let result = unsafe { instance.unsafe_execute(3, &[]) };
assert!(result.trapped());
assert!(!result.value().is_some());
}
@@ -701,31 +687,31 @@ mod tests {
let mut instance = instance.unwrap();

// Successful execution.
let result = instance.execute("foo", &[], 0);
let result = instance.execute("foo", &[]);
assert!(result.is_ok());
let result = result.unwrap();
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_u32().unwrap(), 42);

// Successful execution with arguments.
let result = instance.execute("bar", &[TypedValue::U32(42), TypedValue::U64(24)], 0);
let result = instance.execute("bar", &[TypedValue::U32(42), TypedValue::U64(24)]);
assert!(result.is_ok());
let result = result.unwrap();
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_u32().unwrap(), 66);

// Successful execution with 32-bit float argument.
let result = instance.execute("pi32", &[TypedValue::F32(0.5)], 0);
let result = instance.execute("pi32", &[TypedValue::F32(0.5)]);
assert!(result.is_ok());
let result = result.unwrap();
assert!(!result.trapped());
assert!(result.value().is_some());
assert_eq!(result.value().unwrap().as_f32().unwrap(), 0.15923566);

// Successful execution with 64-bit float argument.
let result = instance.execute("pi64", &[TypedValue::F64(0.5)], 0);
let result = instance.execute("pi64", &[TypedValue::F64(0.5)]);
assert!(result.is_ok());
let result = result.unwrap();
assert!(!result.trapped());
@@ -736,23 +722,23 @@ mod tests {
);

// Non-function export.
let result = instance.execute("g1", &[], 0);
let result = instance.execute("g1", &[]);
assert!(result.is_err());

// Export not found.
let result = instance.execute("baz", &[], 0);
let result = instance.execute("baz", &[]);
assert!(result.is_err());

// Passing more arguments than required.
let result = instance.execute("foo", &[TypedValue::U32(42)], 0);
let result = instance.execute("foo", &[TypedValue::U32(42)]);
assert!(result.is_err());

// Passing less arguments than required.
let result = instance.execute("bar", &[], 0);
let result = instance.execute("bar", &[]);
assert!(result.is_err());

// Passing mismatched types.
let result = instance.execute("bar", &[TypedValue::F32(1.0), TypedValue::F64(2.0)], 0);
let result = instance.execute("bar", &[TypedValue::F32(1.0), TypedValue::F64(2.0)]);
assert!(result.is_err());
}
}
9 changes: 5 additions & 4 deletions include/fizzy/fizzy.h
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@ typedef struct FizzyModule FizzyModule;
/// The opaque data type representing an instance (instantiated module).
typedef struct FizzyInstance FizzyInstance;

typedef struct FizzyThreadContext FizzyThreadContext;
Copy link
Member

Choose a reason for hiding this comment

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

Should have a comment, see FizzyInstance above.


/// The data type representing numeric values.
typedef union FizzyValue
{
@@ -52,10 +54,10 @@ typedef struct FizzyExecutionResult
/// @param context Opaque pointer to execution context.
/// @param instance Pointer to module instance.
/// @param args Pointer to the argument array. Can be NULL iff function has no inputs.
/// @param depth Call stack depth.
/// @param ctx Pointer to thread context. Cannot be NULL.
/// @return Result of execution.
typedef FizzyExecutionResult (*FizzyExternalFn)(
void* context, FizzyInstance* instance, const FizzyValue* args, int depth);
void* context, FizzyInstance* instance, const FizzyValue* args, FizzyThreadContext* ctx);

/// Value type.
typedef uint8_t FizzyValueType;
@@ -518,15 +520,14 @@ bool fizzy_find_exported_global(
///
/// @param instance Pointer to module instance. Cannot be NULL.
/// @param args Pointer to the argument array. Can be NULL if function has 0 inputs.
/// @param depth Call stack depth.
/// @return Result of execution.
///
/// @note
/// No validation is done on the number of arguments passed in @p args, nor on their types.
/// When number of passed arguments or their types are different from the ones defined by the
/// function type, behaviour is undefined.
FizzyExecutionResult fizzy_execute(
FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args, int depth);
FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args);

#ifdef __cplusplus
}
26 changes: 18 additions & 8 deletions lib/fizzy/capi.cpp
Original file line number Diff line number Diff line change
@@ -95,6 +95,16 @@ inline fizzy::Instance* unwrap(FizzyInstance* instance) noexcept
return reinterpret_cast<fizzy::Instance*>(instance);
}

inline FizzyThreadContext* wrap(fizzy::ThreadContext& ctx) noexcept
{
return reinterpret_cast<FizzyThreadContext*>(&ctx);
}

inline fizzy::ThreadContext& unwrap(FizzyThreadContext* ctx) noexcept
{
return *reinterpret_cast<fizzy::ThreadContext*>(ctx);
}

inline FizzyExecutionResult wrap(const fizzy::ExecutionResult& result) noexcept
{
return {result.trapped, result.has_value, wrap(result.value)};
@@ -113,19 +123,19 @@ inline fizzy::ExecutionResult unwrap(const FizzyExecutionResult& result) noexcep
inline auto unwrap(FizzyExternalFn func, void* context) noexcept
{
return [func, context](fizzy::Instance& instance, const fizzy::Value* args,
int depth) noexcept -> fizzy::ExecutionResult {
const auto result = func(context, wrap(&instance), wrap(args), depth);
fizzy::ThreadContext& ctx) noexcept -> fizzy::ExecutionResult {
const auto result = func(context, wrap(&instance), wrap(args), wrap(ctx));
return unwrap(result);
};
}

inline FizzyExternalFunction wrap(fizzy::ExternalFunction external_func)
{
static constexpr FizzyExternalFn c_function = [](void* context, FizzyInstance* instance,
const FizzyValue* args,
int depth) -> FizzyExecutionResult {
static constexpr FizzyExternalFn c_function =
[](void* context, FizzyInstance* instance, const FizzyValue* args,
FizzyThreadContext* ctx) -> FizzyExecutionResult {
auto* func = static_cast<fizzy::ExternalFunction*>(context);
return wrap((func->function)(*unwrap(instance), unwrap(args), depth));
return wrap((func->function)(*unwrap(instance), unwrap(args), unwrap(ctx)));
};

auto context = std::make_unique<fizzy::ExternalFunction>(std::move(external_func));
@@ -582,9 +592,9 @@ size_t fizzy_get_instance_memory_size(FizzyInstance* instance)
}

FizzyExecutionResult fizzy_execute(
FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args, int depth)
FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args)
{
const auto result = fizzy::execute(*unwrap(instance), func_idx, unwrap(args), depth);
const auto result = fizzy::execute(*unwrap(instance), func_idx, unwrap(args));
return wrap(result);
}
}
17 changes: 9 additions & 8 deletions lib/fizzy/execute.cpp
Original file line number Diff line number Diff line change
@@ -475,13 +475,14 @@ void branch(const Code& code, OperandStack& stack, const uint8_t*& pc, uint32_t
}

inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance,
OperandStack& stack, int depth)
OperandStack& stack, ThreadContext& ctx)
{
const auto num_args = func_type.inputs.size();
assert(stack.size() >= num_args);
const auto call_args = stack.rend() - num_args;

const auto ret = execute(instance, func_idx, call_args, depth + 1);
const auto guard = ctx.bump_call_depth();
const auto ret = execute(instance, func_idx, call_args, ctx);
// Bubble up traps
if (ret.trapped)
return false;
@@ -501,17 +502,17 @@ inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instan

} // namespace

ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth)
ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, ThreadContext& ctx)
{
assert(depth >= 0);
if (depth >= CallStackLimit)
assert(ctx.depth >= 0);
if (ctx.depth >= CallStackLimit)
return Trap;

const auto& func_type = instance.module->get_function_type(func_idx);

assert(instance.module->imported_function_types.size() == instance.imported_functions.size());
if (func_idx < instance.imported_functions.size())
return instance.imported_functions[func_idx].function(instance, args, depth);
return instance.imported_functions[func_idx].function(instance, args, ctx);

const auto& code = instance.module->get_code(func_idx);
auto* const memory = instance.memory.get();
@@ -594,7 +595,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
const auto called_func_idx = read<uint32_t>(pc);
const auto& called_func_type = instance.module->get_function_type(called_func_idx);

if (!invoke_function(called_func_type, called_func_idx, instance, stack, depth))
if (!invoke_function(called_func_type, called_func_idx, instance, stack, ctx))
goto trap;
break;
}
@@ -621,7 +622,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
goto trap;

if (!invoke_function(
actual_type, called_func.func_idx, *called_func.instance, stack, depth))
actual_type, called_func.func_idx, *called_func.instance, stack, ctx))
goto trap;
break;
}
34 changes: 32 additions & 2 deletions lib/fizzy/execute.hpp
Original file line number Diff line number Diff line change
@@ -41,6 +41,28 @@ constexpr ExecutionResult Void{true};
/// Shortcut for execution that resulted in a trap.
constexpr ExecutionResult Trap{false};


class ThreadContext
{
class [[nodiscard]] Guard
{
ThreadContext& m_thread_context;

public:
explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} {}
~Guard() noexcept { --m_thread_context.depth; }
};

public:
int depth = 0;

Guard bump_call_depth() noexcept
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, this returns private class, I wouldn't think it's possible...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe call it increment_call_depth, bump sounds a bit informal it seems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just an idea: it could be execute member function of ThreadContext bumping depth and redirecting to normal execute

    ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args) noexcept;

then Guard can be completely hidden from user.

But maybe confusingly too many execute variants then.

{
++depth;
return Guard{*this};
}
};

/// Execute a function from an instance.
///
/// @param instance The instance.
@@ -49,7 +71,15 @@ constexpr ExecutionResult Trap{false};
/// @param args The pointer to the arguments. The number of items and their types must match
/// the expected number of input parameters of the function, otherwise undefined
/// behaviour (including crash) happens.
/// @param depth The call depth (indexing starts at 0). Can be left at the default setting.
/// @param ctx The thread context.
/// @return The result of the execution.
ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth = 0);
ExecutionResult execute(
Instance& instance, FuncIdx func_idx, const Value* args, ThreadContext& ctx);

/// Execute a function from an instance starting at default depth of 0.
inline ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args)
Copy link
Member

Choose a reason for hiding this comment

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

Is this version used heavily in the tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.

{
ThreadContext ctx;
return execute(instance, func_idx, args, ctx);
}
} // namespace fizzy
4 changes: 2 additions & 2 deletions lib/fizzy/instantiate.cpp
Original file line number Diff line number Diff line change
@@ -477,8 +477,8 @@ std::optional<ExternalFunction> find_exported_function(Instance& instance, std::
return std::nullopt;

const auto idx = *opt_index;
auto func = [idx, &instance](fizzy::Instance&, const Value* args, int depth) {
return execute(instance, idx, args, depth);
auto func = [idx, &instance](fizzy::Instance&, const Value* args, ThreadContext& ctx) {
return execute(instance, idx, args, ctx);
};

return ExternalFunction{std::move(func), instance.module->get_function_type(idx)};
3 changes: 2 additions & 1 deletion lib/fizzy/instantiate.hpp
Original file line number Diff line number Diff line change
@@ -20,9 +20,10 @@ namespace fizzy
{
struct ExecutionResult;
struct Instance;
class ThreadContext;

/// Function representing WebAssembly or host function execution.
using execute_function = std::function<ExecutionResult(Instance&, const Value*, int depth)>;
using execute_function = std::function<ExecutionResult(Instance&, const Value*, ThreadContext&)>;

/// Function with associated input/output types,
/// used to represent both imported and exported functions.
Loading