> Registering a tracepoint handler was resulting so far in undefined behavior at
> runtime.
>
> The RTE_TRACE_POINT_REGISTER() macro was casting the tracepoint handler
> (which expects arguments) to a void (*)(void).
> At runtime, calling this handler while registering resulted in reading the
> current stack with no relation to this function prototype.
>
> Instead, declare an additional inline _register() handler for each tracepoint
> and make sure that the emitting macros in rte_trace_point_register.h only
> work on arguments name and type.
>
> The original tracepoint handler prototype is adjusted by adding a
> __rte_unused for each argument (since emitting macros do nothing with
> them).
> This last part introduces an implementation limit of 15 arguments.
>
> With this change in place, the workaround in dmadev tracepoints can be
> removed.
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> lib/dmadev/rte_dmadev_trace.h | 12 ------
> lib/dmadev/rte_dmadev_trace_fp.h | 14 -------
> lib/eal/include/rte_trace_point.h | 4 ++
> lib/eal/include/rte_trace_point_register.h | 44 +++++++++++++++++++---
> 4 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/lib/dmadev/rte_dmadev_trace.h
> b/lib/dmadev/rte_dmadev_trace.h index 1beb938168..1de92655f2 100644
> --- a/lib/dmadev/rte_dmadev_trace.h
> +++ b/lib/dmadev/rte_dmadev_trace.h
> @@ -22,10 +22,6 @@ extern "C" {
> RTE_TRACE_POINT(
> rte_dma_trace_info_get,
> RTE_TRACE_POINT_ARGS(int16_t dev_id, struct rte_dma_info
> *dev_info), -#ifdef _RTE_TRACE_POINT_REGISTER_H_
> - struct rte_dma_info __dev_info = {0};
> - dev_info = &__dev_info;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_string(dev_info->dev_name);
> rte_trace_point_emit_u64(dev_info->dev_capa);
> @@ -42,10 +38,6 @@ RTE_TRACE_POINT(
> rte_dma_trace_configure,
> RTE_TRACE_POINT_ARGS(int16_t dev_id, const struct rte_dma_conf
> *dev_conf,
> int ret),
> -#ifdef _RTE_TRACE_POINT_REGISTER_H_
> - const struct rte_dma_conf __dev_conf = {0};
> - dev_conf = &__dev_conf;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_u16(dev_conf->nb_vchans);
> rte_trace_point_emit_u16(dev_conf->priority);
> @@ -78,10 +70,6 @@ RTE_TRACE_POINT(
> rte_dma_trace_vchan_setup,
> RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan,
> const struct rte_dma_vchan_conf *conf, int ret), -
> #ifdef _RTE_TRACE_POINT_REGISTER_H_
> - const struct rte_dma_vchan_conf __conf = {0};
> - conf = &__conf;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_u16(vchan);
> rte_trace_point_emit_int(conf->direction);
> diff --git a/lib/dmadev/rte_dmadev_trace_fp.h
> b/lib/dmadev/rte_dmadev_trace_fp.h
> index f2656c6726..a1374e78b7 100644
> --- a/lib/dmadev/rte_dmadev_trace_fp.h
> +++ b/lib/dmadev/rte_dmadev_trace_fp.h
> @@ -33,10 +33,6 @@ RTE_TRACE_POINT_FP(
> rte_dma_trace_vchan_status,
> RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan,
> enum rte_dma_vchan_status *status, int ret), -
> #ifdef _RTE_TRACE_POINT_REGISTER_H_
> - enum rte_dma_vchan_status __status = 0;
> - status = &__status;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_u16(vchan);
> rte_trace_point_emit_int_ptr(status);
> @@ -100,12 +96,6 @@ RTE_TRACE_POINT_FP(
> RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan,
> const uint16_t nb_cpls, uint16_t *last_idx,
> bool *has_error, uint16_t ret), -#ifdef
> _RTE_TRACE_POINT_REGISTER_H_
> - uint16_t __last_idx = 0;
> - bool __has_error = false;
> - last_idx = &__last_idx;
> - has_error = &__has_error;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_u16(vchan);
> rte_trace_point_emit_u16(nb_cpls);
> @@ -119,10 +109,6 @@ RTE_TRACE_POINT_FP(
> RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan,
> const uint16_t nb_cpls, uint16_t *last_idx,
> enum rte_dma_status_code *status, uint16_t ret), -
> #ifdef _RTE_TRACE_POINT_REGISTER_H_
> - uint16_t __last_idx = 0;
> - last_idx = &__last_idx;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_u16(vchan);
> rte_trace_point_emit_u16(nb_cpls);
> diff --git a/lib/eal/include/rte_trace_point.h
> b/lib/eal/include/rte_trace_point.h
> index 84de233d59..8a317d31d2 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -36,6 +36,8 @@ extern "C" {
> /** The tracepoint object. */
> typedef RTE_ATOMIC(uint64_t) rte_trace_point_t;
>
> +#ifndef _RTE_TRACE_POINT_REGISTER_H_
> +
> /**
> * Macro to define the tracepoint arguments in RTE_TRACE_POINT macro.
>
> @@ -53,6 +55,8 @@ _tp _args \
> __VA_ARGS__ \
> }
>
> +#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> +
> /**
> * Create a tracepoint.
> *
> diff --git a/lib/eal/include/rte_trace_point_register.h
> b/lib/eal/include/rte_trace_point_register.h
> index ac391eb28f..b036121959 100644
> --- a/lib/eal/include/rte_trace_point_register.h
> +++ b/lib/eal/include/rte_trace_point_register.h
> @@ -18,6 +18,43 @@ extern "C" {
>
> RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
>
> +#define
> RTE_TRACE_POINT_ARGS_COUNT_(_0,_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_11,_1
> 2,_13,_14,_15,N, ...) \
> + N
> +#define RTE_TRACE_POINT_ARGS_COUNT(...) \
> +
> +RTE_TRACE_POINT_ARGS_COUNT_(__VA_ARGS__,15,14,13,12,11,10,9,8,7,6,
> 5,4,3
> +,2,1,0)
> +
> +#define RTE_TRACE_POINT_ARGS_1(a) __rte_unused a #define
> +RTE_TRACE_POINT_ARGS_2(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_1(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_3(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_2(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_4(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_3(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_5(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_4(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_6(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_5(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_7(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_6(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_8(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_7(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_9(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_8(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_10(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_9(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_11(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_10(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_12(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_11(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_13(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_12(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_14(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_13(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_15(a, ...) __rte_unused a,
> +RTE_TRACE_POINT_ARGS_14(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS_FUNC(a) RTE_TRACE_POINT_ARGS_ ## a
> #define
> +RTE_TRACE_POINT_ARGS_EXPAND(...) __VA_ARGS__ #define
> +RTE_TRACE_POINT_ARGS_(N, ...) \
> +
> RTE_TRACE_POINT_ARGS_EXPAND(RTE_TRACE_POINT_ARGS_FUNC(
> N))(__VA_ARGS__)
> +#define RTE_TRACE_POINT_ARGS(...) \
> + (RTE_TRACE_POINT_ARGS_(RTE_TRACE_POINT_ARGS_COUNT(0,
> __VA_ARGS__),
> +__VA_ARGS__))
> +
> +#define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \ extern
> +rte_trace_point_t __##_tp; \ static __rte_always_inline void _tp _args
> +{ } \ static __rte_always_inline void \ _tp ## _register (void) \ { \
> + __rte_trace_point_emit_header_##_mode(&__##_tp); \
> + __VA_ARGS__ \
> +}
> +
> #define RTE_TRACE_POINT_REGISTER(trace, name) \ rte_trace_point_t
> __rte_section("__rte_trace_point") __##trace; \ static const char
> __##trace##_name[] = RTE_STR(name); \ @@ -26,7 +63,7 @@
> RTE_INIT(trace##_init) \
> if (!rte_trace_feature_is_enabled()) \
> return; \
> __rte_trace_point_register(&__##trace, __##trace##_name, \
> - (void (*)(void)) trace); \
> + trace ## _register); \
> }
>
> #define __rte_trace_point_emit_header_generic(t) \ @@ -37,22 +74,17 @@
> RTE_INIT(trace##_init) \
>
> #define __rte_trace_point_emit(name, in, type) \ do { \
> - RTE_BUILD_BUG_ON(sizeof(type) != sizeof(typeof(*in))); \
> __rte_trace_point_emit_field(sizeof(type), name, RTE_STR(type)); \ }
> while (0)
>
> #define rte_trace_point_emit_string(in) \ do { \
> - RTE_SET_USED(in); \
>
> __rte_trace_point_emit_field(__RTE_TRACE_EMIT_STRING_LEN_MAX
> , \
> RTE_STR(in)"[32]", "string_bounded_t"); \ } while (0)
>
> #define rte_trace_point_emit_blob(in, len) \ do { \
> - uint8_t size = len; \
> - RTE_SET_USED(in); \
> - RTE_SET_USED(size); \
> __rte_trace_point_emit_field(sizeof(uint8_t), \
> RTE_STR(in) "_size", \
> RTE_STR(uint8_t)); \
> --
> 2.48.1
Acked-by: Sunil Kumar Kori <sk...@marvell.com>
