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

Iterate callstack API #4033

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
99 changes: 99 additions & 0 deletions core/iwasm/aot/aot_runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -4104,6 +4104,105 @@ aot_frame_update_profile_info(WASMExecEnv *exec_env, bool alloc_frame)
#endif /* end of WASM_ENABLE_AOT_STACK_FRAME != 0 */

#if WASM_ENABLE_DUMP_CALL_STACK != 0
void
aot_iterate_callstack_tiny_frame(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler,
void *user_data)
{
/*
* Note for devs: please refrain from such modifications inside of
* aot_iterate_callstack_tiny_frame
* - any allocations/freeing memory
* - dereferencing any pointers other than: exec_env, exec_env->module_inst,
* exec_env->module_inst->module, pointers between stack's bottom and
* top_boundary For more details check wasm_iterate_callstack in
* wasm_export.h
*/
uint8 *top_boundary = exec_env->wasm_stack.top_boundary;
uint8 *top = exec_env->wasm_stack.top;
uint8 *bottom = exec_env->wasm_stack.bottom;

bool is_top_index_in_range =
top_boundary >= top && top >= (bottom + sizeof(AOTTinyFrame));
if (!is_top_index_in_range) {
return;
}
bool is_top_aligned_with_bottom =
(unsigned long)(top - bottom) % sizeof(AOTTinyFrame) == 0;
if (!is_top_aligned_with_bottom) {
return;
}

AOTTinyFrame *frame = (AOTTinyFrame *)(top - sizeof(AOTTinyFrame));
WASMCApiFrame record_frame;
while (frame && (uint8_t *)frame >= bottom) {
record_frame.instance = exec_env->module_inst;
record_frame.module_offset = 0;
record_frame.func_index = frame->func_index;
record_frame.func_offset = frame->ip_offset;
if (!frame_handler(user_data, &record_frame)) {
break;
}
frame -= 1;
}
}

void
aot_iterate_callstack_standard_frame(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler,
void *user_data)
{
/*
* Note for devs: please refrain from such modifications inside of
* aot_iterate_callstack_standard_frame
* - any allocations/freeing memory
* - dereferencing any pointers other than: exec_env, exec_env->module_inst,
* exec_env->module_inst->module, pointers between stack's bottom and
* top_boundary For more details check wasm_iterate_callstack in
* wasm_export.h
*/
WASMModuleInstance *module_inst =
(WASMModuleInstance *)wasm_exec_env_get_module_inst(exec_env);
AOTFrame *cur_frame = (AOTFrame *)wasm_exec_env_get_cur_frame(exec_env);
uint8 *top_boundary = exec_env->wasm_stack.top_boundary;
uint8 *bottom = exec_env->wasm_stack.bottom;

WASMCApiFrame record_frame;
while (cur_frame && (uint8_t *)cur_frame >= bottom
&& (uint8_t *)cur_frame + sizeof(AOTFrame) <= top_boundary) {
g0djan marked this conversation as resolved.
Show resolved Hide resolved
record_frame.instance = module_inst;
record_frame.module_offset = 0;
record_frame.func_index = (uint32)cur_frame->func_index;
record_frame.func_offset = (uint32)cur_frame->ip_offset;
if (!frame_handler(user_data, &record_frame)) {
break;
}
cur_frame = cur_frame->prev_frame;
}
}

void
aot_iterate_callstack(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler, void *user_data)
{
/*
* Note for devs: please refrain from such modifications inside of
* aot_iterate_callstack
* - any allocations/freeing memory
* - dereferencing any pointers other than: exec_env, exec_env->module_inst,
* exec_env->module_inst->module, pointers between stack's bottom and
* top_boundary For more details check wasm_iterate_callstack in
* wasm_export.h
*/
if (!is_tiny_frame(exec_env)) {
aot_iterate_callstack_standard_frame(exec_env, frame_handler,
user_data);
}
else {
aot_iterate_callstack_tiny_frame(exec_env, frame_handler, user_data);
}
}

