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 <mathieu.desnoyers@efficios.com>
Change-Id: Id528e141d0c213e9bb41f68d6d7e632104daa1cf

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index 367d2d1aaa33512d4374ba4bcabe2cdbe8e137aa..58a0baf4a5c92713d4658c58589010c828128f75 100644 (file)
--- 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 af51f9bc95cf5dae50ae9f231f5498639b9d2592..4fc1ea91aa8c6f49abe3e5871221d4db5110f0af 100644 (file)
--- 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 c403bcbf7e21d21c489653e1fd5c1fd92b29f5a6..b4c666aa560364e064999bbb40ba027e7c131b7a 100644 (file)
--- 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(&notif, &output);
                        else
                                notification_append_empty_capture(&notif);

diff --git a/liblttng-ust/lttng-bytecode-interpreter.c b/liblttng-ust/lttng-bytecode-interpreter.c
index 15ce63634030bff71744e920c209e81f8e3c7807..d1f38f2b06665db3c89a90e06505571ce7aeb247 100644 (file)
--- 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 d75600baee9a92e83e68cdae30a981bfd8c416f0..6ce5e54a39827bf385f5491169a630712a0650fe 100644 (file)
--- 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 04dd2520205e47c8e570b3c8b08e55bd286b9d72..220e8d03a48ef1cef9fa2a103bdb6dfe0cb99647 100644 (file)
--- 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 bf74a1d9cfc3ec48b13d04accda4d6303cafc4b3..9dc69ba9322d836757caa087c6bbfb43f0e33372 100644 (file)
--- 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 14046381789b6804bc31a3149d01cd3f10f765b4..428108646b765d00b9e5df272ec7e97de0bcc1ed 100644 (file)
--- 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 97e3974c2333f77ed15e51aca9c7f5a9a0de2ffb..4e73e3e3e783c41222be8bd7d13fde16bebc623e 100644 (file)
--- 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;
        /*