From c6af61dc9093cd7d47fce4c6a847a8eabb08d18f Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 11 Dec 2023 11:23:10 -0500 Subject: [PATCH] Add indirection for visitor arguments In order to keep the argument vector const, add an indirection for visitor arguments to a non-const area of memory, which is clearly documented as modified by libside. Add a "cached_arg" placeholder pointer to the visitor structures. Signed-off-by: Mathieu Desnoyers --- include/side/abi/type-argument.h | 28 +++++++++++---- include/side/instrumentation-c-api.h | 53 ++++++++++++++++++++-------- src/tracer.c | 40 ++++++++++++++------- tests/unit/test.c | 17 ++++++--- 4 files changed, 98 insertions(+), 40 deletions(-) diff --git a/include/side/abi/type-argument.h b/include/side/abi/type-argument.h index fd1979c..57ce525 100644 --- a/include/side/abi/type-argument.h +++ b/include/side/abi/type-argument.h @@ -61,6 +61,13 @@ enum side_arg_flag { SIDE_ARG_FLAG_INCOMPLETE = (1U << SIDE_ARG_FLAG_INCOMPLETE_BIT), }; +struct side_arg_vla_visitor { + side_ptr_t(void) app_ctx; + /* libside argument cache, initialize to NULL. */ + side_ptr_t(struct side_arg) cached_arg; +} SIDE_PACKED; +side_check_size(struct side_arg_vla_visitor, 32); + union side_arg_static { /* Stack-copy basic types */ union side_bool_value bool_value; @@ -74,7 +81,8 @@ union side_arg_static { side_ptr_t(const struct side_arg_variant) side_variant; side_ptr_t(const struct side_arg_vec) side_array; side_ptr_t(const struct side_arg_vec) side_vla; - void *side_vla_app_visitor_ctx; + /* Pointer to non-const structure. Content modified by libside. */ + side_ptr_t(struct side_arg_vla_visitor) side_vla_visitor; /* Gather basic types */ side_ptr_t(const void) side_bool_gather_ptr; @@ -110,21 +118,25 @@ struct side_arg_dynamic_struct { } SIDE_PACKED; side_check_size(struct side_arg_dynamic_struct, 40); -struct side_dynamic_struct_visitor { +struct side_arg_dynamic_struct_visitor { side_func_ptr_t(side_dynamic_struct_visitor_func) visitor; side_ptr_t(void) app_ctx; side_ptr_t(const struct side_attr) attr; + /* libside argument cache, initialize to NULL. */ + side_ptr_t(struct side_arg) cached_arg; uint32_t nr_attr; } SIDE_PACKED; -side_check_size(struct side_dynamic_struct_visitor, 52); +side_check_size(struct side_arg_dynamic_struct_visitor, 68); -struct side_dynamic_vla_visitor { +struct side_arg_dynamic_vla_visitor { side_func_ptr_t(side_visitor_func) visitor; side_ptr_t(void) app_ctx; side_ptr_t(const struct side_attr) attr; + /* libside argument cache, initialize to NULL. */ + side_ptr_t(struct side_arg) cached_arg; uint32_t nr_attr; } SIDE_PACKED; -side_check_size(struct side_dynamic_vla_visitor, 52); +side_check_size(struct side_arg_dynamic_vla_visitor, 68); union side_arg_dynamic { /* Dynamic basic types */ @@ -154,8 +166,10 @@ union side_arg_dynamic { side_ptr_t(const struct side_arg_dynamic_struct) side_dynamic_struct; side_ptr_t(const struct side_arg_dynamic_vla) side_dynamic_vla; - struct side_dynamic_struct_visitor side_dynamic_struct_visitor; - struct side_dynamic_vla_visitor side_dynamic_vla_visitor; + /* Pointer to non-const structure. Content modified by libside. */ + side_ptr_t(struct side_arg_dynamic_struct_visitor) side_dynamic_struct_visitor; + /* Pointer to non-const structure. Content modified by libside. */ + side_ptr_t(struct side_arg_dynamic_vla_visitor) side_dynamic_vla_visitor; side_padding(58); } SIDE_PACKED; diff --git a/include/side/instrumentation-c-api.h b/include/side/instrumentation-c-api.h index 43ba3c0..00cd20b 100644 --- a/include/side/instrumentation-c-api.h +++ b/include/side/instrumentation-c-api.h @@ -834,7 +834,22 @@ #define side_arg_array(_side_type) { .type = SIDE_ENUM_INIT(SIDE_TYPE_ARRAY), .flags = 0, .u = { .side_static = { .side_array = SIDE_PTR_INIT(_side_type) } } } #define side_arg_vla(_side_type) { .type = SIDE_ENUM_INIT(SIDE_TYPE_VLA), .flags = 0, .u = { .side_static = { .side_vla = SIDE_PTR_INIT(_side_type) } } } -#define side_arg_vla_visitor(_ctx) { .type = SIDE_ENUM_INIT(SIDE_TYPE_VLA_VISITOR), .flags = 0, .u = { .side_static = { .side_vla_app_visitor_ctx = (_ctx) } } } +#define side_arg_vla_visitor(_side_vla_visitor) \ + { \ + .type = SIDE_ENUM_INIT(SIDE_TYPE_VLA_VISITOR), \ + .flags = 0, \ + .u = { \ + .side_static = { \ + .side_vla_visitor = SIDE_PTR_INIT(_side_vla_visitor), \ + } \ + } \ + } + +#define side_arg_define_vla_visitor(_identifier, _ctx) \ + struct side_arg_vla_visitor _identifier = { \ + .app_ctx = SIDE_PTR_INIT(_ctx), \ + .cached_arg = SIDE_PTR_INIT(NULL), \ + } /* Gather field arguments */ @@ -1072,18 +1087,13 @@ }, \ } -#define side_arg_dynamic_vla_visitor(_dynamic_vla_visitor, _ctx, _attr...) \ +#define side_arg_dynamic_vla_visitor(_dynamic_vla_visitor) \ { \ .type = SIDE_ENUM_INIT(SIDE_TYPE_DYNAMIC_VLA_VISITOR), \ .flags = 0, \ .u = { \ .side_dynamic = { \ - .side_dynamic_vla_visitor = { \ - .app_ctx = SIDE_PTR_INIT(_ctx), \ - .visitor = SIDE_PTR_INIT(_dynamic_vla_visitor), \ - .attr = SIDE_PTR_INIT(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ - .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ - }, \ + .side_dynamic_vla_visitor = SIDE_PTR_INIT(_dynamic_vla_visitor), \ }, \ }, \ } @@ -1099,18 +1109,13 @@ }, \ } -#define side_arg_dynamic_struct_visitor(_dynamic_struct_visitor, _ctx, _attr...) \ +#define side_arg_dynamic_struct_visitor(_dynamic_struct_visitor) \ { \ .type = SIDE_ENUM_INIT(SIDE_TYPE_DYNAMIC_STRUCT_VISITOR), \ .flags = 0, \ .u = { \ .side_dynamic = { \ - .side_dynamic_struct_visitor = { \ - .app_ctx = SIDE_PTR_INIT(_ctx), \ - .visitor = SIDE_PTR_INIT(_dynamic_struct_visitor), \ - .attr = SIDE_PTR_INIT(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ - .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ - }, \ + .side_dynamic_struct_visitor = SIDE_PTR_INIT(_dynamic_struct_visitor), \ }, \ }, \ } @@ -1133,6 +1138,24 @@ .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ } +#define side_arg_dynamic_define_struct_visitor(_identifier, _dynamic_struct_visitor, _ctx, _attr...) \ + struct side_arg_dynamic_struct_visitor _identifier = { \ + .visitor = SIDE_PTR_INIT(_dynamic_struct_visitor), \ + .app_ctx = SIDE_PTR_INIT(_ctx), \ + .attr = SIDE_PTR_INIT(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ + .cached_arg = SIDE_PTR_INIT(NULL), \ + .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ + } + +#define side_arg_dynamic_define_vla_visitor(_identifier, _dynamic_vla_visitor, _ctx, _attr...) \ + struct side_arg_dynamic_vla_visitor _identifier = { \ + .visitor = SIDE_PTR_INIT(_dynamic_vla_visitor), \ + .app_ctx = SIDE_PTR_INIT(_ctx), \ + .attr = SIDE_PTR_INIT(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ + .cached_arg = SIDE_PTR_INIT(NULL), \ + .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ + } + #define side_arg_define_vec(_identifier, _sav) \ const struct side_arg _identifier##_vec[] = { _sav }; \ const struct side_arg_vec _identifier = { \ diff --git a/src/tracer.c b/src/tracer.c index b4089a7..e4ccd86 100644 --- a/src/tracer.c +++ b/src/tracer.c @@ -36,7 +36,7 @@ void tracer_print_array(const struct side_type *type_desc, const struct side_arg static void tracer_print_vla(const struct side_type *type_desc, const struct side_arg_vec *side_arg_vec); static -void tracer_print_vla_visitor(const struct side_type *type_desc, void *app_ctx); +void tracer_print_vla_visitor(const struct side_type *type_desc, struct side_arg_vla_visitor *vla_visitor); static void tracer_print_dynamic(const struct side_arg *dynamic_item); static @@ -1242,7 +1242,7 @@ void tracer_print_type(const struct side_type *type_desc, const struct side_arg tracer_print_vla(type_desc, side_ptr_get(item->u.side_static.side_vla)); break; case SIDE_TYPE_VLA_VISITOR: - tracer_print_vla_visitor(type_desc, item->u.side_static.side_vla_app_visitor_ctx); + tracer_print_vla_visitor(type_desc, side_ptr_get(item->u.side_static.side_vla_visitor)); break; /* Stack-copy enumeration types */ @@ -1792,8 +1792,9 @@ enum side_visitor_status tracer_write_elem_cb(const struct side_tracer_visitor_c } static -void tracer_print_vla_visitor(const struct side_type *type_desc, void *app_ctx) +void tracer_print_vla_visitor(const struct side_type *type_desc, struct side_arg_vla_visitor *vla_visitor) { + void *app_ctx; enum side_visitor_status status; struct tracer_visitor_priv tracer_priv = { .elem_type = side_ptr_get(type_desc->u.side_vla_visitor.elem_type), @@ -1805,6 +1806,9 @@ void tracer_print_vla_visitor(const struct side_type *type_desc, void *app_ctx) }; side_visitor_func func; + if (!vla_visitor) + abort(); + app_ctx = side_ptr_get(vla_visitor->app_ctx); print_attributes("attr", ":", side_ptr_get(type_desc->u.side_vla_visitor.attr), type_desc->u.side_vla_visitor.nr_attr); printf("%s", type_desc->u.side_vla_visitor.nr_attr ? ", " : ""); printf("elements: "); @@ -1860,7 +1864,7 @@ enum side_visitor_status tracer_dynamic_struct_write_elem_cb( static void tracer_print_dynamic_struct_visitor(const struct side_arg *item) { - enum side_visitor_status status; + struct side_arg_dynamic_struct_visitor *dynamic_struct_visitor; struct tracer_dynamic_struct_visitor_priv tracer_priv = { .i = 0, }; @@ -1868,13 +1872,18 @@ void tracer_print_dynamic_struct_visitor(const struct side_arg *item) .write_field = tracer_dynamic_struct_write_elem_cb, .priv = &tracer_priv, }; - void *app_ctx = side_ptr_get(item->u.side_dynamic.side_dynamic_struct_visitor.app_ctx); + enum side_visitor_status status; + void *app_ctx; - print_attributes("attr", "::", side_ptr_get(item->u.side_dynamic.side_dynamic_struct_visitor.attr), item->u.side_dynamic.side_dynamic_struct_visitor.nr_attr); - printf("%s", item->u.side_dynamic.side_dynamic_struct_visitor.nr_attr ? ", " : ""); + dynamic_struct_visitor = side_ptr_get(item->u.side_dynamic.side_dynamic_struct_visitor); + if (!dynamic_struct_visitor) + abort(); + app_ctx = side_ptr_get(dynamic_struct_visitor->app_ctx); + print_attributes("attr", "::", side_ptr_get(dynamic_struct_visitor->attr), dynamic_struct_visitor->nr_attr); + printf("%s", dynamic_struct_visitor->nr_attr ? ", " : ""); printf("fields:: "); printf("[ "); - status = side_ptr_get(item->u.side_dynamic.side_dynamic_struct_visitor.visitor)(&tracer_ctx, app_ctx); + status = side_ptr_get(dynamic_struct_visitor->visitor)(&tracer_ctx, app_ctx); switch (status) { case SIDE_VISITOR_STATUS_OK: break; @@ -1922,7 +1931,7 @@ enum side_visitor_status tracer_dynamic_vla_write_elem_cb( static void tracer_print_dynamic_vla_visitor(const struct side_arg *item) { - enum side_visitor_status status; + struct side_arg_dynamic_vla_visitor *dynamic_vla_visitor; struct tracer_dynamic_vla_visitor_priv tracer_priv = { .i = 0, }; @@ -1930,13 +1939,18 @@ void tracer_print_dynamic_vla_visitor(const struct side_arg *item) .write_elem = tracer_dynamic_vla_write_elem_cb, .priv = &tracer_priv, }; - void *app_ctx = side_ptr_get(item->u.side_dynamic.side_dynamic_vla_visitor.app_ctx); + enum side_visitor_status status; + void *app_ctx; - print_attributes("attr", "::", side_ptr_get(item->u.side_dynamic.side_dynamic_vla_visitor.attr), item->u.side_dynamic.side_dynamic_vla_visitor.nr_attr); - printf("%s", item->u.side_dynamic.side_dynamic_vla_visitor.nr_attr ? ", " : ""); + dynamic_vla_visitor = side_ptr_get(item->u.side_dynamic.side_dynamic_vla_visitor); + if (!dynamic_vla_visitor) + abort(); + app_ctx = side_ptr_get(dynamic_vla_visitor->app_ctx); + print_attributes("attr", "::", side_ptr_get(dynamic_vla_visitor->attr), dynamic_vla_visitor->nr_attr); + printf("%s", dynamic_vla_visitor->nr_attr ? ", " : ""); printf("elements:: "); printf("[ "); - status = side_ptr_get(item->u.side_dynamic.side_dynamic_vla_visitor.visitor)(&tracer_ctx, app_ctx); + status = side_ptr_get(dynamic_vla_visitor->visitor)(&tracer_ctx, app_ctx); switch (status) { case SIDE_VISITOR_STATUS_OK: break; diff --git a/tests/unit/test.c b/tests/unit/test.c index 7b14279..ef5e7c0 100644 --- a/tests/unit/test.c +++ b/tests/unit/test.c @@ -186,7 +186,9 @@ void test_vla_visitor(void) .ptr = testarray, .length = SIDE_ARRAY_SIZE(testarray), }; - side_event_call(my_provider_event_vla_visitor, side_arg_list(side_arg_vla_visitor(&ctx), side_arg_s64(42))); + side_arg_define_vla_visitor(side_visitor, &ctx); + side_event_call(my_provider_event_vla_visitor, + side_arg_list(side_arg_vla_visitor(&side_visitor), side_arg_s64(42))); } } @@ -228,7 +230,8 @@ enum side_visitor_status test_outer_visitor(const struct side_tracer_visitor_ctx .ptr = ctx->ptr[i], .length = 2, }; - const struct side_arg elem = side_arg_vla_visitor(&inner_ctx); + side_arg_define_vla_visitor(side_inner_visitor, &inner_ctx); + const struct side_arg elem = side_arg_vla_visitor(&side_inner_visitor); if (tracer_ctx->write_elem(tracer_ctx, &elem) != SIDE_VISITOR_STATUS_OK) return SIDE_VISITOR_STATUS_ERROR; } @@ -262,7 +265,9 @@ void test_vla_visitor_2d(void) .ptr = testarray2d, .length = SIDE_ARRAY_SIZE(testarray2d), }; - side_event_call(my_provider_event_vla_visitor2d, side_arg_list(side_arg_vla_visitor(&ctx), side_arg_s64(42))); + side_arg_define_vla_visitor(side_outer_visitor, &ctx); + side_event_call(my_provider_event_vla_visitor2d, + side_arg_list(side_arg_vla_visitor(&side_outer_visitor), side_arg_s64(42))); } } @@ -596,9 +601,10 @@ void test_dynamic_vla_with_visitor(void) .ptr = testarray_dynamic_vla, .length = SIDE_ARRAY_SIZE(testarray_dynamic_vla), }; + side_arg_dynamic_define_vla_visitor(myvlavisitor, test_dynamic_vla_visitor, &ctx); side_event_call(my_provider_event_dynamic_vla_visitor, side_arg_list( - side_arg_dynamic_vla_visitor(test_dynamic_vla_visitor, &ctx) + side_arg_dynamic_vla_visitor(&myvlavisitor) ) ); } @@ -652,9 +658,10 @@ void test_dynamic_struct_with_visitor(void) .ptr = testarray_dynamic_struct, .length = SIDE_ARRAY_SIZE(testarray_dynamic_struct), }; + side_arg_dynamic_define_struct_visitor(mystructvisitor, test_dynamic_struct_visitor, &ctx); side_event_call(my_provider_event_dynamic_struct_visitor, side_arg_list( - side_arg_dynamic_struct_visitor(test_dynamic_struct_visitor, &ctx) + side_arg_dynamic_struct_visitor(&mystructvisitor) ) ); } -- 2.34.1