bool
aot_create_call_stack(struct WASMExecEnv *exec_env)
{
Expand Down
4 changes: 4 additions & 0 deletions core/iwasm/aot/aot_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,10 @@ aot_frame_update_profile_info(WASMExecEnv *exec_env, bool alloc_frame);
bool
aot_create_call_stack(struct WASMExecEnv *exec_env);

void
aot_iterate_callstack(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler, void *user_data);

/**
* @brief Dump wasm call stack or get the size
*
Expand Down
33 changes: 33 additions & 0 deletions core/iwasm/common/wasm_runtime_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "bh_common.h"
#include "bh_assert.h"
#include "bh_log.h"
#include "wasm_export.h"
#include "wasm_native.h"
#include "wasm_runtime_common.h"
#include "wasm_memory.h"
Expand Down Expand Up @@ -1740,6 +1741,38 @@ wasm_runtime_destroy_exec_env(WASMExecEnv *exec_env)
wasm_exec_env_destroy(exec_env);
}

void
wasm_iterate_callstack(const wasm_exec_env_t exec_env,
const wasm_frame_callback frame_callback,
void *user_data)
{
/*
* Note for devs: please refrain from such modifications inside of
* wasm_iterate_callstack to preserve async-signal-safety
* - any allocations/freeing memory
* - dereferencing any pointers other than: exec_env, exec_env->module_inst,
* exec_env->module_inst->module, pointers between stack's bottom and
* top_boundary For more details check wasm_iterate_callstack in
* wasm_export.h
*/
#if WASM_ENABLE_DUMP_CALL_STACK
WASMModuleInstance *module_inst =
(WASMModuleInstance *)get_module_inst(exec_env);

#if WASM_ENABLE_INTERP != 0
if (module_inst->module_type == Wasm_Module_Bytecode) {
wasm_interp_iterate_callstack(exec_env, frame_callback, user_data);
}
#endif

#if WASM_ENABLE_AOT != 0
if (module_inst->module_type == Wasm_Module_AoT) {
aot_iterate_callstack(exec_env, frame_callback, user_data);
}
#endif
#endif
}

