From 22c30e27e59035f165bfa0540022eeca113fcd59 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 18 Mar 2021 17:04:07 -0400 Subject: [PATCH] Refactoring: bytecode interpreter ABI Refactor the ABI of the bytecode interpreter, which is used by the probe provider callbacks: - Return a "int" rather than uint64_t, - The return values are now: LTTNG_UST_BYTECODE_INTERPRETER_ERROR = -1, LTTNG_UST_BYTECODE_INTERPRETER_OK = 0, - Introduce a bytecode "context". This context is specific for each bytecode "class" (filter or capture). The filter class context has a "result" (accept/reject) output, whereas the capture class has an "output". - The bytecode context is extensible with the struct_size scheme. - Merge filter and capture interpreters into a single callback. That callback uses a new "bytecode type" field within the bytecode private data to figure out the specific bytecode class (filter or capture). Signed-off-by: Mathieu Desnoyers Change-Id: Id528e141d0c213e9bb41f68d6d7e632104daa1cf --- include/lttng/ust-events.h | 40 +++++++---- include/lttng/ust-tracepoint-event.h | 12 ++-- liblttng-ust/event-notifier-notification.c | 4 +- liblttng-ust/lttng-bytecode-interpreter.c | 80 ++++++++++------------ liblttng-ust/lttng-bytecode.c | 41 ++--------- liblttng-ust/lttng-bytecode.h | 25 ++----- liblttng-ust/lttng-events.c | 6 +- liblttng-ust/lttng-ust-comm.c | 6 +- liblttng-ust/ust-events-internal.h | 9 +-- 9 files changed, 100 insertions(+), 123 deletions(-) diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index 367d2d1a..58a0baf4 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -297,17 +297,38 @@ struct lttng_ust_probe_desc { /* Data structures used by the tracer. */ /* - * Bytecode interpreter return value masks. + * Bytecode interpreter return value. */ enum lttng_ust_bytecode_interpreter_ret { - LTTNG_UST_BYTECODE_INTERPRETER_DISCARD = 0, - LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG = (1ULL << 0), - /* Other bits are kept for future use. */ + LTTNG_UST_BYTECODE_INTERPRETER_ERROR = -1, + LTTNG_UST_BYTECODE_INTERPRETER_OK = 0, }; struct lttng_interpreter_output; struct lttng_ust_bytecode_runtime_private; +enum lttng_ust_bytecode_filter_result { + LTTNG_UST_BYTECODE_FILTER_ACCEPT = 0, + LTTNG_UST_BYTECODE_FILTER_REJECT = 1, +}; + +/* + * IMPORTANT: this structure is part of the ABI between the probe and + * UST. Fields need to be only added at the end, never reordered, never + * removed. + * + * The field @struct_size should be used to determine the size of the + * structure. It should be queried before using additional fields added + * at the end of the structure. + */ +struct lttng_ust_bytecode_filter_ctx { + uint32_t struct_size; /* Size of this structure. */ + + enum lttng_ust_bytecode_filter_result result; + + /* End of base ABI. Fields below should be used after checking struct_size. */ +}; + /* * IMPORTANT: this structure is part of the ABI between the probe and * UST. Fields need to be only added at the end, never reordered, never @@ -321,14 +342,9 @@ struct lttng_ust_bytecode_runtime { uint32_t struct_size; /* Size of this structure. */ struct lttng_ust_bytecode_runtime_private *priv; - /* Associated bytecode */ - union { - uint64_t (*filter)(void *interpreter_data, - const char *interpreter_stack_data); - uint64_t (*capture)(void *interpreter_data, - const char *interpreter_stack_data, - struct lttng_interpreter_output *interpreter_output); - } interpreter_funcs; + int (*interpreter_func)(struct lttng_ust_bytecode_runtime *bytecode_runtime, + const char *interpreter_stack_data, + void *ctx); struct cds_list_head node; /* list of bytecode runtime in event */ /* End of base ABI. Fields below should be used after checking struct_size. */ diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h index af51f9bc..4fc1ea91 100644 --- a/include/lttng/ust-tracepoint-event.h +++ b/include/lttng/ust-tracepoint-event.h @@ -824,14 +824,18 @@ void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args)) \ if (caa_unlikely(!cds_list_empty(&__event->filter_bytecode_runtime_head))) { \ struct lttng_ust_bytecode_runtime *__filter_bc_runtime; \ int __filter_record = __event->has_enablers_without_bytecode; \ + struct lttng_ust_bytecode_filter_ctx filter_ctx; \ \ + filter_ctx.struct_size = sizeof(struct lttng_ust_bytecode_filter_ctx); \ __event_prepare_interpreter_stack__##_provider##___##_name(__stackvar.__interpreter_stack_data, \ _TP_ARGS_DATA_VAR(_args)); \ tp_list_for_each_entry_rcu(__filter_bc_runtime, &__event->filter_bytecode_runtime_head, node) { \ - if (caa_unlikely(__filter_bc_runtime->interpreter_funcs.filter(__filter_bc_runtime, \ - __stackvar.__interpreter_stack_data) & LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG)) { \ - __filter_record = 1; \ - break; \ + if (caa_likely(__filter_bc_runtime->interpreter_func(__filter_bc_runtime, \ + __stackvar.__interpreter_stack_data, &filter_ctx) == LTTNG_UST_BYTECODE_INTERPRETER_OK)) { \ + if (caa_unlikely(filter_ctx.result == LTTNG_UST_BYTECODE_FILTER_ACCEPT)) { \ + __filter_record = 1; \ + break; \ + } \ } \ } \ if (caa_likely(!__filter_record)) \ diff --git a/liblttng-ust/event-notifier-notification.c b/liblttng-ust/event-notifier-notification.c index c403bcbf..b4c666aa 100644 --- a/liblttng-ust/event-notifier-notification.c +++ b/liblttng-ust/event-notifier-notification.c @@ -380,8 +380,8 @@ void lttng_event_notifier_notification_send( &event_notifier->capture_bytecode_runtime_head, node) { struct lttng_interpreter_output output; - if (capture_bc_runtime->interpreter_funcs.capture(capture_bc_runtime, - stack_data, &output) & LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG) + if (capture_bc_runtime->interpreter_func(capture_bc_runtime, + stack_data, &output) == LTTNG_UST_BYTECODE_INTERPRETER_OK) notification_append_capture(¬if, &output); else notification_append_empty_capture(¬if); diff --git a/liblttng-ust/lttng-bytecode-interpreter.c b/liblttng-ust/lttng-bytecode-interpreter.c index 15ce6363..d1f38f2b 100644 --- a/liblttng-ust/lttng-bytecode-interpreter.c +++ b/liblttng-ust/lttng-bytecode-interpreter.c @@ -148,17 +148,11 @@ int stack_strcmp(struct estack *stack, int top, const char *cmp_type) return diff; } -uint64_t lttng_bytecode_filter_interpret_false(void *filter_data, - const char *filter_stack_data) +int lttng_bytecode_interpret_error(struct lttng_ust_bytecode_runtime *bytecode_runtime, + const char *stack_data, + void *ctx) { - return LTTNG_UST_BYTECODE_INTERPRETER_DISCARD; -} - -uint64_t lttng_bytecode_capture_interpret_false(void *capture_data, - const char *capture_stack_data, - struct lttng_interpreter_output *output) -{ - return LTTNG_UST_BYTECODE_INTERPRETER_DISCARD; + return LTTNG_UST_BYTECODE_INTERPRETER_ERROR; } #ifdef INTERPRETER_USE_SWITCH @@ -700,7 +694,7 @@ again: return -EINVAL; } - return LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG; + return 0; } /* @@ -711,16 +705,14 @@ again: * For `output` not equal to NULL: * Return 0 on success, negative error value on error. */ -static -uint64_t bytecode_interpret(void *interpreter_data, +int lttng_bytecode_interpret(struct lttng_ust_bytecode_runtime *ust_bytecode, const char *interpreter_stack_data, - struct lttng_interpreter_output *output) + void *caller_ctx) { - struct bytecode_runtime *bytecode = interpreter_data; - struct lttng_ust_ctx *ctx = lttng_ust_rcu_dereference(*bytecode->p.priv->pctx); + struct bytecode_runtime *bytecode = caa_container_of(ust_bytecode, struct bytecode_runtime, p); + struct lttng_ust_ctx *ctx = lttng_ust_rcu_dereference(*ust_bytecode->priv->pctx); void *pc, *next_pc, *start_pc; - int ret = -EINVAL; - uint64_t retval = 0; + int ret = -EINVAL, retval = 0; struct estack _stack; struct estack *stack = &_stack; register int64_t ax = 0, bx = 0; @@ -876,7 +868,7 @@ uint64_t bytecode_interpret(void *interpreter_data, goto end; OP(BYTECODE_OP_RETURN): - /* LTTNG_UST_BYTECODE_INTERPRETER_DISCARD or LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG */ + /* LTTNG_UST_BYTECODE_INTERPRETER_ERROR or LTTNG_UST_BYTECODE_INTERPRETER_OK */ /* Handle dynamic typing. */ switch (estack_ax_t) { case REG_S64: @@ -886,7 +878,7 @@ uint64_t bytecode_interpret(void *interpreter_data, case REG_DOUBLE: case REG_STRING: case REG_PTR: - if (!output) { + if (ust_bytecode->priv->type != LTTNG_UST_BYTECODE_TYPE_CAPTURE) { ret = -EINVAL; goto end; } @@ -902,7 +894,7 @@ uint64_t bytecode_interpret(void *interpreter_data, goto end; OP(BYTECODE_OP_RETURN_S64): - /* LTTNG_UST_BYTECODE_INTERPRETER_DISCARD or LTTNG_UST_BYTECODE_INTERPRETER_RECORD_FLAG */ + /* LTTNG_UST_BYTECODE_INTERPRETER_ERROR or LTTNG_UST_BYTECODE_INTERPRETER_OK */ retval = !!estack_ax_v; ret = 0; goto end; @@ -2482,30 +2474,34 @@ uint64_t bytecode_interpret(void *interpreter_data, END_OP end: - /* Return _DISCARD on error. */ + /* No need to prepare output if an error occurred. */ if (ret) - return LTTNG_UST_BYTECODE_INTERPRETER_DISCARD; + return LTTNG_UST_BYTECODE_INTERPRETER_ERROR; - if (output) { - return lttng_bytecode_interpret_format_output(estack_ax(stack, top), - output); + /* Prepare output. */ + switch (ust_bytecode->priv->type) { + case LTTNG_UST_BYTECODE_TYPE_FILTER: + { + struct lttng_ust_bytecode_filter_ctx *filter_ctx = + (struct lttng_ust_bytecode_filter_ctx *) caller_ctx; + if (retval) + filter_ctx->result = LTTNG_UST_BYTECODE_FILTER_ACCEPT; + else + filter_ctx->result = LTTNG_UST_BYTECODE_FILTER_REJECT; + break; } - - return retval; -} - -uint64_t lttng_bytecode_filter_interpret(void *filter_data, - const char *filter_stack_data) -{ - return bytecode_interpret(filter_data, filter_stack_data, NULL); -} - -uint64_t lttng_bytecode_capture_interpret(void *capture_data, - const char *capture_stack_data, - struct lttng_interpreter_output *output) -{ - return bytecode_interpret(capture_data, capture_stack_data, - (struct lttng_interpreter_output *) output); + case LTTNG_UST_BYTECODE_TYPE_CAPTURE: + ret = lttng_bytecode_interpret_format_output(estack_ax(stack, top), + (struct lttng_interpreter_output *) caller_ctx); + break; + default: + ret = -EINVAL; + break; + } + if (ret) + return LTTNG_UST_BYTECODE_INTERPRETER_ERROR; + else + return LTTNG_UST_BYTECODE_INTERPRETER_OK; } #undef START_OP diff --git a/liblttng-ust/lttng-bytecode.c b/liblttng-ust/lttng-bytecode.c index d75600ba..6ce5e54a 100644 --- a/liblttng-ust/lttng-bytecode.c +++ b/liblttng-ust/lttng-bytecode.c @@ -432,6 +432,7 @@ int link_bytecode(struct lttng_ust_event_desc *event_desc, runtime->p.priv = runtime_priv; runtime->p.struct_size = sizeof(struct lttng_ust_bytecode_runtime); runtime_priv->pub = runtime; + runtime_priv->type = bytecode->type; runtime_priv->bc = bytecode; runtime_priv->pctx = ctx; runtime->len = bytecode->bc.reloc_offset; @@ -466,34 +467,14 @@ int link_bytecode(struct lttng_ust_event_desc *event_desc, goto link_error; } - switch (bytecode->type) { - case LTTNG_UST_BYTECODE_NODE_TYPE_FILTER: - runtime->p.interpreter_funcs.filter = lttng_bytecode_filter_interpret; - break; - case LTTNG_UST_BYTECODE_NODE_TYPE_CAPTURE: - runtime->p.interpreter_funcs.capture = lttng_bytecode_capture_interpret; - break; - default: - abort(); - } - + runtime->p.interpreter_func = lttng_bytecode_interpret; runtime->p.priv->link_failed = 0; cds_list_add_rcu(&runtime->p.node, insert_loc); dbg_printf("Linking successful.\n"); return 0; link_error: - switch (bytecode->type) { - case LTTNG_UST_BYTECODE_NODE_TYPE_FILTER: - runtime->p.interpreter_funcs.filter = lttng_bytecode_filter_interpret_false; - break; - case LTTNG_UST_BYTECODE_NODE_TYPE_CAPTURE: - runtime->p.interpreter_funcs.capture = lttng_bytecode_capture_interpret_false; - break; - default: - abort(); - } - + runtime->p.interpreter_func = lttng_bytecode_interpret_error; runtime_priv->link_failed = 1; cds_list_add_rcu(&runtime->p.node, insert_loc); alloc_error: @@ -501,24 +482,14 @@ alloc_error: return ret; } -void lttng_bytecode_filter_sync_state(struct lttng_ust_bytecode_runtime *runtime) -{ - struct lttng_ust_bytecode_node *bc = runtime->priv->bc; - - if (!bc->enabler->enabled || runtime->priv->link_failed) - runtime->interpreter_funcs.filter = lttng_bytecode_filter_interpret_false; - else - runtime->interpreter_funcs.filter = lttng_bytecode_filter_interpret; -} - -void lttng_bytecode_capture_sync_state(struct lttng_ust_bytecode_runtime *runtime) +void lttng_bytecode_sync_state(struct lttng_ust_bytecode_runtime *runtime) { struct lttng_ust_bytecode_node *bc = runtime->priv->bc; if (!bc->enabler->enabled || runtime->priv->link_failed) - runtime->interpreter_funcs.capture = lttng_bytecode_capture_interpret_false; + runtime->interpreter_func = lttng_bytecode_interpret_error; else - runtime->interpreter_funcs.capture = lttng_bytecode_capture_interpret; + runtime->interpreter_func = lttng_bytecode_interpret; } /* diff --git a/liblttng-ust/lttng-bytecode.h b/liblttng-ust/lttng-bytecode.h index 04dd2520..220e8d03 100644 --- a/liblttng-ust/lttng-bytecode.h +++ b/liblttng-ust/lttng-bytecode.h @@ -318,10 +318,7 @@ __attribute__((visibility("hidden"))) const char *lttng_bytecode_print_op(enum bytecode_op op); __attribute__((visibility("hidden"))) -void lttng_bytecode_filter_sync_state(struct lttng_ust_bytecode_runtime *runtime); - -__attribute__((visibility("hidden"))) -void lttng_bytecode_capture_sync_state(struct lttng_ust_bytecode_runtime *runtime); +void lttng_bytecode_sync_state(struct lttng_ust_bytecode_runtime *runtime); __attribute__((visibility("hidden"))) int lttng_bytecode_validate(struct bytecode_runtime *bytecode); @@ -331,21 +328,13 @@ int lttng_bytecode_specialize(struct lttng_ust_event_desc *event_desc, struct bytecode_runtime *bytecode); __attribute__((visibility("hidden"))) -uint64_t lttng_bytecode_filter_interpret_false(void *filter_data, - const char *filter_stack_data); - -__attribute__((visibility("hidden"))) -uint64_t lttng_bytecode_filter_interpret(void *filter_data, - const char *filter_stack_data); - -__attribute__((visibility("hidden"))) -uint64_t lttng_bytecode_capture_interpret_false(void *capture_data, - const char *capture_stack_data, - struct lttng_interpreter_output *output); +int lttng_bytecode_interpret_error(struct lttng_ust_bytecode_runtime *bytecode_runtime, + const char *stack_data, + void *ctx); __attribute__((visibility("hidden"))) -uint64_t lttng_bytecode_capture_interpret(void *capture_data, - const char *capture_stack_data, - struct lttng_interpreter_output *output); +int lttng_bytecode_interpret(struct lttng_ust_bytecode_runtime *bytecode_runtime, + const char *stack_data, + void *ctx); #endif /* _LTTNG_BYTECODE_H */ diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index bf74a1d9..9dc69ba9 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -1699,7 +1699,7 @@ void lttng_session_sync_event_enablers(struct lttng_ust_session *session) /* Enable filters */ cds_list_for_each_entry(runtime, &event_recorder_priv->pub->parent->filter_bytecode_runtime_head, node) { - lttng_bytecode_filter_sync_state(runtime); + lttng_bytecode_sync_state(runtime); } } lttng_ust_tp_probe_prune_release_queue(); @@ -1911,13 +1911,13 @@ void lttng_event_notifier_group_sync_enablers(struct lttng_event_notifier_group /* Enable filters */ cds_list_for_each_entry(runtime, &event_notifier_priv->pub->parent->filter_bytecode_runtime_head, node) { - lttng_bytecode_filter_sync_state(runtime); + lttng_bytecode_sync_state(runtime); } /* Enable captures. */ cds_list_for_each_entry(runtime, &event_notifier_priv->pub->capture_bytecode_runtime_head, node) { - lttng_bytecode_capture_sync_state(runtime); + lttng_bytecode_sync_state(runtime); } } lttng_ust_tp_probe_prune_release_queue(); diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index 14046381..42810864 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -768,7 +768,7 @@ int handle_bytecode_recv(struct sock_info *sock_info, int sock, struct ustcomm_ust_msg *lum) { struct lttng_ust_bytecode_node *bytecode = NULL; - enum lttng_ust_bytecode_node_type type; + enum lttng_ust_bytecode_type type; const struct lttng_ust_abi_objd_ops *ops; uint32_t data_size, data_size_max, reloc_offset; uint64_t seqnum; @@ -777,14 +777,14 @@ int handle_bytecode_recv(struct sock_info *sock_info, switch (lum->cmd) { case LTTNG_UST_ABI_FILTER: - type = LTTNG_UST_BYTECODE_NODE_TYPE_FILTER; + type = LTTNG_UST_BYTECODE_TYPE_FILTER; data_size = lum->u.filter.data_size; data_size_max = LTTNG_UST_ABI_FILTER_BYTECODE_MAX_LEN; reloc_offset = lum->u.filter.reloc_offset; seqnum = lum->u.filter.seqnum; break; case LTTNG_UST_ABI_CAPTURE: - type = LTTNG_UST_BYTECODE_NODE_TYPE_CAPTURE; + type = LTTNG_UST_BYTECODE_TYPE_CAPTURE; data_size = lum->u.capture.data_size; data_size_max = LTTNG_UST_ABI_CAPTURE_BYTECODE_MAX_LEN; reloc_offset = lum->u.capture.reloc_offset; diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h index 97e3974c..4e73e3e3 100644 --- a/liblttng-ust/ust-events-internal.h +++ b/liblttng-ust/ust-events-internal.h @@ -94,13 +94,13 @@ struct lttng_event_notifier_enabler { uint64_t num_captures; }; -enum lttng_ust_bytecode_node_type { - LTTNG_UST_BYTECODE_NODE_TYPE_FILTER, - LTTNG_UST_BYTECODE_NODE_TYPE_CAPTURE, +enum lttng_ust_bytecode_type { + LTTNG_UST_BYTECODE_TYPE_FILTER, + LTTNG_UST_BYTECODE_TYPE_CAPTURE, }; struct lttng_ust_bytecode_node { - enum lttng_ust_bytecode_node_type type; + enum lttng_ust_bytecode_type type; struct cds_list_head node; struct lttng_enabler *enabler; struct { @@ -275,6 +275,7 @@ struct lttng_ust_event_notifier_private { struct lttng_ust_bytecode_runtime_private { struct bytecode_runtime *pub; /* Public bytecode runtime interface */ + enum lttng_ust_bytecode_type type; struct lttng_ust_bytecode_node *bc; int link_failed; /* -- 2.34.1