From 362a65de6aba2cbc27c61c9fc23e755cb617837f Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 11 Mar 2021 15:26:03 -0500 Subject: [PATCH] Refactoring: introduce bytecode runtime private structure Move private bytecode runtime fields to private structure. Made possible by major ABI bump. Signed-off-by: Mathieu Desnoyers Change-Id: I142d0edafb2fe1231454d145822b60a8a8a91c6e --- include/lttng/ust-events.h | 18 +++++------- liblttng-ust/lttng-bytecode-interpreter.c | 3 +- liblttng-ust/lttng-bytecode-specialize.c | 8 +++--- liblttng-ust/lttng-bytecode-validator.c | 6 ++-- liblttng-ust/lttng-bytecode.c | 35 +++++++++++++++-------- liblttng-ust/ust-events-internal.h | 12 ++++++++ 6 files changed, 51 insertions(+), 31 deletions(-) diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index b44fba59..c739f71d 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -361,16 +361,18 @@ enum lttng_bytecode_interpreter_ret { }; struct lttng_interpreter_output; +struct lttng_ust_bytecode_runtime_private; /* - * This structure is used in the probes. More specifically, the `filter` and - * `node` fields are explicity used in the probes. When modifying this - * structure we must not change the layout of these two fields as it is - * considered ABI. + * This structure is used in the probes. More specifically, the + * `interpreter_funcs` and `node` fields are explicity used in the + * probes. When modifying this structure we must not change the layout + * of these two fields as it is considered ABI. */ struct lttng_bytecode_runtime { + struct lttng_ust_bytecode_runtime_private *priv; + /* Associated bytecode */ - struct lttng_ust_bytecode_node *bc; union { uint64_t (*filter)(void *interpreter_data, const char *interpreter_stack_data); @@ -378,13 +380,7 @@ struct lttng_bytecode_runtime { const char *interpreter_stack_data, struct lttng_interpreter_output *interpreter_output); } interpreter_funcs; - int link_failed; struct cds_list_head node; /* list of bytecode runtime in event */ - /* - * Pointer to a URCU-protected pointer owned by an `struct - * lttng_session`or `struct lttng_event_notifier_group`. - */ - struct lttng_ctx **pctx; }; /* diff --git a/liblttng-ust/lttng-bytecode-interpreter.c b/liblttng-ust/lttng-bytecode-interpreter.c index 9def26cd..2dfc21fb 100644 --- a/liblttng-ust/lttng-bytecode-interpreter.c +++ b/liblttng-ust/lttng-bytecode-interpreter.c @@ -13,6 +13,7 @@ #include #include #include +#include "ust-events-internal.h" #include "lttng-bytecode.h" #include "string-utils.h" @@ -752,7 +753,7 @@ uint64_t bytecode_interpret(void *interpreter_data, struct lttng_interpreter_output *output) { struct bytecode_runtime *bytecode = interpreter_data; - struct lttng_ctx *ctx = lttng_ust_rcu_dereference(*bytecode->p.pctx); + struct lttng_ctx *ctx = lttng_ust_rcu_dereference(*bytecode->p.priv->pctx); void *pc, *next_pc, *start_pc; int ret = -EINVAL; uint64_t retval = 0; diff --git a/liblttng-ust/lttng-bytecode-specialize.c b/liblttng-ust/lttng-bytecode-specialize.c index 2bbcb197..ee65ff3b 100644 --- a/liblttng-ust/lttng-bytecode-specialize.c +++ b/liblttng-ust/lttng-bytecode-specialize.c @@ -380,7 +380,7 @@ static int specialize_context_lookup_name(struct lttng_ctx *ctx, const char *name; offset = ((struct get_symbol *) insn->data)->offset; - name = bytecode->p.bc->bc.data + bytecode->p.bc->bc.reloc_offset + offset; + name = bytecode->p.priv->bc->bc.data + bytecode->p.priv->bc->bc.reloc_offset + offset; return lttng_get_context_index(ctx, name); } @@ -549,7 +549,7 @@ static int specialize_app_context_lookup(struct lttng_ctx **pctx, ssize_t data_offset; offset = ((struct get_symbol *) insn->data)->offset; - orig_name = runtime->p.bc->bc.data + runtime->p.bc->bc.reloc_offset + offset; + orig_name = runtime->p.priv->bc->bc.data + runtime->p.priv->bc->bc.reloc_offset + offset; name = zmalloc(strlen(orig_name) + strlen("$app.") + 1); if (!name) { ret = -ENOMEM; @@ -610,7 +610,7 @@ static int specialize_payload_lookup(const struct lttng_event_desc *event_desc, nr_fields = event_desc->nr_fields; offset = ((struct get_symbol *) insn->data)->offset; - name = runtime->p.bc->bc.data + runtime->p.bc->bc.reloc_offset + offset; + name = runtime->p.priv->bc->bc.data + runtime->p.priv->bc->bc.reloc_offset + offset; for (i = 0; i < nr_fields; i++) { field = &event_desc->fields[i]; if (field->u.ext.nofilter) { @@ -680,7 +680,7 @@ int lttng_bytecode_specialize(const struct lttng_event_desc *event_desc, int ret = -EINVAL; struct vstack _stack; struct vstack *stack = &_stack; - struct lttng_ctx **pctx = bytecode->p.pctx; + struct lttng_ctx **pctx = bytecode->p.priv->pctx; vstack_init(stack); diff --git a/liblttng-ust/lttng-bytecode-validator.c b/liblttng-ust/lttng-bytecode-validator.c index cbae3215..b5bc4904 100644 --- a/liblttng-ust/lttng-bytecode-validator.c +++ b/liblttng-ust/lttng-bytecode-validator.c @@ -255,11 +255,11 @@ int validate_get_symbol(struct bytecode_runtime *bytecode, const char *str, *str_limit; size_t len_limit; - if (sym->offset >= bytecode->p.bc->bc.len - bytecode->p.bc->bc.reloc_offset) + if (sym->offset >= bytecode->p.priv->bc->bc.len - bytecode->p.priv->bc->bc.reloc_offset) return -EINVAL; - str = bytecode->p.bc->bc.data + bytecode->p.bc->bc.reloc_offset + sym->offset; - str_limit = bytecode->p.bc->bc.data + bytecode->p.bc->bc.len; + str = bytecode->p.priv->bc->bc.data + bytecode->p.priv->bc->bc.reloc_offset + sym->offset; + str_limit = bytecode->p.priv->bc->bc.data + bytecode->p.priv->bc->bc.len; len_limit = str_limit - str; if (strnlen(str, len_limit) == len_limit) return -EINVAL; diff --git a/liblttng-ust/lttng-bytecode.c b/liblttng-ust/lttng-bytecode.c index 79349570..c52c79bb 100644 --- a/liblttng-ust/lttng-bytecode.c +++ b/liblttng-ust/lttng-bytecode.c @@ -281,7 +281,7 @@ int apply_context_reloc(struct bytecode_runtime *runtime, struct load_op *op; struct lttng_ctx_field *ctx_field; int idx; - struct lttng_ctx **pctx = runtime->p.pctx; + struct lttng_ctx **pctx = runtime->p.priv->pctx; dbg_printf("Apply context reloc: %u %s\n", reloc_offset, context_name); @@ -393,7 +393,7 @@ int bytecode_is_linked(struct lttng_ust_bytecode_node *bytecode, struct lttng_bytecode_runtime *bc_runtime; cds_list_for_each_entry(bc_runtime, bytecode_runtime_head, node) { - if (bc_runtime->bc == bytecode) + if (bc_runtime->priv->bc == bytecode) return 1; } return 0; @@ -411,6 +411,7 @@ int link_bytecode(const struct lttng_event_desc *event_desc, { int ret, offset, next_offset; struct bytecode_runtime *runtime = NULL; + struct lttng_ust_bytecode_runtime_private *runtime_priv = NULL; size_t runtime_alloc_len; if (!bytecode) @@ -428,8 +429,17 @@ int link_bytecode(const struct lttng_event_desc *event_desc, ret = -ENOMEM; goto alloc_error; } - runtime->p.bc = bytecode; - runtime->p.pctx = ctx; + runtime_priv = zmalloc(sizeof(struct lttng_ust_bytecode_runtime_private)); + if (!runtime_priv) { + free(runtime); + runtime = NULL; + ret = -ENOMEM; + goto alloc_error; + } + runtime->p.priv = runtime_priv; + runtime_priv->pub = runtime; + runtime_priv->bc = bytecode; + runtime_priv->pctx = ctx; runtime->len = bytecode->bc.reloc_offset; /* copy original bytecode */ memcpy(runtime->code, bytecode->bc.data, runtime->len); @@ -473,7 +483,7 @@ int link_bytecode(const struct lttng_event_desc *event_desc, abort(); } - runtime->p.link_failed = 0; + runtime->p.priv->link_failed = 0; cds_list_add_rcu(&runtime->p.node, insert_loc); dbg_printf("Linking successful.\n"); return 0; @@ -490,7 +500,7 @@ link_error: abort(); } - runtime->p.link_failed = 1; + runtime_priv->link_failed = 1; cds_list_add_rcu(&runtime->p.node, insert_loc); alloc_error: dbg_printf("Linking failed.\n"); @@ -499,9 +509,9 @@ alloc_error: void lttng_bytecode_filter_sync_state(struct lttng_bytecode_runtime *runtime) { - struct lttng_ust_bytecode_node *bc = runtime->bc; + struct lttng_ust_bytecode_node *bc = runtime->priv->bc; - if (!bc->enabler->enabled || runtime->link_failed) + 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; @@ -509,9 +519,9 @@ void lttng_bytecode_filter_sync_state(struct lttng_bytecode_runtime *runtime) void lttng_bytecode_capture_sync_state(struct lttng_bytecode_runtime *runtime) { - struct lttng_ust_bytecode_node *bc = runtime->bc; + struct lttng_ust_bytecode_node *bc = runtime->priv->bc; - if (!bc->enabler->enabled || runtime->link_failed) + if (!bc->enabler->enabled || runtime->priv->link_failed) runtime->interpreter_funcs.capture = lttng_bytecode_capture_interpret_false; else runtime->interpreter_funcs.capture = lttng_bytecode_capture_interpret; @@ -545,7 +555,7 @@ void lttng_enabler_link_bytecode(const struct lttng_event_desc *event_desc, * linked with the instance. */ cds_list_for_each_entry(runtime, instance_bytecode_head, node) { - if (runtime->bc == enabler_bc) { + if (runtime->priv->bc == enabler_bc) { found = 1; break; } @@ -565,7 +575,7 @@ void lttng_enabler_link_bytecode(const struct lttng_event_desc *event_desc, */ cds_list_for_each_entry_reverse(runtime, instance_bytecode_head, node) { - if (runtime->bc->bc.seqnum <= enabler_bc->bc.seqnum) { + if (runtime->priv->bc->bc.seqnum <= enabler_bc->bc.seqnum) { /* insert here */ insert_loc = &runtime->node; goto add_within; @@ -601,6 +611,7 @@ void free_filter_runtime(struct cds_list_head *bytecode_runtime_head) cds_list_for_each_entry_safe(runtime, tmp, bytecode_runtime_head, p.node) { free(runtime->data); + free(runtime->p.priv); free(runtime); } } diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h index 1a94ba0e..536a70d9 100644 --- a/liblttng-ust/ust-events-internal.h +++ b/liblttng-ust/ust-events-internal.h @@ -178,6 +178,18 @@ struct lttng_ust_event_private { int registered; /* has reg'd tracepoint probe */ }; +struct lttng_ust_bytecode_runtime_private { + struct bytecode_runtime *pub; /* Public bytecode runtime interface */ + + struct lttng_ust_bytecode_node *bc; + int link_failed; + /* + * Pointer to a URCU-protected pointer owned by an `struct + * lttng_session`or `struct lttng_event_notifier_group`. + */ + struct lttng_ctx **pctx; +}; + static inline struct lttng_enabler *lttng_event_enabler_as_enabler( struct lttng_event_enabler *event_enabler) -- 2.34.1