bool
wasm_runtime_init_thread_env(void)
{
Expand Down
5 changes: 5 additions & 0 deletions core/iwasm/common/wasm_runtime_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,11 @@ wasm_runtime_create_exec_env(WASMModuleInstanceCommon *module_inst,
WASM_RUNTIME_API_EXTERN void
wasm_runtime_destroy_exec_env(WASMExecEnv *exec_env);

WASM_RUNTIME_API_EXTERN void
wasm_iterate_callstack(const wasm_exec_env_t exec_env,
const wasm_frame_callback frame_handler,
void *user_data);

/* See wasm_export.h for description */
WASM_RUNTIME_API_EXTERN WASMModuleInstanceCommon *
wasm_runtime_get_module_inst(WASMExecEnv *exec_env);
Expand Down
35 changes: 35 additions & 0 deletions core/iwasm/include/wasm_export.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ typedef WASMFunctionInstanceCommon *wasm_function_inst_t;
struct WASMMemoryInstance;
typedef struct WASMMemoryInstance *wasm_memory_inst_t;

struct wasm_frame_t;
typedef struct wasm_frame_t *wasm_frame_ptr_t;

/* WASM section */
typedef struct wasm_section_t {
struct wasm_section_t *next;
Expand Down Expand Up @@ -864,6 +867,38 @@ wasm_runtime_create_exec_env(wasm_module_inst_t module_inst,
WASM_RUNTIME_API_EXTERN void
wasm_runtime_destroy_exec_env(wasm_exec_env_t exec_env);

/**
* Callback to be called on wasm_frame_t*.
* It accepts void* as a context that can be used for closures.
* It returns bool so the iterating can stop when the callback returns false.
* E.g. callback that returns false after processing 100 frames
*/
typedef bool (*wasm_frame_callback)(void *, wasm_frame_ptr_t);

/**
* @brief Iterate over callstack frames and execute callback on it.
*
* Caution: This is not a thread-safe function. Ensure the exec_env
* is suspended before calling it from another thread.
*
* Usage: In the callback to read frames fields use APIs
* for wasm_frame_t from wasm_c_api.h
*
* Note: The function is async-signal-safe if called with verified arguments.
* Meaning it's safe to call it from a signal handler even on a signal
* interruption from another thread if next variables hold valid pointers
* - exec_env
* - exec_env->module_inst
* - exec_env->module_inst->module
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, making this complex functionality async-signal-safe is too much maintenance burden, especially when wamr itself doesn't rely on the property at all.

given that you need to suspend the target thread anyway, why don't you call this from another thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

async-signal-safe is too much maintenance burden

Yeah, we understand that and I kept it simple as much as possible. Basically non async-signal-safe implementation would be different only in a few checks removed and there wouldn't be comments that I added in the code.

Particularly this comment about validity of pointers is a theoretical problem atm. We don't know any platform yet where updating pointer variable might be interrupted by a signal, possibly after launch we will see that it never happens.

given that you need to suspend the target thread anyway, why don't you call this from another thread?

Do you mean using wasm_cluster_suspend_thread?
I tried that and there're 2 problems for us:

  • Now there’s no awaiting till thread actually gets suspended
  • Suspension happens only after certain checks so we're not getting stacktraces that we need. E.g. if there's sleep somewhere, there won't be sleep in stacktrace reported, it'd report calls after sleep has finished. If there's a deadlock stacktrace won't be reported at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*
* @param exec_env the execution environment that containes frames
* @param callback the callback function provided by the user
* @param user_data context for callback provided by the user
*/
WASM_RUNTIME_API_EXTERN void
wasm_iterate_callstack(const wasm_exec_env_t exec_env,
const wasm_frame_callback callback, void *user_data);

/**
* Get the singleton execution environment for the instance.
*
Expand Down
41 changes: 41 additions & 0 deletions core/iwasm/interpreter/wasm_runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "wasm_runtime.h"
#include "wasm.h"
#include "wasm_exec_env.h"
#include "wasm_loader.h"
#include "wasm_interp.h"
#include "bh_common.h"
Expand Down Expand Up @@ -4196,6 +4197,46 @@ wasm_get_module_inst_mem_consumption(const WASMModuleInstance *module_inst,
|| (WASM_ENABLE_MEMORY_TRACING != 0) */

#if WASM_ENABLE_DUMP_CALL_STACK != 0
void
wasm_interp_iterate_callstack(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler,
void *user_data)
{
/*
* Note for devs: please refrain from such modifications inside of
* wasm_interp_iterate_callstack
* - any allocations/freeing memory
* - dereferencing any pointers other than: exec_env, exec_env->module_inst,
* exec_env->module_inst->module, pointers between stack's bottom and
* top_boundary For more details check wasm_iterate_callstack in
* wasm_export.h
*/
WASMModuleInstance *module_inst =
(WASMModuleInstance *)wasm_exec_env_get_module_inst(exec_env);
WASMInterpFrame *cur_frame = wasm_exec_env_get_cur_frame(exec_env);
uint8 *top_boundary = exec_env->wasm_stack.top_boundary;
uint8 *bottom = exec_env->wasm_stack.bottom;

WASMCApiFrame record_frame;
while (cur_frame && (uint8_t *)cur_frame >= bottom
&& (uint8_t *)cur_frame + sizeof(WASMInterpFrame) <= top_boundary) {
if (!cur_frame->function) {
cur_frame = cur_frame->prev_frame;
continue;
}
record_frame.instance = module_inst;
record_frame.module_offset = 0;
// It's safe to dereference module_inst->e because "e" is asigned only
// once in wasm_instantiate
record_frame.func_index =
(uint32)(cur_frame->function - module_inst->e->functions);
if (!frame_handler(user_data, &record_frame)) {
break;
}
cur_frame = cur_frame->prev_frame;
}
}

bool
wasm_interp_create_call_stack(struct WASMExecEnv *exec_env)
{
Expand Down
6 changes: 6 additions & 0 deletions core/iwasm/interpreter/wasm_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,12 @@ wasm_get_table_inst(const WASMModuleInstance *module_inst, uint32 tbl_idx)
}

#if WASM_ENABLE_DUMP_CALL_STACK != 0

void
wasm_interp_iterate_callstack(WASMExecEnv *exec_env,
const wasm_frame_callback frame_handler,
void *user_data);

bool
wasm_interp_create_call_stack(struct WASMExecEnv *exec_env);

Expand Down
Loading